[Intel-gfx] [PATCH 2/3] drm/i915: Check timings against hardware maximums

[Ville Syrjälä]

On Fri, Jun 15, 2018 at 01:30:24PM -0700, Paulo Zanoni wrote:
> Em Sex, 2018-06-15 às 20:44 +0300, Ville Syrjala escreveu:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Validate that all display timings fit within the number of bits
> > we have in the transcoder timing registers.
> > 
> > The limits are:
> > hsw+:
> >  4k: vdisplay, vblank_start
> >  8k: everything else
> > gen3+:
> >  4k: h/vdisplay, h/vblank_start
> >  8k: everything else
> > gen2:
> >  2k: h/vdisplay, h/vblank_start
> >  4k: everything else
> > 
> > Also document the fact that the mode_config.max_width/height limits
> > refer to just the max framebuffer dimensions we support. Which may
> > be larger than the max hdisplay/vdisplay.
> 
> Verified against the specs. I can also confirm that the gen2+ specs
> still exist at the old URLs :).

Cool. Thanks for digging through them.

> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> While at it, can't you try to implement the other restrictions listed
> in those active/sync registers? Like hdisplay needs be multiples of 2
> on gen2, hblank minimum being 32 on HSW (138 with audio), minimum
> vblank veing 5 or 8, etc?

I did more or less collect up the minimum/alignment restrictions as
well, but wasn't really convinced checking for them is all that useful.
There are probably some we should check like the HDMI/audio
hblank/front porch restrictions, and if those aren't met we should
probably fall back to driving the link on DVI mode or w/o audio.

If we do check all the minimums then we'd have to decide whether to
check against the absolute minimum from the spec for each platforms
(which would end up being somewhat messy on account of the limits having
changed so many times) or if we should just simplify a bit and declare
eg. a minimum of 8 (or even higher) for hdisplay/vdisplay across the
board.

> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 35
> > +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index f6655f482b67..6e3aa6815b30 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14572,6 +14572,10 @@ static enum drm_mode_status
> >  intel_mode_valid(struct drm_device *dev,
> >  		 const struct drm_display_mode *mode)
> >  {
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > +	int hdisplay_max, htotal_max;
> > +	int vdisplay_max, vtotal_max;
> > +
> >  	/*
> >  	 * Can't reject DBLSCAN here because Xorg ddxen can add
> > piles
> >  	 * of DBLSCAN modes to the output's mode list when they
> > detect
> > @@ -14601,6 +14605,36 @@ intel_mode_valid(struct drm_device *dev,
> >  			   DRM_MODE_FLAG_CLKDIV2))
> >  		return MODE_BAD;
> >  
> > +	if (INTEL_GEN(dev_priv) >= 9 ||
> > +	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv)) {
> > +		hdisplay_max = 8192; /* FDI max 4096 handled
> > elsewhere */
> > +		vdisplay_max = 4096;
> > +		htotal_max = 8192;
> > +		vtotal_max = 8192;
> > +	} else if (INTEL_GEN(dev_priv) >= 3) {
> > +		hdisplay_max = 4096;
> > +		vdisplay_max = 4096;
> > +		htotal_max = 8192;
> > +		vtotal_max = 8192;
> > +	} else {
> > +		hdisplay_max = 2048;
> > +		vdisplay_max = 2048;
> > +		htotal_max = 4096;
> > +		vtotal_max = 4096;
> > +	}
> > +
> > +	if (mode->hdisplay > hdisplay_max ||
> > +	    mode->hsync_start > htotal_max ||
> > +	    mode->hsync_end > htotal_max ||
> > +	    mode->htotal > htotal_max)
> > +		return MODE_H_ILLEGAL;
> > +
> > +	if (mode->vdisplay > vdisplay_max ||
> > +	    mode->vsync_start > vtotal_max ||
> > +	    mode->vsync_end > vtotal_max ||
> > +	    mode->vtotal > vtotal_max)
> > +		return MODE_V_ILLEGAL;
> > +
> >  	return MODE_OK;
> >  }
> >  
> > @@ -15039,6 +15073,7 @@ int intel_modeset_init(struct drm_device
> > *dev)
> >  		}
> >  	}
> >  
> > +	/* maximum framebuffer dimensions */
> >  	if (IS_GEN2(dev_priv)) {
> >  		dev->mode_config.max_width = 2048;
> >  		dev->mode_config.max_height = 2048;

-- 
Ville Syrjälä
Intel