[Intel-gfx] [PATCH v3 03/11] drm/i915: HDMI pixel clock check

[Daniel Vetter]

On Fri, Aug 14, 2015 at 04:03:55PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 14, 2015 at 10:30:26AM +0200, Daniel Vetter wrote:
> > On Wed, Aug 12, 2015 at 09:34:54PM +0300, Ville Syrjälä wrote:
> > > On Fri, Jul 31, 2015 at 03:13:52PM +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 HDMI.
> > > > 
> > > > V2:
> > > > - removed computation for max dot clock
> > > > 
> > > > V3:
> > > > - cleanup by removing unnecessary lines
> > > > 
> > > > Signed-off-by: Mika Kahola <mika.kahola at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_hdmi.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > > > index 70bad5b..3149e5f 100644
> > > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > > @@ -1193,10 +1193,14 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
> > > >  	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
> > > >  	enum drm_mode_status status;
> > > >  	int clock;
> > > > +	int max_pixclk = to_i915(connector->dev)->max_dotclk;
> > > >  
> > > >  	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> > > >  		return MODE_NO_DBLESCAN;
> > > >  
> > > > +	if (mode->clock > max_pixclk)
> > > > +		return MODE_CLOCK_HIGH;
> > > > +
> > > >  	clock = mode->clock;
> > > 
> > > I believe we should do something like this here:
> > > 
> > > 	clock = mode->clock;
> > > 
> > > 	if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == DRM_MODE_FLAG_3D_FRAME_PACKING)
> > > 		clock *= 2;
> > 
> > That's already done in drm_mode_set_crtcinfo, i915 uses the STEREO_DOUBLE
> > mode of that function.
> 
> Only for modesets. .mode_valid() only deals with the results of
> .get_modes() etc., so the timings won't have been doubled. And even then
> we only double the .crtc_ timings, which is where I previously gave up
> trying to use the same mode validation code during modeset and .fill_modes().
> I suppose we can make it happen if we really try, but in the meantime I
> think it's at least an improvement if we stop advertising modes we for
> sure can't use.

Oh right totally mixed things up, you're correct.
-Daniel

> 
> > -Daniel
> > 
> > > 
> > > 	if (clock > max_pixclk)
> > > 		return MODE_CLOCK_HIGH;
> > > 
> > > >  	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> > > >  		clock *= 2;
> > > 
> > > The stero handling should probably be in a separate patch, but in
> > > preparation for it you could already put the dotclk check between the
> > > clock= assigment and the DBCLK doubling (since DBLCLK only affects the
> > > port_clock and not pixel clock).
> > > 
> > > > -- 
> > > > 1.9.1
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx at lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel OTC
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch