Hi

Am 03.05.20 um 18:25 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> One nit about a bit name below.
> Acked-by: Sam Ravnborg <sam@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 <tzimmermann@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.

You mean 'the old code was better,' right?

Best regards
Thomas

> 
>> +}
>> +
>>  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

-- 
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