[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f6a49350-74d0-4923-7c80-8e6233dc135b@suse.de>
Date: Tue, 11 Oct 2022 15:21:10 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Javier Martinez Canillas <javierm@...hat.com>,
linux-kernel@...r.kernel.org
Cc: Daniel Vetter <daniel@...ll.ch>, David Airlie <airlied@...ux.ie>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH 3/3] drm/crtc-helper: Add a drm_crtc_helper_atomic_check()
helper
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@...hat.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@...e.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
Download attachment "OpenPGP_signature" of type "application/pgp-signature" (841 bytes)
Powered by blists - more mailing lists