Re: [Intel-gfx] [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

2019-09-13 Thread Laurent Pinchart
Hi José,

Thank you for the patch.

On Thu, Sep 12, 2019 at 12:51:31PM -0700, José Roberto de Souza wrote:
> This 3 non-atomic drivers all have the same function getting the
> only encoder available in the connector, also atomic drivers have
> this fallback. So moving it a common place and sharing between atomic
> and non-atomic drivers.
> 
> While at it I also removed the mention of
> drm_atomic_helper_best_encoder() that was renamed in
> commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
> drm_atomic_helper_best_encoder").
> 
> v3: moving drm_connector_get_single_encoder to drm_kms_helper module
> 
> Suggested-by: Ville Syrjälä 
> Cc: Ville Syrjälä 
> Cc: Daniel Vetter 
> Cc: Laurent Pinchart 
> Cc: dri-de...@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: José Roberto de Souza 
> ---
>  drivers/gpu/drm/ast/ast_mode.c | 12 
>  drivers/gpu/drm/drm_atomic_helper.c| 15 ++-
>  drivers/gpu/drm/drm_crtc_helper.c  | 17 -
>  drivers/gpu/drm/drm_crtc_helper_internal.h |  3 +++
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 11 ---
>  drivers/gpu/drm/udl/udl_connector.c|  8 
>  include/drm/drm_modeset_helper_vtables.h   |  6 +++---
>  7 files changed, 24 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index d349c721501c..eef95e1af06b 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct drm_encoder 
> *encoder)
>   kfree(encoder);
>  }
>  
> -
> -static struct drm_encoder *ast_best_single_encoder(struct drm_connector 
> *connector)
> -{
> - int enc_id = connector->encoder_ids[0];
> - /* pick the encoder ids */
> - if (enc_id)
> - return drm_encoder_find(connector->dev, NULL, enc_id);
> - return NULL;
> -}
> -
> -
>  static const struct drm_encoder_funcs ast_enc_funcs = {
>   .destroy = ast_encoder_destroy,
>  };
> @@ -847,7 +836,6 @@ static void ast_connector_destroy(struct drm_connector 
> *connector)
>  static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
>   .mode_valid = ast_mode_valid,
>   .get_modes = ast_get_modes,
> - .best_encoder = ast_best_single_encoder,
>  };
>  
>  static const struct drm_connector_funcs ast_connector_funcs = {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 4706439fb490..9d7e4da6c292 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
> *state,
>   }
>  }
>  
> -/*
> - * For connectors that support multiple encoders, either the
> - * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> - */
> -static struct drm_encoder *
> -pick_single_encoder_for_connector(struct drm_connector *connector)
> -{
> - WARN_ON(connector->encoder_ids[1]);
> - return drm_encoder_find(connector->dev, NULL, 
> connector->encoder_ids[0]);
> -}
> -
>  static int handle_conflicting_encoders(struct drm_atomic_state *state,
>  bool disable_conflicting_encoders)
>  {
> @@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct 
> drm_atomic_state *state,
>   else if (funcs->best_encoder)
>   new_encoder = funcs->best_encoder(connector);
>   else
> - new_encoder = 
> pick_single_encoder_for_connector(connector);
> + new_encoder = 
> drm_connector_get_single_encoder(connector);
>  
>   if (new_encoder) {
>   if (encoder_mask & drm_encoder_mask(new_encoder)) {
> @@ -359,7 +348,7 @@ update_connector_routing(struct drm_atomic_state *state,
>   else if (funcs->best_encoder)
>   new_encoder = funcs->best_encoder(connector);
>   else
> - new_encoder = pick_single_encoder_for_connector(connector);
> + new_encoder = drm_connector_get_single_encoder(connector);
>  
>   if (!new_encoder) {
>   DRM_DEBUG_ATOMIC("No suitable encoder found for 
> [CONNECTOR:%d:%s]\n",
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
> b/drivers/gpu/drm/drm_crtc_helper.c
> index a51824a7e7c1..4a7447a53cea 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -460,6 +460,17 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
>   __drm_helper_disable_unused_functions(dev);
>  }
>  
> +/*
> + * For connectors that support multiple encoders, either the
> + * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> + */
> +struct drm_encoder *
> +drm_connector_get_single_encoder(struct drm_connector *connector)
> +{
> + WARN_ON(connector->encoder_ids[1]);
> + return drm_encoder_find(connector->dev, NULL, 
> 

Re: [Intel-gfx] [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

2019-09-13 Thread Ville Syrjälä
On Thu, Sep 12, 2019 at 12:51:31PM -0700, José Roberto de Souza wrote:
> This 3 non-atomic drivers all have the same function getting the
> only encoder available in the connector, also atomic drivers have
> this fallback. So moving it a common place and sharing between atomic
> and non-atomic drivers.
> 
> While at it I also removed the mention of
> drm_atomic_helper_best_encoder() that was renamed in
> commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
> drm_atomic_helper_best_encoder").
> 
> v3: moving drm_connector_get_single_encoder to drm_kms_helper module
> 
> Suggested-by: Ville Syrjälä 
> Cc: Ville Syrjälä 
> Cc: Daniel Vetter 
> Cc: Laurent Pinchart 
> Cc: dri-de...@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: José Roberto de Souza 

lgtm
Reviewed-by: Ville Syrjälä 

> ---
>  drivers/gpu/drm/ast/ast_mode.c | 12 
>  drivers/gpu/drm/drm_atomic_helper.c| 15 ++-
>  drivers/gpu/drm/drm_crtc_helper.c  | 17 -
>  drivers/gpu/drm/drm_crtc_helper_internal.h |  3 +++
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 11 ---
>  drivers/gpu/drm/udl/udl_connector.c|  8 
>  include/drm/drm_modeset_helper_vtables.h   |  6 +++---
>  7 files changed, 24 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index d349c721501c..eef95e1af06b 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct drm_encoder 
> *encoder)
>   kfree(encoder);
>  }
>  
> -
> -static struct drm_encoder *ast_best_single_encoder(struct drm_connector 
> *connector)
> -{
> - int enc_id = connector->encoder_ids[0];
> - /* pick the encoder ids */
> - if (enc_id)
> - return drm_encoder_find(connector->dev, NULL, enc_id);
> - return NULL;
> -}
> -
> -
>  static const struct drm_encoder_funcs ast_enc_funcs = {
>   .destroy = ast_encoder_destroy,
>  };
> @@ -847,7 +836,6 @@ static void ast_connector_destroy(struct drm_connector 
> *connector)
>  static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
>   .mode_valid = ast_mode_valid,
>   .get_modes = ast_get_modes,
> - .best_encoder = ast_best_single_encoder,
>  };
>  
>  static const struct drm_connector_funcs ast_connector_funcs = {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 4706439fb490..9d7e4da6c292 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
> *state,
>   }
>  }
>  
> -/*
> - * For connectors that support multiple encoders, either the
> - * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> - */
> -static struct drm_encoder *
> -pick_single_encoder_for_connector(struct drm_connector *connector)
> -{
> - WARN_ON(connector->encoder_ids[1]);
> - return drm_encoder_find(connector->dev, NULL, 
> connector->encoder_ids[0]);
> -}
> -
>  static int handle_conflicting_encoders(struct drm_atomic_state *state,
>  bool disable_conflicting_encoders)
>  {
> @@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct 
> drm_atomic_state *state,
>   else if (funcs->best_encoder)
>   new_encoder = funcs->best_encoder(connector);
>   else
> - new_encoder = 
> pick_single_encoder_for_connector(connector);
> + new_encoder = 
> drm_connector_get_single_encoder(connector);
>  
>   if (new_encoder) {
>   if (encoder_mask & drm_encoder_mask(new_encoder)) {
> @@ -359,7 +348,7 @@ update_connector_routing(struct drm_atomic_state *state,
>   else if (funcs->best_encoder)
>   new_encoder = funcs->best_encoder(connector);
>   else
> - new_encoder = pick_single_encoder_for_connector(connector);
> + new_encoder = drm_connector_get_single_encoder(connector);
>  
>   if (!new_encoder) {
>   DRM_DEBUG_ATOMIC("No suitable encoder found for 
> [CONNECTOR:%d:%s]\n",
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
> b/drivers/gpu/drm/drm_crtc_helper.c
> index a51824a7e7c1..4a7447a53cea 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -460,6 +460,17 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
>   __drm_helper_disable_unused_functions(dev);
>  }
>  
> +/*
> + * For connectors that support multiple encoders, either the
> + * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> + */
> +struct drm_encoder *
> +drm_connector_get_single_encoder(struct drm_connector *connector)
> +{
> + WARN_ON(connector->encoder_ids[1]);
> + return drm_encoder_find(connector->dev, NULL, 
> 

[Intel-gfx] [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

2019-09-12 Thread José Roberto de Souza
This 3 non-atomic drivers all have the same function getting the
only encoder available in the connector, also atomic drivers have
this fallback. So moving it a common place and sharing between atomic
and non-atomic drivers.

While at it I also removed the mention of
drm_atomic_helper_best_encoder() that was renamed in
commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
drm_atomic_helper_best_encoder").

v3: moving drm_connector_get_single_encoder to drm_kms_helper module

Suggested-by: Ville Syrjälä 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Laurent Pinchart 
Cc: dri-de...@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/ast/ast_mode.c | 12 
 drivers/gpu/drm/drm_atomic_helper.c| 15 ++-
 drivers/gpu/drm/drm_crtc_helper.c  | 17 -
 drivers/gpu/drm/drm_crtc_helper_internal.h |  3 +++
 drivers/gpu/drm/mgag200/mgag200_mode.c | 11 ---
 drivers/gpu/drm/udl/udl_connector.c|  8 
 include/drm/drm_modeset_helper_vtables.h   |  6 +++---
 7 files changed, 24 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index d349c721501c..eef95e1af06b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct drm_encoder 
*encoder)
kfree(encoder);
 }
 
-
-static struct drm_encoder *ast_best_single_encoder(struct drm_connector 
*connector)
-{
-   int enc_id = connector->encoder_ids[0];
-   /* pick the encoder ids */
-   if (enc_id)
-   return drm_encoder_find(connector->dev, NULL, enc_id);
-   return NULL;
-}
-
-
 static const struct drm_encoder_funcs ast_enc_funcs = {
.destroy = ast_encoder_destroy,
 };
@@ -847,7 +836,6 @@ static void ast_connector_destroy(struct drm_connector 
*connector)
 static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
.mode_valid = ast_mode_valid,
.get_modes = ast_get_modes,
-   .best_encoder = ast_best_single_encoder,
 };
 
 static const struct drm_connector_funcs ast_connector_funcs = {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 4706439fb490..9d7e4da6c292 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
*state,
}
 }
 
-/*
- * For connectors that support multiple encoders, either the
- * .atomic_best_encoder() or .best_encoder() operation must be implemented.
- */
-static struct drm_encoder *
-pick_single_encoder_for_connector(struct drm_connector *connector)
-{
-   WARN_ON(connector->encoder_ids[1]);
-   return drm_encoder_find(connector->dev, NULL, 
connector->encoder_ids[0]);
-}
-
 static int handle_conflicting_encoders(struct drm_atomic_state *state,
   bool disable_conflicting_encoders)
 {
@@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct 
drm_atomic_state *state,
else if (funcs->best_encoder)
new_encoder = funcs->best_encoder(connector);
else
-   new_encoder = 
pick_single_encoder_for_connector(connector);
+   new_encoder = 
drm_connector_get_single_encoder(connector);
 
if (new_encoder) {
if (encoder_mask & drm_encoder_mask(new_encoder)) {
@@ -359,7 +348,7 @@ update_connector_routing(struct drm_atomic_state *state,
else if (funcs->best_encoder)
new_encoder = funcs->best_encoder(connector);
else
-   new_encoder = pick_single_encoder_for_connector(connector);
+   new_encoder = drm_connector_get_single_encoder(connector);
 
if (!new_encoder) {
DRM_DEBUG_ATOMIC("No suitable encoder found for 
[CONNECTOR:%d:%s]\n",
diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
b/drivers/gpu/drm/drm_crtc_helper.c
index a51824a7e7c1..4a7447a53cea 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -460,6 +460,17 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
__drm_helper_disable_unused_functions(dev);
 }
 
+/*
+ * For connectors that support multiple encoders, either the
+ * .atomic_best_encoder() or .best_encoder() operation must be implemented.
+ */
+struct drm_encoder *
+drm_connector_get_single_encoder(struct drm_connector *connector)
+{
+   WARN_ON(connector->encoder_ids[1]);
+   return drm_encoder_find(connector->dev, NULL, 
connector->encoder_ids[0]);
+}
+
 /**
  * drm_crtc_helper_set_config - set a new config from userspace
  * @set: mode set configuration
@@ -625,7 +636,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
new_encoder = connector->encoder;
for (ro = 0; ro < 

Re: [Intel-gfx] [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

2019-09-12 Thread kbuild test robot
Hi "Jos??,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc8 next-20190904]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Jos-Roberto-de-Souza/drm-connector-Share-with-non-atomic-drivers-the-function-to-get-the-single-encoder/20190912-213415
config: x86_64-randconfig-g003-201936 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 

All errors (new ones prefixed by >>):

>> ERROR: "drm_connector_get_single_encoder" 
>> [drivers/gpu/drm/drm_kms_helper.ko] undefined!

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

2019-09-11 Thread Souza, Jose
On Wed, 2019-09-11 at 21:10 +0300, Ville Syrjälä wrote:
> On Wed, Sep 11, 2019 at 10:56:02AM -0700, José Roberto de Souza
> wrote:
> > This 3 non-atomic drivers all have the same function getting the
> > only encoder available in the connector, also atomic drivers have
> > this fallback. So moving it a common place and sharing between
> > atomic
> > and non-atomic drivers.
> > 
> > While at it I also removed the mention of
> > drm_atomic_helper_best_encoder() that was renamed in
> > commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
> > drm_atomic_helper_best_encoder").
> > 
> > Suggested-by: Ville Syrjälä 
> > Cc: Ville Syrjälä 
> > Cc: Daniel Vetter 
> > Cc: Laurent Pinchart 
> > Cc: dri-de...@lists.freedesktop.org
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: José Roberto de Souza 
> > ---
> >  drivers/gpu/drm/ast/ast_mode.c   | 12 
> >  drivers/gpu/drm/drm_atomic_helper.c  | 15 ++-
> >  drivers/gpu/drm/drm_connector.c  | 11 +++
> >  drivers/gpu/drm/drm_crtc_helper.c|  8 +++-
> >  drivers/gpu/drm/drm_crtc_internal.h  |  2 ++
> >  drivers/gpu/drm/mgag200/mgag200_mode.c   | 11 ---
> >  drivers/gpu/drm/udl/udl_connector.c  |  8 
> >  include/drm/drm_modeset_helper_vtables.h |  6 +++---
> >  8 files changed, 25 insertions(+), 48 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ast/ast_mode.c
> > b/drivers/gpu/drm/ast/ast_mode.c
> > index d349c721501c..eef95e1af06b 100644
> > --- a/drivers/gpu/drm/ast/ast_mode.c
> > +++ b/drivers/gpu/drm/ast/ast_mode.c
> > @@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct
> > drm_encoder *encoder)
> > kfree(encoder);
> >  }
> >  
> > -
> > -static struct drm_encoder *ast_best_single_encoder(struct
> > drm_connector *connector)
> > -{
> > -   int enc_id = connector->encoder_ids[0];
> > -   /* pick the encoder ids */
> > -   if (enc_id)
> > -   return drm_encoder_find(connector->dev, NULL, enc_id);
> > -   return NULL;
> > -}
> > -
> > -
> >  static const struct drm_encoder_funcs ast_enc_funcs = {
> > .destroy = ast_encoder_destroy,
> >  };
> > @@ -847,7 +836,6 @@ static void ast_connector_destroy(struct
> > drm_connector *connector)
> >  static const struct drm_connector_helper_funcs
> > ast_connector_helper_funcs = {
> > .mode_valid = ast_mode_valid,
> > .get_modes = ast_get_modes,
> > -   .best_encoder = ast_best_single_encoder,
> >  };
> >  
> >  static const struct drm_connector_funcs ast_connector_funcs = {
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 4706439fb490..9d7e4da6c292 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct
> > drm_atomic_state *state,
> > }
> >  }
> >  
> > -/*
> > - * For connectors that support multiple encoders, either the
> > - * .atomic_best_encoder() or .best_encoder() operation must be
> > implemented.
> > - */
> > -static struct drm_encoder *
> > -pick_single_encoder_for_connector(struct drm_connector *connector)
> > -{
> > -   WARN_ON(connector->encoder_ids[1]);
> > -   return drm_encoder_find(connector->dev, NULL, connector-
> > >encoder_ids[0]);
> > -}
> > -
> >  static int handle_conflicting_encoders(struct drm_atomic_state
> > *state,
> >bool
> > disable_conflicting_encoders)
> >  {
> > @@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct
> > drm_atomic_state *state,
> > else if (funcs->best_encoder)
> > new_encoder = funcs->best_encoder(connector);
> > else
> > -   new_encoder =
> > pick_single_encoder_for_connector(connector);
> > +   new_encoder =
> > drm_connector_get_single_encoder(connector);
> >  
> > if (new_encoder) {
> > if (encoder_mask &
> > drm_encoder_mask(new_encoder)) {
> > @@ -359,7 +348,7 @@ update_connector_routing(struct
> > drm_atomic_state *state,
> > else if (funcs->best_encoder)
> > new_encoder = funcs->best_encoder(connector);
> > else
> > -   new_encoder =
> > pick_single_encoder_for_connector(connector);
> > +   new_encoder =
> > drm_connector_get_single_encoder(connector);
> >  
> > if (!new_encoder) {
> > DRM_DEBUG_ATOMIC("No suitable encoder found for
> > [CONNECTOR:%d:%s]\n",
> > diff --git a/drivers/gpu/drm/drm_connector.c
> > b/drivers/gpu/drm/drm_connector.c
> > index 4c766624b20d..3e2a632cf861 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -2334,3 +2334,14 @@ struct drm_tile_group
> > *drm_mode_create_tile_group(struct drm_device *dev,
> > return tg;
> >  }
> >  EXPORT_SYMBOL(drm_mode_create_tile_group);
> > +
> > +/*
> > + * For connectors that support multiple encoders, either the
> > + * .atomic_best_encoder() or .best_encoder() 

Re: [Intel-gfx] [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

2019-09-11 Thread Ville Syrjälä
On Wed, Sep 11, 2019 at 10:56:02AM -0700, José Roberto de Souza wrote:
> This 3 non-atomic drivers all have the same function getting the
> only encoder available in the connector, also atomic drivers have
> this fallback. So moving it a common place and sharing between atomic
> and non-atomic drivers.
> 
> While at it I also removed the mention of
> drm_atomic_helper_best_encoder() that was renamed in
> commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
> drm_atomic_helper_best_encoder").
> 
> Suggested-by: Ville Syrjälä 
> Cc: Ville Syrjälä 
> Cc: Daniel Vetter 
> Cc: Laurent Pinchart 
> Cc: dri-de...@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: José Roberto de Souza 
> ---
>  drivers/gpu/drm/ast/ast_mode.c   | 12 
>  drivers/gpu/drm/drm_atomic_helper.c  | 15 ++-
>  drivers/gpu/drm/drm_connector.c  | 11 +++
>  drivers/gpu/drm/drm_crtc_helper.c|  8 +++-
>  drivers/gpu/drm/drm_crtc_internal.h  |  2 ++
>  drivers/gpu/drm/mgag200/mgag200_mode.c   | 11 ---
>  drivers/gpu/drm/udl/udl_connector.c  |  8 
>  include/drm/drm_modeset_helper_vtables.h |  6 +++---
>  8 files changed, 25 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index d349c721501c..eef95e1af06b 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct drm_encoder 
> *encoder)
>   kfree(encoder);
>  }
>  
> -
> -static struct drm_encoder *ast_best_single_encoder(struct drm_connector 
> *connector)
> -{
> - int enc_id = connector->encoder_ids[0];
> - /* pick the encoder ids */
> - if (enc_id)
> - return drm_encoder_find(connector->dev, NULL, enc_id);
> - return NULL;
> -}
> -
> -
>  static const struct drm_encoder_funcs ast_enc_funcs = {
>   .destroy = ast_encoder_destroy,
>  };
> @@ -847,7 +836,6 @@ static void ast_connector_destroy(struct drm_connector 
> *connector)
>  static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
>   .mode_valid = ast_mode_valid,
>   .get_modes = ast_get_modes,
> - .best_encoder = ast_best_single_encoder,
>  };
>  
>  static const struct drm_connector_funcs ast_connector_funcs = {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 4706439fb490..9d7e4da6c292 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
> *state,
>   }
>  }
>  
> -/*
> - * For connectors that support multiple encoders, either the
> - * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> - */
> -static struct drm_encoder *
> -pick_single_encoder_for_connector(struct drm_connector *connector)
> -{
> - WARN_ON(connector->encoder_ids[1]);
> - return drm_encoder_find(connector->dev, NULL, 
> connector->encoder_ids[0]);
> -}
> -
>  static int handle_conflicting_encoders(struct drm_atomic_state *state,
>  bool disable_conflicting_encoders)
>  {
> @@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct 
> drm_atomic_state *state,
>   else if (funcs->best_encoder)
>   new_encoder = funcs->best_encoder(connector);
>   else
> - new_encoder = 
> pick_single_encoder_for_connector(connector);
> + new_encoder = 
> drm_connector_get_single_encoder(connector);
>  
>   if (new_encoder) {
>   if (encoder_mask & drm_encoder_mask(new_encoder)) {
> @@ -359,7 +348,7 @@ update_connector_routing(struct drm_atomic_state *state,
>   else if (funcs->best_encoder)
>   new_encoder = funcs->best_encoder(connector);
>   else
> - new_encoder = pick_single_encoder_for_connector(connector);
> + new_encoder = drm_connector_get_single_encoder(connector);
>  
>   if (!new_encoder) {
>   DRM_DEBUG_ATOMIC("No suitable encoder found for 
> [CONNECTOR:%d:%s]\n",
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..3e2a632cf861 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2334,3 +2334,14 @@ struct drm_tile_group 
> *drm_mode_create_tile_group(struct drm_device *dev,
>   return tg;
>  }
>  EXPORT_SYMBOL(drm_mode_create_tile_group);
> +
> +/*
> + * For connectors that support multiple encoders, either the
> + * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> + */
> +struct drm_encoder *
> +drm_connector_get_single_encoder(struct drm_connector *connector)
> +{
> + WARN_ON(connector->encoder_ids[1]);
> + return drm_encoder_find(connector->dev, NULL, 
> connector->encoder_ids[0]);
> +}

I believe we 

Re: [Intel-gfx] [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

2019-09-11 Thread Ville Syrjälä
On Wed, Sep 11, 2019 at 08:01:55PM +, Souza, Jose wrote:
> On Wed, 2019-09-11 at 21:10 +0300, Ville Syrjälä wrote:
> > On Wed, Sep 11, 2019 at 10:56:02AM -0700, José Roberto de Souza
> > wrote:
> > > This 3 non-atomic drivers all have the same function getting the
> > > only encoder available in the connector, also atomic drivers have
> > > this fallback. So moving it a common place and sharing between
> > > atomic
> > > and non-atomic drivers.
> > > 
> > > While at it I also removed the mention of
> > > drm_atomic_helper_best_encoder() that was renamed in
> > > commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
> > > drm_atomic_helper_best_encoder").
> > > 
> > > Suggested-by: Ville Syrjälä 
> > > Cc: Ville Syrjälä 
> > > Cc: Daniel Vetter 
> > > Cc: Laurent Pinchart 
> > > Cc: dri-de...@lists.freedesktop.org
> > > Cc: intel-gfx@lists.freedesktop.org
> > > Signed-off-by: José Roberto de Souza 
> > > ---
> > >  drivers/gpu/drm/ast/ast_mode.c   | 12 
> > >  drivers/gpu/drm/drm_atomic_helper.c  | 15 ++-
> > >  drivers/gpu/drm/drm_connector.c  | 11 +++
> > >  drivers/gpu/drm/drm_crtc_helper.c|  8 +++-
> > >  drivers/gpu/drm/drm_crtc_internal.h  |  2 ++
> > >  drivers/gpu/drm/mgag200/mgag200_mode.c   | 11 ---
> > >  drivers/gpu/drm/udl/udl_connector.c  |  8 
> > >  include/drm/drm_modeset_helper_vtables.h |  6 +++---
> > >  8 files changed, 25 insertions(+), 48 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/ast/ast_mode.c
> > > b/drivers/gpu/drm/ast/ast_mode.c
> > > index d349c721501c..eef95e1af06b 100644
> > > --- a/drivers/gpu/drm/ast/ast_mode.c
> > > +++ b/drivers/gpu/drm/ast/ast_mode.c
> > > @@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct
> > > drm_encoder *encoder)
> > >   kfree(encoder);
> > >  }
> > >  
> > > -
> > > -static struct drm_encoder *ast_best_single_encoder(struct
> > > drm_connector *connector)
> > > -{
> > > - int enc_id = connector->encoder_ids[0];
> > > - /* pick the encoder ids */
> > > - if (enc_id)
> > > - return drm_encoder_find(connector->dev, NULL, enc_id);
> > > - return NULL;
> > > -}
> > > -
> > > -
> > >  static const struct drm_encoder_funcs ast_enc_funcs = {
> > >   .destroy = ast_encoder_destroy,
> > >  };
> > > @@ -847,7 +836,6 @@ static void ast_connector_destroy(struct
> > > drm_connector *connector)
> > >  static const struct drm_connector_helper_funcs
> > > ast_connector_helper_funcs = {
> > >   .mode_valid = ast_mode_valid,
> > >   .get_modes = ast_get_modes,
> > > - .best_encoder = ast_best_single_encoder,
> > >  };
> > >  
> > >  static const struct drm_connector_funcs ast_connector_funcs = {
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > index 4706439fb490..9d7e4da6c292 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct
> > > drm_atomic_state *state,
> > >   }
> > >  }
> > >  
> > > -/*
> > > - * For connectors that support multiple encoders, either the
> > > - * .atomic_best_encoder() or .best_encoder() operation must be
> > > implemented.
> > > - */
> > > -static struct drm_encoder *
> > > -pick_single_encoder_for_connector(struct drm_connector *connector)
> > > -{
> > > - WARN_ON(connector->encoder_ids[1]);
> > > - return drm_encoder_find(connector->dev, NULL, connector-
> > > >encoder_ids[0]);
> > > -}
> > > -
> > >  static int handle_conflicting_encoders(struct drm_atomic_state
> > > *state,
> > >  bool
> > > disable_conflicting_encoders)
> > >  {
> > > @@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct
> > > drm_atomic_state *state,
> > >   else if (funcs->best_encoder)
> > >   new_encoder = funcs->best_encoder(connector);
> > >   else
> > > - new_encoder =
> > > pick_single_encoder_for_connector(connector);
> > > + new_encoder =
> > > drm_connector_get_single_encoder(connector);
> > >  
> > >   if (new_encoder) {
> > >   if (encoder_mask &
> > > drm_encoder_mask(new_encoder)) {
> > > @@ -359,7 +348,7 @@ update_connector_routing(struct
> > > drm_atomic_state *state,
> > >   else if (funcs->best_encoder)
> > >   new_encoder = funcs->best_encoder(connector);
> > >   else
> > > - new_encoder =
> > > pick_single_encoder_for_connector(connector);
> > > + new_encoder =
> > > drm_connector_get_single_encoder(connector);
> > >  
> > >   if (!new_encoder) {
> > >   DRM_DEBUG_ATOMIC("No suitable encoder found for
> > > [CONNECTOR:%d:%s]\n",
> > > diff --git a/drivers/gpu/drm/drm_connector.c
> > > b/drivers/gpu/drm/drm_connector.c
> > > index 4c766624b20d..3e2a632cf861 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -2334,3 +2334,14 @@ struct drm_tile_group
> > > 

[Intel-gfx] [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

2019-09-11 Thread José Roberto de Souza
This 3 non-atomic drivers all have the same function getting the
only encoder available in the connector, also atomic drivers have
this fallback. So moving it a common place and sharing between atomic
and non-atomic drivers.

While at it I also removed the mention of
drm_atomic_helper_best_encoder() that was renamed in
commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
drm_atomic_helper_best_encoder").

Suggested-by: Ville Syrjälä 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Laurent Pinchart 
Cc: dri-de...@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/ast/ast_mode.c   | 12 
 drivers/gpu/drm/drm_atomic_helper.c  | 15 ++-
 drivers/gpu/drm/drm_connector.c  | 11 +++
 drivers/gpu/drm/drm_crtc_helper.c|  8 +++-
 drivers/gpu/drm/drm_crtc_internal.h  |  2 ++
 drivers/gpu/drm/mgag200/mgag200_mode.c   | 11 ---
 drivers/gpu/drm/udl/udl_connector.c  |  8 
 include/drm/drm_modeset_helper_vtables.h |  6 +++---
 8 files changed, 25 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index d349c721501c..eef95e1af06b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct drm_encoder 
*encoder)
kfree(encoder);
 }
 
-
-static struct drm_encoder *ast_best_single_encoder(struct drm_connector 
*connector)
-{
-   int enc_id = connector->encoder_ids[0];
-   /* pick the encoder ids */
-   if (enc_id)
-   return drm_encoder_find(connector->dev, NULL, enc_id);
-   return NULL;
-}
-
-
 static const struct drm_encoder_funcs ast_enc_funcs = {
.destroy = ast_encoder_destroy,
 };
@@ -847,7 +836,6 @@ static void ast_connector_destroy(struct drm_connector 
*connector)
 static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
.mode_valid = ast_mode_valid,
.get_modes = ast_get_modes,
-   .best_encoder = ast_best_single_encoder,
 };
 
 static const struct drm_connector_funcs ast_connector_funcs = {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 4706439fb490..9d7e4da6c292 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
*state,
}
 }
 
-/*
- * For connectors that support multiple encoders, either the
- * .atomic_best_encoder() or .best_encoder() operation must be implemented.
- */
-static struct drm_encoder *
-pick_single_encoder_for_connector(struct drm_connector *connector)
-{
-   WARN_ON(connector->encoder_ids[1]);
-   return drm_encoder_find(connector->dev, NULL, 
connector->encoder_ids[0]);
-}
-
 static int handle_conflicting_encoders(struct drm_atomic_state *state,
   bool disable_conflicting_encoders)
 {
@@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct 
drm_atomic_state *state,
else if (funcs->best_encoder)
new_encoder = funcs->best_encoder(connector);
else
-   new_encoder = 
pick_single_encoder_for_connector(connector);
+   new_encoder = 
drm_connector_get_single_encoder(connector);
 
if (new_encoder) {
if (encoder_mask & drm_encoder_mask(new_encoder)) {
@@ -359,7 +348,7 @@ update_connector_routing(struct drm_atomic_state *state,
else if (funcs->best_encoder)
new_encoder = funcs->best_encoder(connector);
else
-   new_encoder = pick_single_encoder_for_connector(connector);
+   new_encoder = drm_connector_get_single_encoder(connector);
 
if (!new_encoder) {
DRM_DEBUG_ATOMIC("No suitable encoder found for 
[CONNECTOR:%d:%s]\n",
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4c766624b20d..3e2a632cf861 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2334,3 +2334,14 @@ struct drm_tile_group *drm_mode_create_tile_group(struct 
drm_device *dev,
return tg;
 }
 EXPORT_SYMBOL(drm_mode_create_tile_group);
+
+/*
+ * For connectors that support multiple encoders, either the
+ * .atomic_best_encoder() or .best_encoder() operation must be implemented.
+ */
+struct drm_encoder *
+drm_connector_get_single_encoder(struct drm_connector *connector)
+{
+   WARN_ON(connector->encoder_ids[1]);
+   return drm_encoder_find(connector->dev, NULL, 
connector->encoder_ids[0]);
+}
diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
b/drivers/gpu/drm/drm_crtc_helper.c
index a51824a7e7c1..a1f3c388e398 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -48,6 +48,8 @@
 #include 
 #include 
 
+#include "drm_crtc_internal.h"
+