Re: [PATCH 1/9] drm/i915: Check pixel clock when setting mode for DP

[Mika Kahola <mika.kahola@intel.com>]

On Fri, 2015-07-03 at 15:57 +0300, Ville Syrjälä wrote:
> On Fri, Jul 03, 2015 at 02:35:49PM +0300, Mika Kahola wrote:
> > It is possible the we request to have a mode that has
> > higher pixel clock than our HW can support. This patch
> > checks if requested pixel clock is lower than the one
> > supported by the HW. The requested mode is discarded
> > if we cannot support the requested pixel clock.
> > 
> > This patch applies to DisplayPort.
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 25 ++++++++++++++++++++++++-
> >  1 file changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index fcc64e5..2e55dff 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -197,6 +197,26 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
> >  	return (max_link_clock * max_lanes * 8) / 10;
> >  }
> >  
> > +static int
> > +intel_dp_max_pixclk(struct intel_dp *intel_dp)
> > +{
> > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > +	struct intel_encoder *encoder = &intel_dig_port->base;
> > +	struct drm_device *dev = encoder->base.dev;
> > +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +
> > +	if (IS_CHERRYVIEW(dev))
> > +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
> 
> Maybe we should precompute this stuff and store as max_dotclock into dev_priv?
> It has really nothing to do with DP.
> 
> > +	else if (IS_VALLEYVIEW(dev))
> > +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 90);
> > +	else if (IS_BROADWELL(dev) && intel_crtc->config->ips_enabled)
> 
> You can't look at the current crtc config at this point. Here we just
> need to consider the max we can support under any conditions and filter
> the modes based on that. That does mean that under some circumstances
> not all of the validated modes will actually work, but there's really
> nothing sensible we can do about that.
> 
> In the specific case of IPS, we can always choose to disable it at
> modeset time if the mode would otherwise exceed the limit. So IPS is
> never a good reason for rejecting a mode.
> 
> > +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
> > +	else
> > +		return dev_priv->max_cdclk_freq;
> 
> 90% is the correct limit for all older platforms.
> 
> Exceptions are CHV which has the 95% limit, and starting from HSW+
> we should be able to handle 100%.
> 
> For gen2/3 we also have the option of using double wide mode which means
> the limit there should be 2 * cdclk * 9 / 10.
> 

Thanks for the comments. I rephrase this patch series and store the
dotclock in dev_priv. This way, I hope, I can get rid of the repetitive
code what Chris pointed out.

Cheers,
Mika

> > +}
> > +
> >  static enum drm_mode_status
> >  intel_dp_mode_valid(struct drm_connector *connector,
> >  		    struct drm_display_mode *mode)
> > @@ -206,6 +226,7 @@ intel_dp_mode_valid(struct drm_connector *connector,
> >  	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
> >  	int target_clock = mode->clock;
> >  	int max_rate, mode_rate, max_lanes, max_link_clock;
> > +	int max_pixclk;
> >  
> >  	if (is_edp(intel_dp) && fixed_mode) {
> >  		if (mode->hdisplay > fixed_mode->hdisplay)
> > @@ -223,7 +244,9 @@ intel_dp_mode_valid(struct drm_connector *connector,
> >  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
> >  	mode_rate = intel_dp_link_required(target_clock, 18);
> >  
> > -	if (mode_rate > max_rate)
> > +	max_pixclk = intel_dp_max_pixclk(intel_dp);
> > +
> > +	if ((mode_rate > max_rate) || (target_clock > max_pixclk))
> >  		return MODE_CLOCK_HIGH;
> >  
> >  	if (mode->clock < 10000)
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx