Re: [PATCH v3 06/17] drm/i915/dp: Read out DP SDPs (Secondary Data Packet)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2020-02-05 at 21:59 +0530, Shankar, Uma wrote:
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf
> > Of Gwan-
> > gyeong Mun
> > Sent: Tuesday, February 4, 2020 4:50 AM
> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Cc: linux-fbdev@xxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > Subject: [PATCH v3 06/17] drm/i915/dp: Read out DP SDPs (Secondary
> > Data Packet)
> 
> Drop the content in bracket.
> 
> > It adds code to read the DP SDPs from the video DIP and unpack them
> > into the crtc
> > state.
> > 
> > It adds routines that read out DP VSC SDP and DP HDR Metadata
> > Infoframe SDP In
> > order to unpack DP VSC SDP, it adds intel_dp_vsc_sdp_unpack()
> > function.
> > It follows DP 1.4a spec. [Table 2-116: VSC SDP Header Bytes] and
> > [Table 2-117: VSC
> > SDP Payload for DB16 through DB18]
> > 
> > In order to unpack DP HDR Metadata Infoframe SDP, it adds
> > intel_dp_hdr_metadata_infoframe_sdp_unpack(). And it follows DP
> > 1.4a spec.
> > ([Table 2-125: INFOFRAME SDP v1.2 Header Bytes] and [Table 2-126:
> > INFOFRAME
> > SDP v1.2 Payload Data Bytes - DB0 through DB31]) and CTA-861-G
> > spec. [Table-42
> > Dynamic Range and Mastering InfoFrame].
> > 
> > A nameing rule and style of intel_read_dp_sdp() function references
> 
> Typo in naming.
> 
> > intel_read_infoframe() function of intel_hdmi.c
> > 
> > v2: Minor style fix
> > v3: Replace a structure name to drm_dp_vsc_sdp from
> > intel_dp_vsc_sdp
> > 
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 170
> > ++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_dp.h |   3 +
> >  2 files changed, 173 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index dd7e5588001e..d4ece0a824c0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4925,6 +4925,176 @@ void intel_dp_set_infoframes(struct
> > intel_encoder
> > *encoder,
> >  	intel_write_dp_sdp(encoder, crtc_state,
> > HDMI_PACKET_TYPE_GAMUT_METADATA);  }
> > 
> > +static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc,
> > +				   const void *buffer, size_t size) {
> > +	const struct dp_sdp *sdp = buffer;
> > +
> > +	if (size < sizeof(struct dp_sdp))
> > +		return -EINVAL;
> > +
> > +	memset(vsc, 0, size);
> > +
> > +	if (sdp->sdp_header.HB0 != 0)
> > +		return -EINVAL;
> > +
> > +	if (sdp->sdp_header.HB1 != DP_SDP_VSC)
> > +		return -EINVAL;
> > +	vsc->sdp_type = sdp->sdp_header.HB1;
> > +
> > +	if (sdp->sdp_header.HB2 == 0x2 && sdp->sdp_header.HB3 == 0x8) {
> > +		vsc->revision = sdp->sdp_header.HB2;
> > +		vsc->length = sdp->sdp_header.HB3;
> > +	} else if (sdp->sdp_header.HB2 == 0x4 && sdp->sdp_header.HB3 ==
> > 0xe) {
> > +		vsc->revision = sdp->sdp_header.HB2;
> > +		vsc->length = sdp->sdp_header.HB3;
> > +	} else if (sdp->sdp_header.HB2 == 0x5 && sdp->sdp_header.HB3 ==
> > 0x13) {
> > +		vsc->revision = sdp->sdp_header.HB2;
> > +		vsc->length = sdp->sdp_header.HB3;
> 
> The above 2 lines can be done unconditionally, may be combine the if
> checks.
> 
> > +		vsc->colorspace = (sdp->db[16] >> 4) & 0xf;
> > +		vsc->colorimetry = sdp->db[16] & 0xf;
> > +		vsc->dynamic_range = (sdp->db[17] >> 7) & 0x1;
> > +
> > +		switch (sdp->db[17] & 0x7) {
> > +		case 0x1:
> > +			vsc->bpc = 8;
> > +			break;
> > +		case 0x2:
> > +			vsc->bpc = 10;
> > +			break;
> > +		case 0x3:
> > +			vsc->bpc = 12;
> > +			break;
> > +		case 0x4:
> > +			vsc->bpc = 16;
> > +			break;
> > +		default:
> > +			MISSING_CASE(sdp->db[17] & 0x7);
> 
> Handle 6bpc case as well.
> 
Yes,  I'll update everything that you commented.
> > +			return -EINVAL;
> > +		}
> > +
> > +		vsc->content_type = sdp->db[18] & 0x7;
> > +	} else {
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int
> > +intel_dp_hdr_metadata_infoframe_sdp_unpack(struct
> > hdmi_drm_infoframe
> > *drm_infoframe,
> > +					   const void *buffer, size_t
> > size) {
> > +	int ret;
> > +
> > +	const struct dp_sdp *sdp = buffer;
> > +
> > +	if (size < sizeof(struct dp_sdp))
> > +		return -EINVAL;
> > +
> > +	if (sdp->sdp_header.HB0 != 0)
> > +		return -EINVAL;
> > +
> > +	if (sdp->sdp_header.HB1 != HDMI_INFOFRAME_TYPE_DRM)
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * Least Significant Eight Bits of (Data Byte Count – 1)
> > +	 * 1Dh (i.e., Data Byte Count = 30 bytes).
> > +	 */
> > +	if (sdp->sdp_header.HB2 != 0x1D)
> > +		return -EINVAL;
> > +
> > +	/* Most Significant Two Bits of (Data Byte Count – 1), Clear to
> > 00b. */
> > +	if ((sdp->sdp_header.HB3 & 0x3) != 0)
> > +		return -EINVAL;
> > +
> > +	/* INFOFRAME SDP Version Number */
> > +	if (((sdp->sdp_header.HB3 >> 2) & 0x3f) != 0x13)
> > +		return -EINVAL;
> > +
> > +	/* CTA Header Byte 2 (INFOFRAME Version Number) */
> > +	if (sdp->db[0] != 1)
> > +		return -EINVAL;
> > +
> > +	/* CTA Header Byte 3 (Length of INFOFRAME):
> > HDMI_DRM_INFOFRAME_SIZE */
> > +	if (sdp->db[1] != HDMI_DRM_INFOFRAME_SIZE)
> > +		return -EINVAL;
> > +
> > +	ret = hdmi_drm_infoframe_unpack_only(drm_infoframe, &sdp-
> > >db[2],
> > +					     HDMI_DRM_INFOFRAME_SIZE);
> > +
> > +	return ret;
> > +}
> > +
> > +static void intel_read_dp_vsc_sdp(struct intel_encoder *encoder,
> > +				  struct intel_crtc_state *crtc_state,
> > +				  struct drm_dp_vsc_sdp *vsc)
> > +{
> > +	struct intel_digital_port *intel_dig_port =
> > enc_to_dig_port(encoder);
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +	unsigned int type = DP_SDP_VSC;
> > +	struct dp_sdp sdp = {};
> > +	int ret;
> > +
> > +	/* When PSR is enabled, VSC SDP is handled by PSR routine */
> > +	if (intel_psr_enabled(intel_dp))
> > +		return;
> > +
> > +	if ((crtc_state->infoframes.enable &
> > +	     intel_hdmi_infoframe_enable(type)) == 0)
> > +		return;
> > +
> > +	intel_dig_port->read_infoframe(encoder, crtc_state, type, &sdp,
> > +sizeof(sdp));
> > +
> > +	ret = intel_dp_vsc_sdp_unpack(vsc, &sdp, sizeof(sdp));
> > +
> > +	if (ret)
> > +		DRM_DEBUG_KMS("Failed to unpack DP VSC SDP\n"); }
> > +
> > +static void intel_read_dp_hdr_metadata_infoframe_sdp(struct
> > intel_encoder
> > *encoder,
> > +						     struct
> > intel_crtc_state
> > *crtc_state,
> > +						     struct
> > hdmi_drm_infoframe
> > *drm_infoframe) {
> > +	struct intel_digital_port *intel_dig_port =
> > enc_to_dig_port(encoder);
> > +	unsigned int type = HDMI_PACKET_TYPE_GAMUT_METADATA;
> > +	struct dp_sdp sdp = {};
> > +	int ret;
> > +
> > +	if ((crtc_state->infoframes.enable &
> > +	    intel_hdmi_infoframe_enable(type)) == 0)
> > +		return;
> > +
> > +	intel_dig_port->read_infoframe(encoder, crtc_state, type, &sdp,
> > +				       sizeof(sdp));
> > +
> > +	ret = intel_dp_hdr_metadata_infoframe_sdp_unpack(drm_infoframe,
> > &sdp,
> > +							 sizeof(sdp));
> > +
> > +	if (ret)
> > +		DRM_DEBUG_KMS("Failed to unpack DP HDR Metadata
> > Infoframe
> > SDP\n"); }
> > +
> > +void intel_read_dp_sdp(struct intel_encoder *encoder,
> > +		       struct intel_crtc_state *crtc_state,
> > +		       unsigned int type)
> > +{
> > +	switch (type) {
> > +	case DP_SDP_VSC:
> > +		intel_read_dp_vsc_sdp(encoder, crtc_state,
> > +				      &crtc_state->infoframes.vsc);
> > +		break;
> > +	case HDMI_PACKET_TYPE_GAMUT_METADATA:
> > +		intel_read_dp_hdr_metadata_infoframe_sdp(encoder,
> > crtc_state,
> > +							 &crtc_state-
> > > infoframes.drm.drm);
> > +		break;
> > +	default:
> > +		MISSING_CASE(type);
> > +		break;
> > +	}
> > +}
> > +
> >  static void
> >  intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
> >  		       const struct intel_crtc_state *crtc_state, diff
> > --git
> > a/drivers/gpu/drm/i915/display/intel_dp.h
> > b/drivers/gpu/drm/i915/display/intel_dp.h
> > index 0dc09a463ee1..e8f9ba962d09 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -119,6 +119,9 @@ void intel_dp_hdr_metadata_enable(struct
> > intel_dp
> > *intel_dp,  void intel_dp_set_infoframes(struct intel_encoder
> > *encoder, bool
> > enable,
> >  			     const struct intel_crtc_state *crtc_state,
> >  			     const struct drm_connector_state
> > *conn_state);
> > +void intel_read_dp_sdp(struct intel_encoder *encoder,
> > +		       struct intel_crtc_state *crtc_state,
> > +		       unsigned int type);
> >  bool intel_digital_port_connected(struct intel_encoder *encoder);
> > 
> >  static inline unsigned int intel_dp_unused_lane_mask(int
> > lane_count)
> > --
> > 2.24.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux