Re: [PATCH 2/2] drm/ast: Add reduced/non-reduced mode parsing for wide screen mode

[Egbert Eich <eich@suse.com>]

Hi YC,

Y.C. Chen writes:
 > From: "Y.C. Chen" <yc_chen@aspeedtech.com>
 > 
 > Signed-off-by: Y.C. Chen <yc_chen@aspeedtech.com>
 > ---
 >  drivers/gpu/drm/ast/ast_mode.c   | 32 +++++++++++++++++++++++-------
 >  drivers/gpu/drm/ast/ast_tables.h | 42 ++++++++++++++++++++++++----------------
 >  2 files changed, 50 insertions(+), 24 deletions(-)
 > 
 > diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
 > index 5389350..5533920 100644
 > --- a/drivers/gpu/drm/ast/ast_mode.c
 > +++ b/drivers/gpu/drm/ast/ast_mode.c
 > @@ -141,14 +141,30 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo
 >  	}
 >  
 >  	refresh_rate = drm_mode_vrefresh(mode);
 > -	while (vbios_mode->enh_table->refresh_rate < refresh_rate) {
 > -		vbios_mode->enh_table++;
 > -		if ((vbios_mode->enh_table->refresh_rate > refresh_rate) ||
 > -		    (vbios_mode->enh_table->refresh_rate == 0xff)) {
 > +	do {
 > +		if ((vbios_mode->enh_table->flags & WideScreenMode) &&
 > +		    (((mode->flags & DRM_MODE_FLAG_NVSYNC)  &&
 > +		      (vbios_mode->enh_table->flags & PVSync))  ||
 > +	 	     ((mode->flags & DRM_MODE_FLAG_PVSYNC)  &&
 > +		      (vbios_mode->enh_table->flags & NVSync))  ||
 > +		     ((mode->flags & DRM_MODE_FLAG_NHSYNC)  &&
 > +		      (vbios_mode->enh_table->flags & PHSync))  ||
 > +		     ((mode->flags & DRM_MODE_FLAG_PHSYNC)  &&
 > +		      (vbios_mode->enh_table->flags & NHSync)))) {
 > +			vbios_mode->enh_table++;
 > +			continue;
 > +		}
 > +		if (vbios_mode->enh_table->refresh_rate < refresh_rate) {
 > +			vbios_mode->enh_table++;
 > +		}
 > +		if ((vbios_mode->enh_table->refresh_rate_index > 1) &&
 > +		    (vbios_mode->enh_table->refresh_rate > refresh_rate)) {
 >  			vbios_mode->enh_table--;
 >  			break;
 >  		}
 > -	}
 > +	} while (vbios_mode->enh_table->refresh_rate != 0xff);
 > +	if (vbios_mode->enh_table->refresh_rate == 0xff)
 > +		vbios_mode->enh_table--;

I've tested this and experimented around a bit and came up with code like this:

        bool check_sync;
        struct ast_vbios_enhtable *best = NULL;
        [..]

        refresh_rate = drm_mode_vrefresh(mode);
        check_sync = vbios_mode->enh_table->flags & WideScreenMode;
        do {
                struct ast_vbios_enhtable *loop = best = vbios_mode->enh_table;

                while (loop->refresh_rate != 0xff) {
                        if ((check_sync) &&
                            (((mode->flags & DRM_MODE_FLAG_NVSYNC)  &&
                              (loop->flags & PVSync))  ||
                             ((mode->flags & DRM_MODE_FLAG_PVSYNC)  &&
                              (loop->flags & NVSync))  ||
                             ((mode->flags & DRM_MODE_FLAG_NHSYNC)  &&
                              (loop->flags & PHSync))  ||
                             ((mode->flags & DRM_MODE_FLAG_PHSYNC)  &&
                              (loop->flags & NHSync)))) {
                                loop++;
                                continue;
                        }
                        if (loop->refresh_rate <= refresh_rate
                            && loop->refresh_rate > best->refresh_rate)
                                best = loop;
                        loop++;
                }
                if (!check_sync)
                        break;
                check_sync = 0;
        } while (1);
        if (!best)
                return false;
        vbios_mode->enh_table = best;

This way the code doesn't make the assumption that the refresh rates
are in ascending order and we can map the sync polarities for all modes,
not just the wide screen ones.

 >  
 >  	hborder = (vbios_mode->enh_table->flags & HBorder) ? 8 : 0;
 >  	vborder = (vbios_mode->enh_table->flags & VBorder) ? 8 : 0;
 > @@ -419,8 +435,10 @@ static void ast_set_sync_reg(struct drm_device *dev, struct drm_display_mode *mo
 >  	struct ast_private *ast = dev->dev_private;
 >  	u8 jreg;
 >  
 > -	jreg = ast_io_read8(ast, AST_IO_MISC_PORT_READ);
 > -	jreg |= (vbios_mode->enh_table->flags & SyncNN);
 > +	jreg  = ast_io_read8(ast, AST_IO_MISC_PORT_READ);
 > +	jreg &= 0xC0;

This should be: 
        jreg &= ~0xC0U;

 > +	if (vbios_mode->enh_table->flags & NVSync) jreg |= 0x80;
 > +	if (vbios_mode->enh_table->flags & NHSync) jreg |= 0x40;
 >  	ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, jreg);
 >  }
 >  

Cheers,
	Egbert.