From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 5/8] drm/i915/hdmi: Port the infoframe code to the common hdmi helpers Date: Mon, 5 Aug 2013 16:12:11 +0300 Message-ID: <20130805131210.GT5004@intel.com> References: <1375464180-7259-1-git-send-email-damien.lespiau@intel.com> <1375464180-7259-6-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: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id F023FE6E59 for ; Mon, 5 Aug 2013 06:12:35 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1375464180-7259-6-git-send-email-damien.lespiau@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Damien Lespiau Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Aug 02, 2013 at 06:22:57PM +0100, Damien Lespiau wrote: > Let's use the drivers/video/hmdi.c and drm infoframe helpers to build > our infoframes. > = > Signed-off-by: Damien Lespiau > --- > drivers/gpu/drm/i915/intel_hdmi.c | 94 +++++++++++++++++++++++++++++----= ------ > 1 file changed, 70 insertions(+), 24 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/int= el_hdmi.c > index d3225df..57dd413 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -328,14 +328,55 @@ 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. > + * > + * The other constraint is that we write data to a 32 bits register, so = we > + * need to round the buffer up to the next multiple of 4, if not already= one. > + */ > +#define INFOFRAME_BUFFER_SIZE(type) \ > + (ALIGN(HDMI_INFOFRAME_SIZE(type) + 1, 4)) > + > +/* now the packing buffer needs to be able to hold the largest infoframe= */ > +#define DIP_BUFFER_SIZE \ > + (max(INFOFRAME_BUFFER_SIZE(AVI), INFOFRAME_BUFFER_SIZE(SPD))) > + > 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[DIP_BUFFER_SIZE]; > + ssize_t len; > + > + memset(buffer, 0, sizeof(buffer)); Could just zero initialize 'buffer' when declaring it. However hdmi_*_infoframe_pack() already contain a memset(), so we shouldn't even need to zero it here. > + > + /* see comment above for the reason for this offset */ > + len =3D hdmi_infoframe_pack(frame, buffer + 1, sizeof(buffer) - 1); > + if (len < 0) > + return; > = > - intel_dip_infoframe_csum(frame); > - intel_hdmi->write_infoframe(encoder, frame->type, (uint8_t *)frame, > - DIP_HEADER_SIZE + frame->len); > + /* 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, > @@ -343,40 +384,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; Perhaps that should be part of drm_hdmi_avi_infoframe_from_display_mode()? > = > 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 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC