Re: [PATCH 02/18] drm/atomic-helper: Unexport drm_atomic_helper_best_encoder
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/18] drm/atomic-helper: Unexport drm_atomic_helper_best_encoder
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/18] drm/atomic-helper: Unexport drm_atomic_helper_best_encoder
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
