From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 07/12] drm/i915/hdmi: Port the infoframe code to the common hdmi helpers Date: Wed, 7 Aug 2013 14:00:36 +0300 Message-ID: <20130807110036.GV5004@intel.com> References: <1375817544-14565-1-git-send-email-damien.lespiau@intel.com> <1375817544-14565-8-git-send-email-damien.lespiau@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <1375817544-14565-8-git-send-email-damien.lespiau@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: Damien Lespiau Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Aug 06, 2013 at 08:32:19PM +0100, Damien Lespiau wrote: > Let's use the drivers/video/hmdi.c and drm infoframe helpers to build > our infoframes. > = > v2: Simplify the logic to compute the buffer size. We can just take the > maximum infoframe size rounded to 32, which happens to be what the > hardware let us write anyway. > = > v3: Remove unnecessary memset() (Ville Syrj=E4l=E4) > = > Signed-off-by: Damien Lespiau Reviewed-by: Ville Syrj=E4l=E4 > --- > drivers/gpu/drm/i915/intel_hdmi.c | 82 +++++++++++++++++++++++++++------= ------ > 1 file changed, 58 insertions(+), 24 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/int= el_hdmi.c > index ee67e23..455dfa7 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -325,14 +325,43 @@ static void hsw_write_infoframe(struct drm_encoder = *encoder, > POSTING_READ(ctl_reg); > } > = > +/* > + * The data we write to the DIP data buffer registers is 1 byte bigger t= han the > + * HDMI infoframe size because of an ECC/reserved byte at position 3 (st= arting > + * at 0). It's also a byte used by DisplayPort so the same DIP registers= can be > + * used for both technologies. > + * > + * DW0: Reserved/ECC/DP | HB2 | HB1 | HB0 > + * DW1: DB3 | DB2 | DB1 | DB0 > + * DW2: DB7 | DB6 | DB5 | DB4 > + * DW3: ... > + * > + * (HB is Header Byte, DB is Data Byte) > + * > + * The hdmi pack() functions don't know about that hardware specific hol= e so we > + * trick them by giving an offset into the buffer and moving back the he= ader > + * bytes by one. > + */ > static void intel_set_infoframe(struct drm_encoder *encoder, > - struct dip_infoframe *frame) > + union hdmi_infoframe *frame) > { > struct intel_hdmi *intel_hdmi =3D enc_to_intel_hdmi(encoder); > + uint8_t buffer[VIDEO_DIP_DATA_SIZE]; > + ssize_t len; > = > - intel_dip_infoframe_csum(frame); > - intel_hdmi->write_infoframe(encoder, frame->type, (uint8_t *)frame, > - DIP_HEADER_SIZE + frame->len); > + /* see comment above for the reason for this offset */ > + len =3D hdmi_infoframe_pack(frame, buffer + 1, sizeof(buffer) - 1); > + if (len < 0) > + return; > + > + /* Insert the 'hole' (see big comment above) at position 3 */ > + buffer[0] =3D buffer[1]; > + buffer[1] =3D buffer[2]; > + buffer[2] =3D buffer[3]; > + buffer[3] =3D 0; > + len++; > + > + intel_hdmi->write_infoframe(encoder, frame->any.type, buffer, len); > } > = > static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, > @@ -340,40 +369,45 @@ static void intel_hdmi_set_avi_infoframe(struct drm= _encoder *encoder, > { > struct intel_hdmi *intel_hdmi =3D enc_to_intel_hdmi(encoder); > struct intel_crtc *intel_crtc =3D to_intel_crtc(encoder->crtc); > - struct dip_infoframe avi_if =3D { > - .type =3D DIP_TYPE_AVI, > - .ver =3D DIP_VERSION_AVI, > - .len =3D DIP_LEN_AVI, > - }; > + union hdmi_infoframe frame; > + int ret; > + > + ret =3D drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, > + adjusted_mode); > + if (ret < 0) { > + DRM_ERROR("couldn't fill AVI infoframe\n"); > + return; > + } > = > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > - avi_if.body.avi.YQ_CN_PR |=3D DIP_AVI_PR_2; > + frame.avi.pixel_repeat =3D 1; > = > if (intel_hdmi->rgb_quant_range_selectable) { > if (intel_crtc->config.limited_color_range) > - avi_if.body.avi.ITC_EC_Q_SC |=3D DIP_AVI_RGB_QUANT_RANGE_LIMITED; > + frame.avi.quantization_range =3D > + HDMI_QUANTIZATION_RANGE_LIMITED; > else > - avi_if.body.avi.ITC_EC_Q_SC |=3D DIP_AVI_RGB_QUANT_RANGE_FULL; > + frame.avi.quantization_range =3D > + HDMI_QUANTIZATION_RANGE_FULL; > } > = > - avi_if.body.avi.VIC =3D drm_match_cea_mode(adjusted_mode); > - > - intel_set_infoframe(encoder, &avi_if); > + intel_set_infoframe(encoder, &frame); > } > = > static void intel_hdmi_set_spd_infoframe(struct drm_encoder *encoder) > { > - struct dip_infoframe spd_if; > + union hdmi_infoframe frame; > + int ret; > + > + ret =3D hdmi_spd_infoframe_init(&frame.spd, "Intel", "Integrated gfx"); > + if (ret < 0) { > + DRM_ERROR("couldn't fill SPD infoframe\n"); > + return; > + } > = > - memset(&spd_if, 0, sizeof(spd_if)); > - spd_if.type =3D DIP_TYPE_SPD; > - spd_if.ver =3D DIP_VERSION_SPD; > - spd_if.len =3D DIP_LEN_SPD; > - strcpy(spd_if.body.spd.vn, "Intel"); > - strcpy(spd_if.body.spd.pd, "Integrated gfx"); > - spd_if.body.spd.sdi =3D DIP_SPD_PC; > + frame.spd.sdi =3D HDMI_SPD_SDI_PC; > = > - intel_set_infoframe(encoder, &spd_if); > + intel_set_infoframe(encoder, &frame); > } > = > static void g4x_set_infoframes(struct drm_encoder *encoder, > -- = > 1.8.3.1 > = > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- = Ville Syrj=E4l=E4 Intel OTC