Re: [PATCH v2 3/9] drm/i915: Enable default_phase in GCP when possible

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Jun 01, 2015 at 09:49:53PM +0000, Konduru, Chandra wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> > ville.syrjala@linux.intel.com
> > Sent: Tuesday, May 05, 2015 7:06 AM
> > To: intel-gfx@lists.freedesktop.org
> > Subject: [Intel-gfx] [PATCH v2 3/9] drm/i915: Enable default_phase in GCP when
> > possible
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > When the video timings are suitably aligned so that all different periods start at
> > phase 0 (ie. none of the periods start mid-pixel) we can inform the sink about
> > this. Supposedly the sink can then optimize certain things. Obviously this is only
> > relevant when outputting >8bpc data since otherwise there are no mid-pixel
> > phases.
> > 
> > v2: Rebased due to crtc->config changes
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 48
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 87c4905..2e98e33 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -560,6 +560,49 @@ static bool hdmi_sink_is_deep_color(struct
> > drm_encoder *encoder)
> >  	return false;
> >  }
> > 
> > +/*
> > + * Determine if default_phase=1 can be indicated in the GCP infoframe.
> > + *
> > + * From HDMI specification 1.4a:
> > + * - The first pixel of each Video Data Period shall always have a
> > +pixel packing phase of 0
> > + * - The first pixel following each Video Data Period shall have a
> > +pixel packing phase of 0
> > + * - The PP bits shall be constant for all GCPs and will be equal to
> > +the last packing phase
> > + * - The first pixel following every transition of HSYNC or VSYNC shall have a
> > pixel packing
> > + *   phase of 0
> > + */
> > +static bool gcp_default_phase_possible(int pipe_bpp,
> > +				       const struct drm_display_mode *mode) {
> > +	unsigned int pixels_per_group;
> > +
> > +	switch (pipe_bpp) {
> > +	case 30:
> > +		/* 4 pixels in 5 clocks */
> > +		pixels_per_group = 4;
> > +		break;
> > +	case 36:
> > +		/* 2 pixels in 3 clocks */
> > +		pixels_per_group = 2;
> > +		break;
> > +	case 48:
> > +		/* 1 pixel in 2 clocks */
> > +		pixels_per_group = 1;
> > +		break;
> > +	default:
> > +		/* phase information not relevant for 8bpc */
> > +		return false;
> > +	}
> > +
> > +	return mode->crtc_hdisplay % pixels_per_group == 0 &&
> > +		mode->crtc_htotal % pixels_per_group == 0 &&
> > +		mode->crtc_hblank_start % pixels_per_group == 0 &&
> > +		mode->crtc_hblank_end % pixels_per_group == 0 &&
> > +		mode->crtc_hsync_start % pixels_per_group == 0 &&
> > +		mode->crtc_hsync_end % pixels_per_group == 0 &&
> 
> For hsync, bspec says Hsync is an even number.
> Isn't it above check should be something like (hsync_end - hsync_start) % 2 == 0?
> And similarly for front & back porches, right? 

If X and Y are even then (X - Y) is even too. Also the text in bspec is
less informative than the text in HDMI spec, which is why I quited the
HDMI spec instead.

> 
> > +		((mode->flags & DRM_MODE_FLAG_INTERLACE) == 0 ||
> > +		 mode->crtc_htotal/2 % pixels_per_group == 0); }
> > +
> >  static bool intel_hdmi_set_gcp_infoframe(struct drm_encoder *encoder)  {
> >  	struct drm_i915_private *dev_priv = encoder->dev->dev_private; @@ -
> > 579,6 +622,11 @@ static bool intel_hdmi_set_gcp_infoframe(struct
> > drm_encoder *encoder)
> >  	if (hdmi_sink_is_deep_color(encoder))
> >  		val |= GCP_COLOR_INDICATION;
> > 
> > +	/* Enable default_phase whenever the display mode is suitably aligned
> > */
> > +	if (gcp_default_phase_possible(crtc->config->pipe_bpp,
> > +				       &crtc->config->base.adjusted_mode))
> > +		val |= GCP_DEFAULT_PHASE_ENABLE;
> > +
> >  	I915_WRITE(reg, val);
> > 
> >  	return val != 0;
> > --
> > 2.0.5
> > 
> > _______________________________________________
> > 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