Hi Thomas.

On Wed, Apr 29, 2020 at 04:32:31PM +0200, Thomas Zimmermann wrote:
> The framebuffer's pitch is now set in mgag200_set_offset().
> 
> Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>

I failed to follow this code.

> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 41 +++++++++++++++++---------
>  1 file changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 92dee31f16c42..eb83e471d72fc 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1003,6 +1003,32 @@ static void mgag200_set_mode_regs(struct mga_device 
> *mdev,
>       mga_crtc_set_plls(mdev, mode->clock);
>  }
>  
> +static void mgag200_set_offset(struct mga_device *mdev,
> +                            const struct drm_framebuffer *fb)
> +{
> +     unsigned int offset;
> +     uint8_t crtc13, crtcext0;
> +     u8 bppshift;
> +
> +     bppshift = mdev->bpp_shifts[fb->format->cpp[0] - 1];
Hmm, use of cpp is deprecated. But is continue to be used all over.

> +
> +     offset = fb->pitches[0] / fb->format->cpp[0];
> +     if (fb->format->cpp[0] * 8 == 24)
> +             offset = (offset * 3) >> (4 - bppshift);
> +     else
> +             offset = offset >> (4 - bppshift);
> +
> +     RREG_ECRT(0, crtcext0);
> +
> +     crtc13 = offset & 0xff;
> +
> +     crtcext0 &= ~GENMASK(5, 4);
> +     crtcext0 |= (offset & GENMASK(9, 8)) >> 4;
Lot's of hardcoded numbers.
Could the reg file include these so you had more readable defined names?

> +
> +     WREG_CRT(0x13, crtc13);
> +     WREG_ECRT(0x00, crtcext0);
> +}
> +
>  static int mga_crtc_mode_set(struct drm_crtc *crtc,
>                               struct drm_display_mode *mode,
>                               struct drm_display_mode *adjusted_mode,
> @@ -1011,7 +1037,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 pitch;
>       int option = 0, option2 = 0;
>       int i;
>       unsigned char misc = 0;
> @@ -1122,12 +1147,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>       WREG_SEQ(3, 0);
>       WREG_SEQ(4, 0xe);
>  
> -     pitch = fb->pitches[0] / fb->format->cpp[0];
> -     if (fb->format->cpp[0] * 8 == 24)
> -             pitch = (pitch * 3) >> (4 - bppshift);
> -     else
> -             pitch = pitch >> (4 - bppshift);
> -
>       WREG_GFX(0, 0);
>       WREG_GFX(1, 0);
>       WREG_GFX(2, 0);
> @@ -1144,20 +1163,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>       WREG_CRT(13, 0);
>       WREG_CRT(14, 0);
>       WREG_CRT(15, 0);
> -     WREG_CRT(19, pitch & 0xFF);
> -
> -     ext_vga[0] = 0;
>  
>       /* TODO interlace */
>  
> -     ext_vga[0] |= (pitch & 0x300) >> 4;
>       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;
>  
> -     WREG_ECRT(0, ext_vga[0]);
>       WREG_ECRT(3, ext_vga[3]);
>       WREG_ECRT(4, ext_vga[4]);
>  
> @@ -1171,8 +1185,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>               WREG_ECRT(6, 0);
>       }
>  
> -     WREG_ECRT(0, ext_vga[0]);
> -
>       misc = RREG8(MGA_MISC_IN);
>       misc |= MGAREG_MISC_IOADSEL |
>               MGAREG_MISC_RAMMAPEN |
> @@ -1180,6 +1192,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>       WREG8(MGA_MISC_OUT, misc);
>  
>       mga_crtc_do_set_base(mdev, fb, old_fb);
> +     mgag200_set_offset(mdev, fb);
>  
>       mgag200_set_mode_regs(mdev, mode);

        Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to