Re: [PATCH v3 03/11] drm/i915: HDMI pixel clock check

From: Daniel Vetter <daniel@ffwll.ch>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v3 03/11] drm/i915: HDMI pixel clock check
Date: Fri, 14 Aug 2015 10:30:26 +0200	[thread overview]
Message-ID: <20150814083026.GK17734@phenom.ffwll.local> (raw)
In-Reply-To: <20150812183454.GH5176@intel.com>

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@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.
-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@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