Re: [PATCH RESEND] drm/atomic-helper: rename drm_atomic_helper_check_wb_encoder_state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 5 Dec 2023 at 13:48, Maxime Ripard <mripard@xxxxxxxxxx> wrote:
>
> On Tue, Dec 05, 2023 at 03:37:15AM +0200, Dmitry Baryshkov wrote:
> > On 04/12/2023 10:38, Maxime Ripard wrote:
> > > On Sat, Dec 02, 2023 at 12:07:49AM +0200, Dmitry Baryshkov wrote:
> > > > The drm_atomic_helper_check_wb_encoder_state() function doesn't use
> > > > encoder for anything other than getting the drm_device instance. The
> > > > function's description talks about checking the writeback connector
> > > > state, not the encoder state. Moreover, there is no such thing as an
> > > > encoder state, encoders generally do not have a state on their own.
> > > >
> > > > Drop the first argument and rename the function to
> > > > drm_atomic_helper_check_wb_connector_state().
> > > >
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > > > ---
> > > >
> > > > Resending, no reaction for two months
> > > >
> > > > ---
> > > >   drivers/gpu/drm/drm_atomic_helper.c   | 10 ++++------
> > > >   drivers/gpu/drm/vkms/vkms_writeback.c |  2 +-
> > > >   include/drm/drm_atomic_helper.h       |  3 +--
> > > >   3 files changed, 6 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > > index 2444fc33dd7c..d69591381f00 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > @@ -795,8 +795,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> > > >   EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
> > > >   /**
> > > > - * drm_atomic_helper_check_wb_encoder_state() - Check writeback encoder state
> > > > - * @encoder: encoder state to check
> > > > + * drm_atomic_helper_check_wb_connector_state() - Check writeback connector state
> > > >    * @conn_state: connector state to check
> > > >    *
> > > >    * Checks if the writeback connector state is valid, and returns an error if it
> > > > @@ -806,8 +805,7 @@ EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
> > > >    * Zero for success or -errno
> > > >    */
> > > >   int
> > > > -drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
> > > > -                                  struct drm_connector_state *conn_state)
> > > > +drm_atomic_helper_check_wb_connector_state(struct drm_connector_state *conn_state)
> > >
> > > AFAIK, all the helpers take the object as first argument, so I'm fine
> > > with the name change but it should take a drm_connector too. And ideally
> > > a drm_atomic_state pointer instead of drm_connector_state too.
> >
> > I think we then might take even further step and pass
> > drm_writeback_connector to this function. I'll send this as a part of v2.
>
> ... Which is still not the usual function prototype for atomic_check
> helpers.

On one hand, you are correct. On the other hand, don't we want to
emphasise that this function is to be called only for
drm_writeback_connector objects?




-- 
With best wishes
Dmitry



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux