On 08/05/2023 09:44, Thomas Zimmermann wrote:
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.
Ok I can change that. This patch is just a cleanup, and is not really
necessary for DMA.
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@xxxxxxxxxx>
---
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.
This code was overly complex, but this special case is compensated by
the "bpp_shift" thing. The formula width * bpp / 128 is in the G200
documentation 4.6.5
u32 offset = fb->pitches[0] / fb->format->cpp[0];
so offset is now the width in pixels. (pitches[0] is the width in bytes)
bppshift is 0 for 24 bits, 1 for 16 bits and 2 for 32 bits
offset:
16bit: (bppshift = 1) ; pitch = width * 2
offset = width >> (4 - bppshift) => width / 8 => pitch / 16
24bit: (bppshift = 0) ; pitch = width * 3
offset = (width * 3) >> (4 - bppshift) => width * 3 / 16 => pitch / 16
32bit: (bppshift = 2) ; pith = width * 4
offset = width >> (4 - bppshift) => width / 4 => pitch / 16
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);
}
--
Jocelyn
