Re: [PATCH v3 09/11] drm/i915: DisplayPort-MST pixel clock check

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Aug 12, 2015 at 09:49:12PM +0300, Ville Syrjälä wrote:
> On Fri, Jul 31, 2015 at 03:13:58PM +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 MST.
> > 
> > V2:
> > - removed computation for max pixel clock
> > 
> > V3:
> > - cleanup by removing unnecessary lines
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp_mst.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 585f0a4..fcf03d0 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -347,6 +347,8 @@ static enum drm_mode_status
> >  intel_dp_mst_mode_valid(struct drm_connector *connector,
> >  			struct drm_display_mode *mode)
> >  {
> > +	int max_pixclk = to_i915(connector->dev)->max_dotclk;
> 
> The pixclk vs. dotclk in every patch is tickling my ocd nerves. I'd say
> pick one and stick to it everywhere. I guess I'm probably to blame here
> since I've been using dotclock in the .get_config() paths, and pixclk
> in the cdclk calculations. With grep I can't say which one is winning,
> so I guess you could pick whichever seems more awesome.

+1 from me for a follow-up series to standardize on one and then maybe
even add a small DOC: kerneldoc comment explaining what all the different
clocks are that we have. We have much improved kerneldoc support now.
-Daniel

> 
> Apart from that this patch looks good so:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > +
> >  	/* TODO - validate mode against available PBN for link */
> >  	if (mode->clock < 10000)
> >  		return MODE_CLOCK_LOW;
> > @@ -354,6 +356,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
> >  	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> >  		return MODE_H_ILLEGAL;
> >  
> > +	if (mode->clock > max_pixclk)
> > +		return MODE_CLOCK_HIGH;
> > +
> >  	return MODE_OK;
> >  }
> >  
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx