lkml.org 
[lkml]   [2022]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH 3/3] drm/crtc-helper: Add a drm_crtc_helper_atomic_check() helper
From
Hi

Am 10.10.22 um 19:02 schrieb Javier Martinez Canillas:
> Provides a default CRTC state check handler for CRTCs that only have one
> primary plane attached.
>
> There are some drivers that duplicate this logic in their helpers, such as
> simpledrm and ssd130x. Factor out this common code into a CRTC helper and
> make drivers use it.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

There really isn't much here for now. I suspect that there are more
drivers that could use this helper. If you merge this before ofdrm, I'll
rebase ofdrm on top.

Please also see my comment below.


> ---
>
> drivers/gpu/drm/drm_crtc_helper.c | 24 ++++++++++++++++++++++++
> drivers/gpu/drm/solomon/ssd130x.c | 14 ++------------
> drivers/gpu/drm/tiny/simpledrm.c | 14 ++------------
> include/drm/drm_crtc_helper.h | 2 ++
> 4 files changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 457448cc60f7..4ad3abaa98f4 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -421,6 +421,30 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
> }
> EXPORT_SYMBOL(drm_crtc_helper_set_mode);
>
> +/**
> + * drm_crtc_helper_atomic_check() - Helper to check CRTC atomic-state
> + * @crtc: CRTC to check
> + * @state: atomic state object
> + *
> + * Provides a default CRTC-state check handler for CRTCs that only have
> + * one primary plane attached to it.
> + *
> + * This is often the case for the CRTC of simple framebuffers.

I'd add a reference to drm_plane_helper_atomic_check() to this
paragraph. Like

See drm_plane_helper_atomic_check() for the respective plane helpers.

And also reference back from the plane-check helper to the CRTC-check
helper.

Best regards
Thomas

> + *
> + * RETURNS:
> + * Zero on success, or an errno code otherwise.
> + */
> +int drm_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state)
> +{
> + struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> +
> + if (!new_crtc_state->enable)
> + return 0;
> +
> + return drm_atomic_helper_check_crtc_primary_plane(new_crtc_state);
> +}
> +EXPORT_SYMBOL(drm_crtc_helper_atomic_check);
> +
> static void
> drm_crtc_helper_disable(struct drm_crtc *crtc)
> {
> diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
> index 0d4ab65233db..f2795f90ea69 100644
> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> @@ -20,6 +20,7 @@
>
> #include <drm/drm_atomic.h>
> #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> #include <drm/drm_damage_helper.h>
> #include <drm/drm_edid.h>
> #include <drm/drm_fb_helper.h>
> @@ -645,17 +646,6 @@ static enum drm_mode_status ssd130x_crtc_helper_mode_valid(struct drm_crtc *crtc
> return MODE_OK;
> }
>
> -static int ssd130x_crtc_helper_atomic_check(struct drm_crtc *crtc,
> - struct drm_atomic_state *new_state)
> -{
> - struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(new_state, crtc);
> -
> - if (!new_crtc_state->enable)
> - return 0;
> -
> - return drm_atomic_helper_check_crtc_primary_plane(new_crtc_state);
> -}
> -
> /*
> * The CRTC is always enabled. Screen updates are performed by
> * the primary plane's atomic_update function. Disabling clears
> @@ -663,7 +653,7 @@ static int ssd130x_crtc_helper_atomic_check(struct drm_crtc *crtc,
> */
> static const struct drm_crtc_helper_funcs ssd130x_crtc_helper_funcs = {
> .mode_valid = ssd130x_crtc_helper_mode_valid,
> - .atomic_check = ssd130x_crtc_helper_atomic_check,
> + .atomic_check = drm_crtc_helper_atomic_check,
> };
>
> static void ssd130x_crtc_reset(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index f03f17f62a56..cbb100753154 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -11,6 +11,7 @@
> #include <drm/drm_atomic.h>
> #include <drm/drm_atomic_state_helper.h>
> #include <drm/drm_connector.h>
> +#include <drm/drm_crtc_helper.h>
> #include <drm/drm_damage_helper.h>
> #include <drm/drm_device.h>
> #include <drm/drm_drv.h>
> @@ -545,17 +546,6 @@ static enum drm_mode_status simpledrm_crtc_helper_mode_valid(struct drm_crtc *cr
> return drm_crtc_helper_mode_valid_fixed(crtc, mode, &sdev->mode);
> }
>
> -static int simpledrm_crtc_helper_atomic_check(struct drm_crtc *crtc,
> - struct drm_atomic_state *new_state)
> -{
> - struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(new_state, crtc);
> -
> - if (!new_crtc_state->enable)
> - return 0;
> -
> - return drm_atomic_helper_check_crtc_primary_plane(new_crtc_state);
> -}
> -
> /*
> * The CRTC is always enabled. Screen updates are performed by
> * the primary plane's atomic_update function. Disabling clears
> @@ -563,7 +553,7 @@ static int simpledrm_crtc_helper_atomic_check(struct drm_crtc *crtc,
> */
> static const struct drm_crtc_helper_funcs simpledrm_crtc_helper_funcs = {
> .mode_valid = simpledrm_crtc_helper_mode_valid,
> - .atomic_check = simpledrm_crtc_helper_atomic_check,
> + .atomic_check = drm_crtc_helper_atomic_check,
> };
>
> static const struct drm_crtc_funcs simpledrm_crtc_funcs = {
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index a6d520d5b6ca..1840db247f69 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -50,6 +50,8 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
> struct drm_display_mode *mode,
> int x, int y,
> struct drm_framebuffer *old_fb);
> +int drm_crtc_helper_atomic_check(struct drm_crtc *crtc,
> + struct drm_atomic_state *state);
> bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
> bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
>

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2022-10-11 15:22    [W:0.077 / U:0.408 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site