Re: [PATCH v3 3/4] drm/hibmc: Do not use cpp from struct drm_format_info

[Yongbang Shi <shiyongbang@huawei.com>]

Hi, Thomas.

> Replace uses of struct drm_format_info's cpp with appropriate interfaces.
> The cpp field contains the characters per pixel. It is deprecated and
> should be avoided.
> 
> Calculate the line width in bytes with drm_format_info_min_pitch(). This
> is the preferred way of getting pixel and line sizes.
> 
> Program HIB_CRT_DISP_CTL_FORMAT from the format's 4CC code instead of
> calculating the field's value from the cpp.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> index 2e6e189bec1a..79c33c778d2c 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> @@ -100,8 +100,8 @@ static int hibmc_plane_atomic_check(struct drm_plane *plane,
>  static void hibmc_plane_atomic_update(struct drm_plane *plane,
>  				      struct drm_atomic_commit *state)
>  {
> -	struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state,
> -									   plane);
> +	struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state, plane);
> +	struct drm_framebuffer *fb = new_state->fb;
>  	u32 reg;
>  	s64 gpu_addr = 0;
>  	u32 line_l;
> @@ -119,7 +119,7 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane,
>  
>  	writel(gpu_addr, priv->mmio + HIBMC_CRT_FB_ADDRESS);
>  
> -	reg = new_state->fb->width * (new_state->fb->format->cpp[0]);
> +	reg = drm_format_info_min_pitch(fb->format, 0, fb->width);
>  
>  	line_l = new_state->fb->pitches[0];

Since the variable `fb` was defined earlier, it can be used directly here without needing `new_state->fb`.

This function uses `new_state->fb` in two other places:
https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/drm-misc-next/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c?ref_type=heads#L129
https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/drm-misc-next/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c?ref_type=heads#L132

Thanks,
Yongbang.

>  	writel(HIBMC_FIELD(HIBMC_CRT_FB_WIDTH_WIDTH, reg) |
> @@ -129,8 +129,14 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane,
>  	/* SET PIXEL FORMAT */
>  	reg = readl(priv->mmio + HIBMC_CRT_DISP_CTL);
>  	reg &= ~HIBMC_CRT_DISP_CTL_FORMAT_MASK;
> -	reg |= HIBMC_FIELD(HIBMC_CRT_DISP_CTL_FORMAT,
> -			   new_state->fb->format->cpp[0] * 8 / 16);
> +	switch (fb->format->format) {
> +	case DRM_FORMAT_XRGB8888:
> +		reg |= HIBMC_FIELD(HIBMC_CRT_DISP_CTL_FORMAT, 2);
> +		break;
> +	case DRM_FORMAT_RGB565:
> +		reg |= HIBMC_FIELD(HIBMC_CRT_DISP_CTL_FORMAT, 1);
> +		break;
> +	}
>  	writel(reg, priv->mmio + HIBMC_CRT_DISP_CTL);
>  }
>