Re: [Intel-gfx] [PATCH 02/18] drm/atomic-helper: Unexport drm_atomic_helper_best_encoder

2018-10-04 Thread Daniel Vetter
On Thu, Oct 04, 2018 at 02:33:24PM -0400, Sean Paul wrote:
> On Tue, Oct 02, 2018 at 03:35:10PM +0200, Daniel Vetter wrote:
> > It's the default. The exported version was kinda a transition state,
> > before we made this the default.
> > 
> > To stop new atomic drivers from using it (instead of just relying on
> > the default) let's unexport it.
> > 
> > Signed-off-by: Daniel Vetter 
> > Cc: Gustavo Padovan 
> > Cc: Maarten Lankhorst 
> > Cc: Sean Paul 
> > Cc: David Airlie 
> > Cc: VMware Graphics 
> > Cc: Sinclair Yeh 
> > Cc: Thomas Hellstrom 
> > Cc: Archit Taneja 
> > Cc: Neil Armstrong 
> > Cc: Laurent Pinchart 
> > Cc: Hans Verkuil 
> > Cc: Daniel Vetter 
> > Cc: Russell King 
> > Cc: Jernej Skrabec 
> > Cc: Jani Nikula 
> > Cc: Pierre-Hugues Husson 
> > Cc: Fabio Estevam 
> > ---
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  1 -
> >  drivers/gpu/drm/drm_atomic_helper.c   | 24 +++
> >  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c   |  1 -
> >  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c  |  1 -
> >  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c  |  1 -
> >  include/drm/drm_atomic_helper.h   |  2 --
> >  6 files changed, 7 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > index ac37c50d6c4b..5ac979d3450b 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -1957,7 +1957,6 @@ static const struct drm_connector_funcs 
> > dw_hdmi_connector_funcs = {
> >  
> >  static const struct drm_connector_helper_funcs 
> > dw_hdmi_connector_helper_funcs = {
> > .get_modes = dw_hdmi_connector_get_modes,
> > -   .best_encoder = drm_atomic_helper_best_encoder,
> >  };
> >  
> >  static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index f92b7cf4cbd7..8c93f33fe92f 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -92,6 +92,13 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
> > *state,
> > }
> >  }
> >  
> > +static struct drm_encoder *
> > +drm_atomic_helper_best_encoder(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)
> >  {
> > @@ -3376,23 +3383,6 @@ int drm_atomic_helper_page_flip_target(struct 
> > drm_crtc *crtc,
> >  }
> >  EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
> >  
> > -/**
> > - * drm_atomic_helper_best_encoder - Helper for
> > - * _connector_helper_funcs.best_encoder callback
> > - * @connector: Connector control structure
> > - *
> > - * This is a _connector_helper_funcs.best_encoder callback helper for
> > - * connectors that support exactly 1 encoder, statically determined at 
> > driver
> > - * init time.
> > - */
> > -struct drm_encoder *
> > -drm_atomic_helper_best_encoder(struct drm_connector *connector)
> > -{
> > -   WARN_ON(connector->encoder_ids[1]);
> > -   return drm_encoder_find(connector->dev, NULL, 
> > connector->encoder_ids[0]);
> > -}
> > -EXPORT_SYMBOL(drm_atomic_helper_best_encoder);
> > -
> >  /**
> >   * DOC: atomic state reset and initialization
> >   *
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c 
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> > index 723578117191..4b5378495eea 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> > @@ -274,7 +274,6 @@ static const struct drm_connector_funcs 
> > vmw_legacy_connector_funcs = {
> >  
> >  static const struct
> >  drm_connector_helper_funcs vmw_ldu_connector_helper_funcs = {
> > -   .best_encoder = drm_atomic_helper_best_encoder,
> >  };
> 
> Seems like you can remove this entirely, as well as the helper funcs
> registration call? Same goes for a few other drivers.

Needs a huge audit, at least in the past we've had cases where everything
started oopsing because helpers didn't check carefully whether the vtable
pointer was NULL or not.

Heck even this patch here blew up on amdgpu at first.

So good idea, but maybe not in this patch here :-)
-Daniel

> >  /*
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c 
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > index ad0de7f0cd60..4c68ad6f3605 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> > @@ -389,7 +389,6 @@ static const struct drm_connector_funcs 
> > vmw_sou_connector_funcs = {
> >  
> >  static const struct
> >  drm_connector_helper_funcs vmw_sou_connector_helper_funcs = {
> > -   .best_encoder = drm_atomic_helper_best_encoder,
> >  };
> >  
> >  
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c 
> > 

Re: [Intel-gfx] [PATCH 02/18] drm/atomic-helper: Unexport drm_atomic_helper_best_encoder

2018-10-04 Thread Sean Paul
On Tue, Oct 02, 2018 at 03:35:10PM +0200, Daniel Vetter wrote:
> It's the default. The exported version was kinda a transition state,
> before we made this the default.
> 
> To stop new atomic drivers from using it (instead of just relying on
> the default) let's unexport it.
> 
> Signed-off-by: Daniel Vetter 
> Cc: Gustavo Padovan 
> Cc: Maarten Lankhorst 
> Cc: Sean Paul 
> Cc: David Airlie 
> Cc: VMware Graphics 
> Cc: Sinclair Yeh 
> Cc: Thomas Hellstrom 
> Cc: Archit Taneja 
> Cc: Neil Armstrong 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Daniel Vetter 
> Cc: Russell King 
> Cc: Jernej Skrabec 
> Cc: Jani Nikula 
> Cc: Pierre-Hugues Husson 
> Cc: Fabio Estevam 
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  1 -
>  drivers/gpu/drm/drm_atomic_helper.c   | 24 +++
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c   |  1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c  |  1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c  |  1 -
>  include/drm/drm_atomic_helper.h   |  2 --
>  6 files changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index ac37c50d6c4b..5ac979d3450b 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1957,7 +1957,6 @@ static const struct drm_connector_funcs 
> dw_hdmi_connector_funcs = {
>  
>  static const struct drm_connector_helper_funcs 
> dw_hdmi_connector_helper_funcs = {
>   .get_modes = dw_hdmi_connector_get_modes,
> - .best_encoder = drm_atomic_helper_best_encoder,
>  };
>  
>  static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index f92b7cf4cbd7..8c93f33fe92f 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -92,6 +92,13 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
> *state,
>   }
>  }
>  
> +static struct drm_encoder *
> +drm_atomic_helper_best_encoder(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)
>  {
> @@ -3376,23 +3383,6 @@ int drm_atomic_helper_page_flip_target(struct drm_crtc 
> *crtc,
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
>  
> -/**
> - * drm_atomic_helper_best_encoder - Helper for
> - *   _connector_helper_funcs.best_encoder callback
> - * @connector: Connector control structure
> - *
> - * This is a _connector_helper_funcs.best_encoder callback helper for
> - * connectors that support exactly 1 encoder, statically determined at driver
> - * init time.
> - */
> -struct drm_encoder *
> -drm_atomic_helper_best_encoder(struct drm_connector *connector)
> -{
> - WARN_ON(connector->encoder_ids[1]);
> - return drm_encoder_find(connector->dev, NULL, 
> connector->encoder_ids[0]);
> -}
> -EXPORT_SYMBOL(drm_atomic_helper_best_encoder);
> -
>  /**
>   * DOC: atomic state reset and initialization
>   *
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> index 723578117191..4b5378495eea 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> @@ -274,7 +274,6 @@ static const struct drm_connector_funcs 
> vmw_legacy_connector_funcs = {
>  
>  static const struct
>  drm_connector_helper_funcs vmw_ldu_connector_helper_funcs = {
> - .best_encoder = drm_atomic_helper_best_encoder,
>  };

Seems like you can remove this entirely, as well as the helper funcs
registration call? Same goes for a few other drivers.

Sean

>  
>  /*
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index ad0de7f0cd60..4c68ad6f3605 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -389,7 +389,6 @@ static const struct drm_connector_funcs 
> vmw_sou_connector_funcs = {
>  
>  static const struct
>  drm_connector_helper_funcs vmw_sou_connector_helper_funcs = {
> - .best_encoder = drm_atomic_helper_best_encoder,
>  };
>  
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index f30e839f7bfd..e28bb08114a5 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -1037,7 +1037,6 @@ static const struct drm_connector_funcs 
> vmw_stdu_connector_funcs = {
>  
>  static const struct
>  drm_connector_helper_funcs vmw_stdu_connector_helper_funcs = {
> - .best_encoder = drm_atomic_helper_best_encoder,
>  };
>  
>  
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 657af7b39379..e60c4f0f8827 100644
> --- a/include/drm/drm_atomic_helper.h
> 

Re: [Intel-gfx] [PATCH 02/18] drm/atomic-helper: Unexport drm_atomic_helper_best_encoder

2018-10-04 Thread Laurent Pinchart
Hi Daniel,

On Wednesday, 3 October 2018 12:08:38 EEST Daniel Vetter wrote:
> On Tue, Oct 02, 2018 at 04:53:12PM +0300, Laurent Pinchart wrote:
> > On Tuesday, 2 October 2018 16:35:10 EEST Daniel Vetter wrote:
> > > It's the default. The exported version was kinda a transition state,
> > > before we made this the default.
> > > 
> > > To stop new atomic drivers from using it (instead of just relying on
> > > the default) let's unexport it.
> > > 
> > > Signed-off-by: Daniel Vetter 
> > > Cc: Gustavo Padovan 
> > > Cc: Maarten Lankhorst 
> > > Cc: Sean Paul 
> > > Cc: David Airlie 
> > > Cc: VMware Graphics 
> > > Cc: Sinclair Yeh 
> > > Cc: Thomas Hellstrom 
> > > Cc: Archit Taneja 
> > > Cc: Neil Armstrong 
> > > Cc: Laurent Pinchart 
> > > Cc: Hans Verkuil 
> > > Cc: Daniel Vetter 
> > > Cc: Russell King 
> > > Cc: Jernej Skrabec 
> > > Cc: Jani Nikula 
> > > Cc: Pierre-Hugues Husson 
> > > Cc: Fabio Estevam 
> > > ---
> > > 
> > >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  1 -
> > >  drivers/gpu/drm/drm_atomic_helper.c   | 24 +++
> > >  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c   |  1 -
> > >  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c  |  1 -
> > >  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c  |  1 -
> > >  include/drm/drm_atomic_helper.h   |  2 --
> > >  6 files changed, 7 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > > ac37c50d6c4b..5ac979d3450b 100644
> > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > 
> > [snip]
> > 
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > > b/drivers/gpu/drm/drm_atomic_helper.c index f92b7cf4cbd7..8c93f33fe92f
> > > 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -92,6 +92,13 @@ drm_atomic_helper_plane_changed(struct
> > > drm_atomic_state
> > > *state, }
> > > 
> > >  }
> > > 
> > > +static struct drm_encoder *
> > > +drm_atomic_helper_best_encoder(struct drm_connector *connector)
> > > +{
> > > + WARN_ON(connector->encoder_ids[1]);
> > 
> > As you're removing the documentation, I would add a comment here to
> > explain the WARN_ON. Something along the lines of "For connectors that
> > support multiple encoders, the .atomic_best_encoder() or .atomic_encoder()
> > operation must be implemented".
> > 
> > You could also rename the function to make it more explicit that it's a
> > default for the single encoder case.
> 
> So pick_single_encoder_for_connector()?

Works for me.

> I think that would avoid the need for a comment.

I'd still keep the comment, it won't hurt, and you have it already :-)

> r-b: you with that fixed up?

With the comment and function renamed,

Reviewed-by: Laurent Pinchart 

> > > + 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)
> > >  
> > >  {
> > > 
> > > @@ -3376,23 +3383,6 @@ int drm_atomic_helper_page_flip_target(struct
> > > drm_crtc *crtc, }
> > > 
> > >  EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
> > > 
> > > -/**
> > > - * drm_atomic_helper_best_encoder - Helper for
> > > - *   _connector_helper_funcs.best_encoder callback
> > > - * @connector: Connector control structure
> > > - *
> > > - * This is a _connector_helper_funcs.best_encoder callback helper
> > > for
> > > - * connectors that support exactly 1 encoder, statically determined at
> > > driver - * init time.
> > > - */
> > > -struct drm_encoder *
> > > -drm_atomic_helper_best_encoder(struct drm_connector *connector)
> > > -{
> > > - WARN_ON(connector->encoder_ids[1]);
> > > - return drm_encoder_find(connector->dev, NULL,
> > > connector->encoder_ids[0]);
> > > -}
> > > -EXPORT_SYMBOL(drm_atomic_helper_best_encoder);
> > > -
> > > 
> > >  /**
> > >  
> > >   * DOC: atomic state reset and initialization
> > >   *
> > 
> > [snip]


-- 
Regards,

Laurent Pinchart



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/18] drm/atomic-helper: Unexport drm_atomic_helper_best_encoder

2018-10-03 Thread Daniel Vetter
On Tue, Oct 02, 2018 at 04:53:12PM +0300, Laurent Pinchart wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Tuesday, 2 October 2018 16:35:10 EEST Daniel Vetter wrote:
> > It's the default. The exported version was kinda a transition state,
> > before we made this the default.
> > 
> > To stop new atomic drivers from using it (instead of just relying on
> > the default) let's unexport it.
> > 
> > Signed-off-by: Daniel Vetter 
> > Cc: Gustavo Padovan 
> > Cc: Maarten Lankhorst 
> > Cc: Sean Paul 
> > Cc: David Airlie 
> > Cc: VMware Graphics 
> > Cc: Sinclair Yeh 
> > Cc: Thomas Hellstrom 
> > Cc: Archit Taneja 
> > Cc: Neil Armstrong 
> > Cc: Laurent Pinchart 
> > Cc: Hans Verkuil 
> > Cc: Daniel Vetter 
> > Cc: Russell King 
> > Cc: Jernej Skrabec 
> > Cc: Jani Nikula 
> > Cc: Pierre-Hugues Husson 
> > Cc: Fabio Estevam 
> > ---
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  1 -
> >  drivers/gpu/drm/drm_atomic_helper.c   | 24 +++
> >  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c   |  1 -
> >  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c  |  1 -
> >  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c  |  1 -
> >  include/drm/drm_atomic_helper.h   |  2 --
> >  6 files changed, 7 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > ac37c50d6c4b..5ac979d3450b 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> 
> [snip]
> 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c index f92b7cf4cbd7..8c93f33fe92f
> > 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -92,6 +92,13 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> > *state, }
> >  }
> > 
> > +static struct drm_encoder *
> > +drm_atomic_helper_best_encoder(struct drm_connector *connector)
> > +{
> > +   WARN_ON(connector->encoder_ids[1]);
> 
> As you're removing the documentation, I would add a comment here to explain 
> the WARN_ON. Something along the lines of "For connectors that support 
> multiple encoders, the .atomic_best_encoder() or .atomic_encoder() operation 
> must be implemented".
> 
> You could also rename the function to make it more explicit that it's a 
> default for the single encoder case.

So pick_single_encoder_for_connector()? I think that would avoid the need
for a comment. r-b: you with that fixed up?

Thanks, Daniel

> 
> > +   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)
> >  {
> > @@ -3376,23 +3383,6 @@ int drm_atomic_helper_page_flip_target(struct
> > drm_crtc *crtc, }
> >  EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
> > 
> > -/**
> > - * drm_atomic_helper_best_encoder - Helper for
> > - * _connector_helper_funcs.best_encoder callback
> > - * @connector: Connector control structure
> > - *
> > - * This is a _connector_helper_funcs.best_encoder callback helper for
> > - * connectors that support exactly 1 encoder, statically determined at
> > driver - * init time.
> > - */
> > -struct drm_encoder *
> > -drm_atomic_helper_best_encoder(struct drm_connector *connector)
> > -{
> > -   WARN_ON(connector->encoder_ids[1]);
> > -   return drm_encoder_find(connector->dev, NULL, 
> > connector->encoder_ids[0]);
> > -}
> > -EXPORT_SYMBOL(drm_atomic_helper_best_encoder);
> > -
> >  /**
> >   * DOC: atomic state reset and initialization
> >   *
> 
> [snip]
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 02/18] drm/atomic-helper: Unexport drm_atomic_helper_best_encoder

2018-10-02 Thread Laurent Pinchart
Hi Daniel,

Thank you for the patch.

On Tuesday, 2 October 2018 16:35:10 EEST Daniel Vetter wrote:
> It's the default. The exported version was kinda a transition state,
> before we made this the default.
> 
> To stop new atomic drivers from using it (instead of just relying on
> the default) let's unexport it.
> 
> Signed-off-by: Daniel Vetter 
> Cc: Gustavo Padovan 
> Cc: Maarten Lankhorst 
> Cc: Sean Paul 
> Cc: David Airlie 
> Cc: VMware Graphics 
> Cc: Sinclair Yeh 
> Cc: Thomas Hellstrom 
> Cc: Archit Taneja 
> Cc: Neil Armstrong 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Daniel Vetter 
> Cc: Russell King 
> Cc: Jernej Skrabec 
> Cc: Jani Nikula 
> Cc: Pierre-Hugues Husson 
> Cc: Fabio Estevam 
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  1 -
>  drivers/gpu/drm/drm_atomic_helper.c   | 24 +++
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c   |  1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c  |  1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c  |  1 -
>  include/drm/drm_atomic_helper.h   |  2 --
>  6 files changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> ac37c50d6c4b..5ac979d3450b 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c

[snip]

> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index f92b7cf4cbd7..8c93f33fe92f
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -92,6 +92,13 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> *state, }
>  }
> 
> +static struct drm_encoder *
> +drm_atomic_helper_best_encoder(struct drm_connector *connector)
> +{
> + WARN_ON(connector->encoder_ids[1]);

As you're removing the documentation, I would add a comment here to explain 
the WARN_ON. Something along the lines of "For connectors that support 
multiple encoders, the .atomic_best_encoder() or .atomic_encoder() operation 
must be implemented".

You could also rename the function to make it more explicit that it's a 
default for the single encoder case.

> + 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)
>  {
> @@ -3376,23 +3383,6 @@ int drm_atomic_helper_page_flip_target(struct
> drm_crtc *crtc, }
>  EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
> 
> -/**
> - * drm_atomic_helper_best_encoder - Helper for
> - *   _connector_helper_funcs.best_encoder callback
> - * @connector: Connector control structure
> - *
> - * This is a _connector_helper_funcs.best_encoder callback helper for
> - * connectors that support exactly 1 encoder, statically determined at
> driver - * init time.
> - */
> -struct drm_encoder *
> -drm_atomic_helper_best_encoder(struct drm_connector *connector)
> -{
> - WARN_ON(connector->encoder_ids[1]);
> - return drm_encoder_find(connector->dev, NULL, 
> connector->encoder_ids[0]);
> -}
> -EXPORT_SYMBOL(drm_atomic_helper_best_encoder);
> -
>  /**
>   * DOC: atomic state reset and initialization
>   *

[snip]

-- 
Regards,

Laurent Pinchart



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 02/18] drm/atomic-helper: Unexport drm_atomic_helper_best_encoder

2018-10-02 Thread Daniel Vetter
It's the default. The exported version was kinda a transition state,
before we made this the default.

To stop new atomic drivers from using it (instead of just relying on
the default) let's unexport it.

Signed-off-by: Daniel Vetter 
Cc: Gustavo Padovan 
Cc: Maarten Lankhorst 
Cc: Sean Paul 
Cc: David Airlie 
Cc: VMware Graphics 
Cc: Sinclair Yeh 
Cc: Thomas Hellstrom 
Cc: Archit Taneja 
Cc: Neil Armstrong 
Cc: Laurent Pinchart 
Cc: Hans Verkuil 
Cc: Daniel Vetter 
Cc: Russell King 
Cc: Jernej Skrabec 
Cc: Jani Nikula 
Cc: Pierre-Hugues Husson 
Cc: Fabio Estevam 
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  1 -
 drivers/gpu/drm/drm_atomic_helper.c   | 24 +++
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c   |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c  |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c  |  1 -
 include/drm/drm_atomic_helper.h   |  2 --
 6 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ac37c50d6c4b..5ac979d3450b 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1957,7 +1957,6 @@ static const struct drm_connector_funcs 
dw_hdmi_connector_funcs = {
 
 static const struct drm_connector_helper_funcs dw_hdmi_connector_helper_funcs 
= {
.get_modes = dw_hdmi_connector_get_modes,
-   .best_encoder = drm_atomic_helper_best_encoder,
 };
 
 static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index f92b7cf4cbd7..8c93f33fe92f 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -92,6 +92,13 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
*state,
}
 }
 
+static struct drm_encoder *
+drm_atomic_helper_best_encoder(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)
 {
@@ -3376,23 +3383,6 @@ int drm_atomic_helper_page_flip_target(struct drm_crtc 
*crtc,
 }
 EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
 
-/**
- * drm_atomic_helper_best_encoder - Helper for
- * _connector_helper_funcs.best_encoder callback
- * @connector: Connector control structure
- *
- * This is a _connector_helper_funcs.best_encoder callback helper for
- * connectors that support exactly 1 encoder, statically determined at driver
- * init time.
- */
-struct drm_encoder *
-drm_atomic_helper_best_encoder(struct drm_connector *connector)
-{
-   WARN_ON(connector->encoder_ids[1]);
-   return drm_encoder_find(connector->dev, NULL, 
connector->encoder_ids[0]);
-}
-EXPORT_SYMBOL(drm_atomic_helper_best_encoder);
-
 /**
  * DOC: atomic state reset and initialization
  *
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
index 723578117191..4b5378495eea 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
@@ -274,7 +274,6 @@ static const struct drm_connector_funcs 
vmw_legacy_connector_funcs = {
 
 static const struct
 drm_connector_helper_funcs vmw_ldu_connector_helper_funcs = {
-   .best_encoder = drm_atomic_helper_best_encoder,
 };
 
 /*
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index ad0de7f0cd60..4c68ad6f3605 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -389,7 +389,6 @@ static const struct drm_connector_funcs 
vmw_sou_connector_funcs = {
 
 static const struct
 drm_connector_helper_funcs vmw_sou_connector_helper_funcs = {
-   .best_encoder = drm_atomic_helper_best_encoder,
 };
 
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index f30e839f7bfd..e28bb08114a5 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1037,7 +1037,6 @@ static const struct drm_connector_funcs 
vmw_stdu_connector_funcs = {
 
 static const struct
 drm_connector_helper_funcs vmw_stdu_connector_helper_funcs = {
-   .best_encoder = drm_atomic_helper_best_encoder,
 };
 
 
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 657af7b39379..e60c4f0f8827 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -144,8 +144,6 @@ int drm_atomic_helper_page_flip_target(
uint32_t flags,
uint32_t target,
struct drm_modeset_acquire_ctx *ctx);
-struct drm_encoder *
-drm_atomic_helper_best_encoder(struct drm_connector *connector);
 
 /* default implementations for state handling */
 void drm_atomic_helper_crtc_reset(struct