From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 3/3] [VPG HSW-A] drm/i915: Populate all fields of AVI infoframe Date: Thu, 15 Aug 2013 10:43:10 +0300 Message-ID: <20130815074310.GE7159@intel.com> References: <1376542743-25845-1-git-send-email-vandana.kannan@intel.com> <1376542743-25845-3-git-send-email-vandana.kannan@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id B8132E7164 for ; Thu, 15 Aug 2013 00:43:28 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1376542743-25845-3-git-send-email-vandana.kannan@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: vandana.kannan@intel.com Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Thu, Aug 15, 2013 at 10:29:03AM +0530, vandana.kannan@intel.com wrote: > From: vkannan > = > Populate bar information, colorimetry, IT content, quantization range fie= lds > of AVI infoframe based on CEA 861-D spec. > = > Signed-off-by: Vandana Kannan > --- > drivers/gpu/drm/i915/intel_drv.h | 4 ++++ > drivers/gpu/drm/i915/intel_hdmi.c | 39 +++++++++++++++++++++++++++++++= ++++-- > 2 files changed, 41 insertions(+), 2 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/inte= l_drv.h > index 97df85d..e02c442 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -412,6 +412,10 @@ struct cxsr_latency { > #define DIP_AVI_RGB_QUANT_RANGE_DEFAULT (0 << 2) > #define DIP_AVI_RGB_QUANT_RANGE_LIMITED (1 << 2) > #define DIP_AVI_RGB_QUANT_RANGE_FULL (2 << 2) > +#define DIP_AVI_IT_CONTENT (1 << 7) > +#define DIP_AVI_BAR_BOTH (3 << 2) > +#define DIP_AVI_COLOR_ITU601 (1 << 6) > +#define DIP_AVI_COLOR_ITU709 (2 << 6) > = > #define DIP_TYPE_SPD 0x83 > #define DIP_VERSION_SPD 0x1 > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/int= el_hdmi.c > index 7cf6fc5..dbbc02b 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -340,9 +340,22 @@ static void intel_hdmi_set_avi_infoframe(struct drm_= encoder *encoder, > .type =3D DIP_TYPE_AVI, > .ver =3D DIP_VERSION_AVI, > .len =3D DIP_LEN_AVI, > + .body.avi.Y_A_B_S =3D 0, > .body.avi.C_M_R =3D 8, > + .body.avi.ITC_EC_Q_SC =3D 0, > + .body.avi.VIC =3D 0, > + .body.avi.YQ_CN_PR =3D 0, > + .body.avi.top_bar_end =3D 0, > + .body.avi.bottom_bar_start =3D 0, > + .body.avi.left_bar_end =3D 0, > + .body.avi.right_bar_start =3D 0, This is not needed. All the members not explicitly initialized are initialized to 0 implicitly. > }; > = > + /* Bar information */ > + avi_if.body.avi.Y_A_B_S |=3D DIP_AVI_BAR_BOTH; Now you're sending bars left/right/top/bottom=3D0 bars. AFAICS that would indicate that the entire picture is made of of the bottom and right bars, ie. no actual content in the picture. > + > + avi_if.body.avi.VIC =3D drm_match_cea_mode(adjusted_mode); > + > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > avi_if.body.avi.YQ_CN_PR |=3D DIP_AVI_PR_2; > = > @@ -351,10 +364,17 @@ static void intel_hdmi_set_avi_infoframe(struct drm= _encoder *encoder, > avi_if.body.avi.ITC_EC_Q_SC |=3D DIP_AVI_RGB_QUANT_RANGE_LIMITED; > else > avi_if.body.avi.ITC_EC_Q_SC |=3D DIP_AVI_RGB_QUANT_RANGE_FULL; > + } else { > + /* Set full range quantization for non-CEA modes > + and 640x480 */ > + if ((avi_if.body.avi.VIC =3D=3D 0) || (avi_if.body.avi.VIC =3D=3D 1)) > + avi_if.body.avi.ITC_EC_Q_SC |=3D > + DIP_AVI_RGB_QUANT_RANGE_FULL; > + else > + avi_if.body.avi.ITC_EC_Q_SC |=3D > + DIP_AVI_RGB_QUANT_RANGE_LIMITED; > } The spec says we shouldn't send the quantization range unless the QS bit in EDID CEA block is 1. Now you're sending it anyway. What's the deal? > = > - avi_if.body.avi.VIC =3D drm_match_cea_mode(adjusted_mode); > - > /*If picture aspect ratio (PAR) is set to custom value, then use that, > else if VIC > 1, then get PAR from CEA mode list, else, calculate > PAR based on resolution */ > @@ -377,6 +397,21 @@ static void intel_hdmi_set_avi_infoframe(struct drm_= encoder *encoder, > avi_if.body.avi.C_M_R |=3D HDMI_PICTURE_ASPECT_16_9 << 4; > } > = > + if (avi_if.body.avi.VIC) { > + /* colorimetry: Sections 5.1 and 5.2 of CEA 861-D spec */ > + if ((adjusted_mode->vdisplay =3D=3D 480) || > + (adjusted_mode->vdisplay =3D=3D 576) || > + (adjusted_mode->vdisplay =3D=3D 240) || > + (adjusted_mode->vdisplay =3D=3D 288)) { > + avi_if.body.avi.C_M_R |=3D DIP_AVI_COLOR_ITU601; > + } else if ((adjusted_mode->vdisplay =3D=3D 720) || > + (adjusted_mode->vdisplay =3D=3D 1080)) { > + avi_if.body.avi.C_M_R |=3D DIP_AVI_COLOR_ITU709; > + } > + } We're outputting RGB data only, so we should not set the colorimetry bits. When we decide to implement better colorspace support, we're again going to need some new properties to let the user set the colorimetry appropriately. > + > + avi_if.body.avi.ITC_EC_Q_SC |=3D DIP_AVI_IT_CONTENT; > + > intel_set_infoframe(encoder, &avi_if); > } > = > -- = > 1.7.9.5 > = > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- = Ville Syrj=E4l=E4 Intel OTC