Re: [1/8] drm/plane-helper: Move drm_plane_helper_atomic_check() into udl

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Hi

Am 01.12.23 um 03:36 schrieb Sui Jingfeng:
> Hi,
>
>
> On 2023/11/28 18:45, Thomas Zimmermann wrote:
> > The udl driver is the only caller of drm_plane_helper_atomic_check().
> > Move the function into the driver. No functional changes.
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > ---
> >   drivers/gpu/drm/drm_plane_helper.c | 32 ------------------------------
> >   drivers/gpu/drm/udl/udl_modeset.c  | 19 ++++++++++++++++--
> >   include/drm/drm_plane_helper.h     |  2 --
> >   3 files changed, 17 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_plane_helper.c 
> > b/drivers/gpu/drm/drm_plane_helper.c
> > index 5e95089676ff8..7982be4b0306d 100644
> > --- a/drivers/gpu/drm/drm_plane_helper.c
> > +++ b/drivers/gpu/drm/drm_plane_helper.c
> > @@ -279,35 +279,3 @@ void drm_plane_helper_destroy(struct drm_plane 
> > *plane)
> >       kfree(plane);
> >   }
> >   EXPORT_SYMBOL(drm_plane_helper_destroy);
> > -
> > -/**
> > - * drm_plane_helper_atomic_check() - Helper to check plane atomic-state
> > - * @plane: plane to check
> > - * @state: atomic state object
> > - *
> > - * Provides a default plane-state check handler for planes whose 
> > atomic-state
> > - * scale and positioning are not expected to change since the plane 
> > is always
> > - * a fullscreen scanout buffer.
> > - *
> > - * This is often the case for the primary plane of simple 
> > framebuffers. See
> > - * also drm_crtc_helper_atomic_check() for the respective CRTC-state 
> > check
> > - * helper function.
> > - *
> > - * RETURNS:
> > - * Zero on success, or an errno code otherwise.
> > - */
> > -int drm_plane_helper_atomic_check(struct drm_plane *plane, struct 
> > drm_atomic_state *state)
> > -{
> > -    struct drm_plane_state *new_plane_state = 
> > drm_atomic_get_new_plane_state(state, plane);
> > -    struct drm_crtc *new_crtc = new_plane_state->crtc;
> > -    struct drm_crtc_state *new_crtc_state = NULL;
> > -
> > -    if (new_crtc)
> > -        new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
> > -
> > -    return drm_atomic_helper_check_plane_state(new_plane_state, 
> > new_crtc_state,
> > -                           DRM_PLANE_NO_SCALING,
> > -                           DRM_PLANE_NO_SCALING,
> > -                           false, false);
> > -}
> > -EXPORT_SYMBOL(drm_plane_helper_atomic_check);
>
>
> Since this function is removed, does the comments of the 
> drm_crtc_helper_atomic_check()
> function (in the drm_crtc_helper.c) need to update as well? I'm ask 
> because I see the
> comments of the drm_crtc_helper_atomic_check() still referencing this 
> function.

Good point. I'll update that comment. Thanks for reviewing.

Best regards
Thomas

> > diff --git a/drivers/gpu/drm/udl/udl_modeset.c 
> > b/drivers/gpu/drm/udl/udl_modeset.c
> > index 40876bcdd79a4..7702359c90c22 100644
> > --- a/drivers/gpu/drm/udl/udl_modeset.c
> > +++ b/drivers/gpu/drm/udl/udl_modeset.c
> > @@ -21,7 +21,6 @@
> >   #include <drm/drm_gem_framebuffer_helper.h>
> >   #include <drm/drm_gem_shmem_helper.h>
> >   #include <drm/drm_modeset_helper_vtables.h>
> > -#include <drm/drm_plane_helper.h>
> >   #include <drm/drm_probe_helper.h>
> >   #include <drm/drm_vblank.h>
> > @@ -261,6 +260,22 @@ static const uint64_t udl_primary_plane_fmtmods[] 
> > = {
> >       DRM_FORMAT_MOD_INVALID
> >   };
> > +static int udl_primary_plane_helper_atomic_check(struct drm_plane 
> > *plane,
> > +                         struct drm_atomic_state *state)
> > +{
> > +    struct drm_plane_state *new_plane_state = 
> > drm_atomic_get_new_plane_state(state, plane);
> > +    struct drm_crtc *new_crtc = new_plane_state->crtc;
> > +    struct drm_crtc_state *new_crtc_state = NULL;
> > +
> > +    if (new_crtc)
> > +        new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
> > +
> > +    return drm_atomic_helper_check_plane_state(new_plane_state, 
> > new_crtc_state,
> > +                           DRM_PLANE_NO_SCALING,
> > +                           DRM_PLANE_NO_SCALING,
> > +                           false, false);
> > +}
> > +
> >   static void udl_primary_plane_helper_atomic_update(struct drm_plane 
> > *plane,
> >                              struct drm_atomic_state *state)
> >   {
> > @@ -296,7 +311,7 @@ static void 
> > udl_primary_plane_helper_atomic_update(struct drm_plane *plane,
> >   static const struct drm_plane_helper_funcs 
> > udl_primary_plane_helper_funcs = {
> >       DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
> > -    .atomic_check = drm_plane_helper_atomic_check,
> > +    .atomic_check = udl_primary_plane_helper_atomic_check,
> >       .atomic_update = udl_primary_plane_helper_atomic_update,
> >   };
> > diff --git a/include/drm/drm_plane_helper.h 
> > b/include/drm/drm_plane_helper.h
> > index 3a574e8cd22f4..75f9c4830564a 100644
> > --- a/include/drm/drm_plane_helper.h
> > +++ b/include/drm/drm_plane_helper.h
> > @@ -26,7 +26,6 @@
> >   #include <linux/types.h>
> > -struct drm_atomic_state;
> >   struct drm_crtc;
> >   struct drm_framebuffer;
> >   struct drm_modeset_acquire_ctx;
> > @@ -42,7 +41,6 @@ int drm_plane_helper_update_primary(struct drm_plane 
> > *plane, struct drm_crtc *cr
> >   int drm_plane_helper_disable_primary(struct drm_plane *plane,
> >                        struct drm_modeset_acquire_ctx *ctx);
> >   void drm_plane_helper_destroy(struct drm_plane *plane);
> > -int drm_plane_helper_atomic_check(struct drm_plane *plane, struct 
> > drm_atomic_state *state);
> >   /**
> >    * DRM_PLANE_NON_ATOMIC_FUNCS - Default plane functions for 
> > non-atomic drivers

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature