Hi Thomas.

One nit about a bit name below.
Acked-by: Sam Ravnborg <s...@ravnborg.org>

On Wed, Apr 29, 2020 at 04:32:33PM +0200, Thomas Zimmermann wrote:
> 5
> 
> The TAGFIFO state is now reset in mgag200_g200er_reset_tagfifo().
> 
> Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
> ---
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  6 ++++
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 45 +++++++++++++++++---------
>  2 files changed, 35 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h 
> b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index 9b957d9fc7e04..b10da90e0f35a 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -49,6 +49,12 @@
>               WREG8(ATTR_DATA, v);                            \
>       } while (0)                                             \
>  
> +#define RREG_SEQ(reg, v)                                     \
> +     do {                                                    \
> +             WREG8(MGAREG_SEQ_INDEX, reg);                   \
> +             v = RREG8(MGAREG_SEQ_DATA);                     \
> +     } while (0)                                             \
> +
>  #define WREG_SEQ(reg, v)                                     \
>       do {                                                    \
>               WREG8(MGAREG_SEQ_INDEX, reg);                   \
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 73f7135cbb3d8..6b88c306ff4d7 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1091,6 +1091,33 @@ static void mgag200_set_format_regs(struct mga_device 
> *mdev,
>       WREG_ECRT(3, crtcext3);
>  }
>  
> +static void mgag200_g200er_reset_tagfifo(struct mga_device *mdev)
> +{
> +     static uint32_t RESET_FLAG = 0x00200000; /* undocumented magic value */
> +     u8 seq1;
> +     u32 memctl;
> +
> +     /* screen off */
> +     RREG_SEQ(0x01, seq1);
> +     seq1 |= 0x20;
This looks like this:
#define        M_SEQ1_SCROFF            0x20


> +     WREG_SEQ(0x01, seq1);
> +
> +     memctl = RREG32(MGAREG_MEMCTL);
> +
> +     memctl |= RESET_FLAG;
> +     WREG32(MGAREG_MEMCTL, memctl);
> +
> +     udelay(1000);
> +
> +     memctl &= ~RESET_FLAG;
> +     WREG32(MGAREG_MEMCTL, memctl);
> +
> +     /* screen on */
> +     RREG_SEQ(0x01, seq1);
> +     seq1 &= ~0x20;
> +     WREG_SEQ(0x01, seq1);
Here seq1 is read again, the old code used the old value.
I think new code is better.

> +}
> +
>  static int mga_crtc_mode_set(struct drm_crtc *crtc,
>                               struct drm_display_mode *mode,
>                               struct drm_display_mode *adjusted_mode,
> @@ -1225,22 +1252,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  
>       mgag200_set_mode_regs(mdev, mode);
>  
> -     /* reset tagfifo */
> -     if (mdev->type == G200_ER) {
> -             u32 mem_ctl = RREG32(MGAREG_MEMCTL);
> -             u8 seq1;
> -
> -             /* screen off */
> -             WREG8(MGAREG_SEQ_INDEX, 0x01);
> -             seq1 = RREG8(MGAREG_SEQ_DATA) | 0x20;
> -             WREG8(MGAREG_SEQ_DATA, seq1);
> -
> -             WREG32(MGAREG_MEMCTL, mem_ctl | 0x00200000);
> -             udelay(1000);
> -             WREG32(MGAREG_MEMCTL, mem_ctl & ~0x00200000);
> -
> -             WREG8(MGAREG_SEQ_DATA, seq1 & ~0x20);
> -     }
> +     if (mdev->type == G200_ER)
> +             mgag200_g200er_reset_tagfifo(mdev);
>  
>  
>       if (IS_G200_SE(mdev)) {
> -- 
> 2.26.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-dev
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to