[PATCH] drm/simple-kms: Standardize arguments for callbacks

[Thomas Zimmermann]

Hi,

there are two types of callbacks in struct
drm_simple_display_pipe_funcs: functions that are genuine to simple KMS,
and functions that are merely forwarded from another structure (crtc,
plane, etc).

In the former category are enable(), disable(), check(), and update().
Those should probably receive a simple display pipe as their first argument.

In the latter category are mode_valid(), prepare_fb(), cleanup_fb(),
enable_vblank(), and disable_vblank(). IMHO those functions should
receive a pointer to the original structure as their first argument.
This type provides the context in which the operations make sense. (Even
their documentation already refers to the original structure.)

I admit that not all are as shareable as prepare_fb() and enable_fb().
But what else than boiler-plate wrappers do we get from simple display
pipe here?

Best regards
Thomas

Am 22.10.19 um 17:55 schrieb Daniel Vetter:
> Passing the wrong type feels icky, everywhere else we use the pipe as
> the first parameter. Spotted while discussing patches with Thomas
> Zimmermann.
> 
> Cc: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: Noralf Trønnes <noralf at tronnes.org>
> Cc: Gerd Hoffmann <kraxel at redhat.com>
> Cc: Eric Anholt <eric at anholt.net>
> Cc: Emil Velikov <emil.velikov at collabora.com>
> Cc: virtualization at lists.linux-foundation.org
> Cc: Linus Walleij <linus.walleij at linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  drivers/gpu/drm/cirrus/cirrus.c         | 2 +-
>  drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
>  drivers/gpu/drm/pl111/pl111_display.c   | 4 ++--
>  include/drm/drm_simple_kms_helper.h     | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
> index 7d08d067e1a4..248c9f765c45 100644
> --- a/drivers/gpu/drm/cirrus/cirrus.c
> +++ b/drivers/gpu/drm/cirrus/cirrus.c
> @@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
>  /* ------------------------------------------------------------------ */
>  /* cirrus (simple) display pipe					      */
>  
> -static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
> +static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
>  						   const struct drm_display_mode *mode)
>  {
>  	if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index 046055719245..15fb516ae2d8 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
>  		/* Anything goes */
>  		return MODE_OK;
>  
> -	return pipe->funcs->mode_valid(crtc, mode);
> +	return pipe->funcs->mode_valid(pipe, mode);
>  }
>  
>  static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 024771a4083e..703ddc803c55 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
>  }
>  
>  static enum drm_mode_status
> -pl111_mode_valid(struct drm_crtc *crtc,
> +pl111_mode_valid(struct drm_simple_display_pipe *pipe,
>  		 const struct drm_display_mode *mode)
>  {
> -	struct drm_device *drm = crtc->dev;
> +	struct drm_device *drm = pipe->crtc.dev;
>  	struct pl111_drm_dev_private *priv = drm->dev_private;
>  	u32 cpp = priv->variant->fb_bpp / 8;
>  	u64 bw;
> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> index 4d89cd0a60db..15afee9cf049 100644
> --- a/include/drm/drm_simple_kms_helper.h
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
>  	 *
>  	 * drm_mode_status Enum
>  	 */
> -	enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
> +	enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
>  					   const struct drm_display_mode *mode);
>  
>  	/**
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20191022/29fad445/attachment.sig>