| Message ID | 20231128104723.20622-2-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm/plane-helpers: Minor clean ups | expand |
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@suse.de> > --- > 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. > 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
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@suse.de> >> --- >> 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
Hi, On 2023/12/1 16:22, Thomas Zimmermann wrote: > 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@suse.de> >>> --- >>> 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. > OK, with this trivial problem solved: Acked-by: Sui Jingfeng <suijingfeng@loongson.cn> > 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 >
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); 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
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@suse.de> --- 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(-)