[PATCH 2/4] drm/mgag200: Simplify offset and scale computation.

[Thomas Zimmermann]

Hi

Am 05.05.23 um 14:43 schrieb Jocelyn Falempe:
> Now that the driver handles only 16, 24 and 32-bit framebuffer,
> it can be simplified.

I think it should say that the driver never really handled 8-bit colors. 
Or at least I'm not aware of.

> 
> No functional changes.
> 
> offset:
> 16bit: (bppshift = 1)
> offset = width >> (4 - bppshift) => width / 8 => pitch / 16
> 
> 24bit:  (bppshift = 0)
> offset = (width * 3) >> (4 - bppshift)  => width * 3 / 16 => pitch / 16
> 
> 32bit:  (bppshift = 2)
> offset = width >> (4 - bppshift) => width / 4 => pitch / 16
> 
> scale:
> 16bit:
> scale = (1 << bppshift) - 1 => 1
> 24bit:
> scale = ((1 << bppshift) * 3) - 1 => 2
> 32bit:
> scale = (1 << bppshift) - 1 => 3
> 
> Signed-off-by: Jocelyn Falempe <jfalempe at redhat.com>
> ---
>   drivers/gpu/drm/mgag200/mgag200_mode.c | 63 +++++++-------------------
>   1 file changed, 16 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 9a24b9c00745..7d8c65372ac4 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -280,30 +280,16 @@ void mgag200_set_mode_regs(struct mga_device *mdev, const struct drm_display_mod
>   	WREG8(MGA_MISC_OUT, misc);
>   }
>   
> -static u8 mgag200_get_bpp_shift(const struct drm_format_info *format)
> -{
> -	static const u8 bpp_shift[] = {0, 1, 0, 2};
> -
> -	return bpp_shift[format->cpp[0] - 1];
> -}
> -
>   /*
>    * Calculates the HW offset value from the framebuffer's pitch. The
>    * offset is a multiple of the pixel size and depends on the display
> - * format.
> + * format. With width in pixels and pitch in bytes, the formula is:
> + * offset = width * bpp / 128 = pitch / 16
>    */
>   static u32 mgag200_calculate_offset(struct mga_device *mdev,
>   				    const struct drm_framebuffer *fb)
>   {
> -	u32 offset = fb->pitches[0] / fb->format->cpp[0];
> -	u8 bppshift = mgag200_get_bpp_shift(fb->format);
> -
> -	if (fb->format->cpp[0] * 8 == 24)
> -		offset = (offset * 3) >> (4 - bppshift);
> -	else
> -		offset = offset >> (4 - bppshift);
> -
> -	return offset;
> +	return fb->pitches[0] >> 4;

24-bit is different from the rest. I don't understand how you simplified 
this code.

Best regards
Thomas

>   }
>   
>   static void mgag200_set_offset(struct mga_device *mdev,
> @@ -326,48 +312,25 @@ static void mgag200_set_offset(struct mga_device *mdev,
>   void mgag200_set_format_regs(struct mga_device *mdev, const struct drm_format_info *format)
>   {
>   	struct drm_device *dev = &mdev->base;
> -	unsigned int bpp, bppshift, scale;
> +	unsigned int scale;
>   	u8 crtcext3, xmulctrl;
>   
> -	bpp = format->cpp[0] * 8;
> -
> -	bppshift = mgag200_get_bpp_shift(format);
> -	switch (bpp) {
> -	case 24:
> -		scale = ((1 << bppshift) * 3) - 1;
> -		break;
> -	default:
> -		scale = (1 << bppshift) - 1;
> -		break;
> -	}
> -
> -	RREG_ECRT(3, crtcext3);
> -
> -	switch (bpp) {
> -	case 8:
> -		xmulctrl = MGA1064_MUL_CTL_8bits;
> -		break;
> -	case 16:
> -		if (format->depth == 15)
> -			xmulctrl = MGA1064_MUL_CTL_15bits;
> -		else
> -			xmulctrl = MGA1064_MUL_CTL_16bits;
> +	switch (format->format) {
> +	case DRM_FORMAT_RGB565:
> +		xmulctrl = MGA1064_MUL_CTL_16bits;
>   		break;
> -	case 24:
> +	case DRM_FORMAT_RGB888:
>   		xmulctrl = MGA1064_MUL_CTL_24bits;
>   		break;
> -	case 32:
> +	case DRM_FORMAT_XRGB8888:
>   		xmulctrl = MGA1064_MUL_CTL_32_24bits;
>   		break;
>   	default:
>   		/* BUG: We should have caught this problem already. */
> -		drm_WARN_ON(dev, "invalid format depth\n");
> +		drm_WARN_ON(dev, "invalid drm format\n");
>   		return;
>   	}
>   
> -	crtcext3 &= ~GENMASK(2, 0);
> -	crtcext3 |= scale;
> -
>   	WREG_DAC(MGA1064_MUL_CTL, xmulctrl);
>   
>   	WREG_GFX(0, 0x00);
> @@ -383,6 +346,12 @@ void mgag200_set_format_regs(struct mga_device *mdev, const struct drm_format_in
>   	WREG_GFX(7, 0x0f);
>   	WREG_GFX(8, 0x0f);
>   
> +	/* scale is the number of bytes per pixels - 1 */
> +	scale = format->cpp[0] - 1;
> +
> +	RREG_ECRT(3, crtcext3);
> +	crtcext3 &= ~GENMASK(2, 0);
> +	crtcext3 |= scale;
>   	WREG_ECRT(3, crtcext3);
>   }
>   

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20230508/3ae94916/attachment.sig>