[PATCH 07/17] drm/mgag200: Move mode-setting code into separate helper function
Thomas Zimmermann
tzimmermann at suse.de
Thu Apr 30 08:27:02 UTC 2020
Hi
Am 29.04.20 um 20:24 schrieb Sam Ravnborg:
> On Wed, Apr 29, 2020 at 04:32:28PM +0200, Thomas Zimmermann wrote:
>> The mode-setting code is now located in mgag200_set_mode_regs(), sans
>> a few flags that will be moved in a later patch for clarity.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> Looked like trivial code movements, did not look carefully.
It's often not that trivial.
The complicated thing about these functions is to not override bits that
have been set by another functions already. For example, MISC has a
number of bits that control PLL, mode, and hsync/vsync. Each is set in a
different place. I've been careful to read-back MISC each time and
update each setting individually.
Best regards
Thomas
> Acked-by: Sam Ravnborg <sam at ravnborg.org>
>
>> ---
>> drivers/gpu/drm/mgag200/mgag200_mode.c | 140 ++++++++++++++-----------
>> 1 file changed, 78 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> index 12df809d64f7c..749ba6e420ac7 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> @@ -911,6 +911,79 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>> return mga_crtc_do_set_base(mdev, fb, old_fb);
>> }
>>
>> +static void mgag200_set_mode_regs(struct mga_device *mdev,
>> + const struct drm_display_mode *mode)
>> +{
>> + unsigned int hdisplay, hsyncstart, hsyncend, htotal;
>> + unsigned int vdisplay, vsyncstart, vsyncend, vtotal;
>> + uint8_t misc = 0;
>> + uint8_t crtcext1, crtcext2, crtcext5;
>> +
>> + hdisplay = mode->hdisplay / 8 - 1;
>> + hsyncstart = mode->hsync_start / 8 - 1;
>> + hsyncend = mode->hsync_end / 8 - 1;
>> + htotal = mode->htotal / 8 - 1;
>> +
>> + /* Work around hardware quirk */
>> + if ((htotal & 0x07) == 0x06 || (htotal & 0x07) == 0x04)
>> + htotal++;
>> +
>> + vdisplay = mode->vdisplay - 1;
>> + vsyncstart = mode->vsync_start - 1;
>> + vsyncend = mode->vsync_end - 1;
>> + vtotal = mode->vtotal - 2;
>> +
>> + if (mode->flags & DRM_MODE_FLAG_NHSYNC)
>> + misc |= 0x40;
>> + if (mode->flags & DRM_MODE_FLAG_NVSYNC)
>> + misc |= 0x80;
>> +
>> + crtcext1 = (((htotal - 4) & 0x100) >> 8) |
>> + ((hdisplay & 0x100) >> 7) |
>> + ((hsyncstart & 0x100) >> 6) |
>> + (htotal & 0x40);
>> + if (mdev->type == G200_WB || mdev->type == G200_EW3)
>> + crtcext1 |= BIT(7) | /* vrsten */
>> + BIT(3); /* hrsten */
>> +
>> + crtcext2 = ((vtotal & 0xc00) >> 10) |
>> + ((vdisplay & 0x400) >> 8) |
>> + ((vdisplay & 0xc00) >> 7) |
>> + ((vsyncstart & 0xc00) >> 5) |
>> + ((vdisplay & 0x400) >> 3);
>> + crtcext5 = 0x00;
>> +
>> + WREG_CRT(0, htotal - 4);
>> + WREG_CRT(1, hdisplay);
>> + WREG_CRT(2, hdisplay);
>> + WREG_CRT(3, (htotal & 0x1F) | 0x80);
>> + WREG_CRT(4, hsyncstart);
>> + WREG_CRT(5, ((htotal & 0x20) << 2) | (hsyncend & 0x1F));
>> + WREG_CRT(6, vtotal & 0xFF);
>> + WREG_CRT(7, ((vtotal & 0x100) >> 8) |
>> + ((vdisplay & 0x100) >> 7) |
>> + ((vsyncstart & 0x100) >> 6) |
>> + ((vdisplay & 0x100) >> 5) |
>> + ((vdisplay & 0x100) >> 4) | /* linecomp */
>> + ((vtotal & 0x200) >> 4) |
>> + ((vdisplay & 0x200) >> 3) |
>> + ((vsyncstart & 0x200) >> 2));
>> + WREG_CRT(9, ((vdisplay & 0x200) >> 4) |
>> + ((vdisplay & 0x200) >> 3));
>> + WREG_CRT(16, vsyncstart & 0xFF);
>> + WREG_CRT(17, (vsyncend & 0x0F) | 0x20);
>> + WREG_CRT(18, vdisplay & 0xFF);
>> + WREG_CRT(20, 0);
>> + WREG_CRT(21, vdisplay & 0xFF);
>> + WREG_CRT(22, (vtotal + 1) & 0xFF);
>> + WREG_CRT(23, 0xc3);
>> + WREG_CRT(24, vdisplay & 0xFF);
>> +
>> + WREG_ECRT(0x01, crtcext1);
>> + WREG_ECRT(0x02, crtcext2);
>> + WREG_ECRT(0x05, crtcext5);
>> +}
>> +
>> static int mga_crtc_mode_set(struct drm_crtc *crtc,
>> struct drm_display_mode *mode,
>> struct drm_display_mode *adjusted_mode,
>> @@ -919,8 +992,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>> struct drm_device *dev = crtc->dev;
>> struct mga_device *mdev = dev->dev_private;
>> const struct drm_framebuffer *fb = crtc->primary->fb;
>> - int hdisplay, hsyncstart, hsyncend, htotal;
>> - int vdisplay, vsyncstart, vsyncend, vtotal;
>> int pitch;
>> int option = 0, option2 = 0;
>> int i;
>> @@ -999,12 +1070,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>> break;
>> }
>>
>> - if (mode->flags & DRM_MODE_FLAG_NHSYNC)
>> - misc |= 0x40;
>> - if (mode->flags & DRM_MODE_FLAG_NVSYNC)
>> - misc |= 0x80;
>> -
>> -
>> for (i = 0; i < sizeof(dacvalue); i++) {
>> if ((i <= 0x17) ||
>> (i == 0x1b) ||
>> @@ -1044,20 +1109,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>> else
>> pitch = pitch >> (4 - bppshift);
>>
>> - hdisplay = mode->hdisplay / 8 - 1;
>> - hsyncstart = mode->hsync_start / 8 - 1;
>> - hsyncend = mode->hsync_end / 8 - 1;
>> - htotal = mode->htotal / 8 - 1;
>> -
>> - /* Work around hardware quirk */
>> - if ((htotal & 0x07) == 0x06 || (htotal & 0x07) == 0x04)
>> - htotal++;
>> -
>> - vdisplay = mode->vdisplay - 1;
>> - vsyncstart = mode->vsync_start - 1;
>> - vsyncend = mode->vsync_end - 1;
>> - vtotal = mode->vtotal - 2;
>> -
>> WREG_GFX(0, 0);
>> WREG_GFX(1, 0);
>> WREG_GFX(2, 0);
>> @@ -1068,61 +1119,26 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>> WREG_GFX(7, 0xf);
>> WREG_GFX(8, 0xf);
>>
>> - WREG_CRT(0, htotal - 4);
>> - WREG_CRT(1, hdisplay);
>> - WREG_CRT(2, hdisplay);
>> - WREG_CRT(3, (htotal & 0x1F) | 0x80);
>> - WREG_CRT(4, hsyncstart);
>> - WREG_CRT(5, ((htotal & 0x20) << 2) | (hsyncend & 0x1F));
>> - WREG_CRT(6, vtotal & 0xFF);
>> - WREG_CRT(7, ((vtotal & 0x100) >> 8) |
>> - ((vdisplay & 0x100) >> 7) |
>> - ((vsyncstart & 0x100) >> 6) |
>> - ((vdisplay & 0x100) >> 5) |
>> - ((vdisplay & 0x100) >> 4) | /* linecomp */
>> - ((vtotal & 0x200) >> 4)|
>> - ((vdisplay & 0x200) >> 3) |
>> - ((vsyncstart & 0x200) >> 2));
>> - WREG_CRT(9, ((vdisplay & 0x200) >> 4) |
>> - ((vdisplay & 0x200) >> 3));
>> WREG_CRT(10, 0);
>> WREG_CRT(11, 0);
>> WREG_CRT(12, 0);
>> WREG_CRT(13, 0);
>> WREG_CRT(14, 0);
>> WREG_CRT(15, 0);
>> - WREG_CRT(16, vsyncstart & 0xFF);
>> - WREG_CRT(17, (vsyncend & 0x0F) | 0x20);
>> - WREG_CRT(18, vdisplay & 0xFF);
>> WREG_CRT(19, pitch & 0xFF);
>> - WREG_CRT(20, 0);
>> - WREG_CRT(21, vdisplay & 0xFF);
>> - WREG_CRT(22, (vtotal + 1) & 0xFF);
>> - WREG_CRT(23, 0xc3);
>> - WREG_CRT(24, vdisplay & 0xFF);
>> +
>> + mgag200_set_mode_regs(mdev, mode);
>>
>> ext_vga[0] = 0;
>> - ext_vga[5] = 0;
>>
>> /* TODO interlace */
>>
>> ext_vga[0] |= (pitch & 0x300) >> 4;
>> - ext_vga[1] = (((htotal - 4) & 0x100) >> 8) |
>> - ((hdisplay & 0x100) >> 7) |
>> - ((hsyncstart & 0x100) >> 6) |
>> - (htotal & 0x40);
>> - ext_vga[2] = ((vtotal & 0xc00) >> 10) |
>> - ((vdisplay & 0x400) >> 8) |
>> - ((vdisplay & 0xc00) >> 7) |
>> - ((vsyncstart & 0xc00) >> 5) |
>> - ((vdisplay & 0x400) >> 3);
>> if (fb->format->cpp[0] * 8 == 24)
>> ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80;
>> else
>> ext_vga[3] = ((1 << bppshift) - 1) | 0x80;
>> ext_vga[4] = 0;
>> - if (mdev->type == G200_WB || mdev->type == G200_EW3)
>> - ext_vga[1] |= 0x88;
>>
>> /* Set pixel clocks */
>> misc = 0x2d;
>> @@ -1130,9 +1146,9 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>
>> mga_crtc_set_plls(mdev, mode->clock);
>>
>> - for (i = 0; i < 6; i++) {
>> - WREG_ECRT(i, ext_vga[i]);
>> - }
>> + WREG_ECRT(0, ext_vga[0]);
>> + WREG_ECRT(3, ext_vga[3]);
>> + WREG_ECRT(4, ext_vga[4]);
>>
>> if (mdev->type == G200_ER)
>> WREG_ECRT(0x24, 0x5);
>> --
>> 2.26.0
--
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/20200430/e22d59ed/attachment.sig>
More information about the dri-devel
mailing list