Re: [PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP Header and DB for HDR Static Metadata

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

 



On Tue, 2019-08-27 at 01:14 +0530, Shankar, Uma wrote:
> > -----Original Message-----
> > From: Mun, Gwan-gyeong
> > Sent: Friday, August 23, 2019 3:23 PM
> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; Shankar, Uma <
> > uma.shankar@xxxxxxxxx>;
> > Sharma, Shashank <shashank.sharma@xxxxxxxxx>
> > Subject: [PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP
> > Header and DB for
> > HDR Static Metadata
> > 
> > Function intel_dp_setup_hdr_metadata_infoframe_sdp handles
> > Infoframe SDP
> > header and data block setup for HDR Static Metadata. It enables
> > writing of HDR
> > metadata infoframe SDP to panel. Support for HDR video was
> > introduced in
> > DisplayPort 1.4. It implements the CTA-861-G standard for transport
> > of static HDR
> > metadata. The HDR Metadata will be provided by userspace
> > compositors, based on
> > blending policies and passed to the driver through a blob property.
> > 
> > Because each of GEN11 and prior GEN11 have different register size
> > for HDR
> > Metadata Infoframe SDP packet, it adds and uses different register
> > size.
> > 
> > Setup Infoframe SDP header and data block in function
> > intel_dp_setup_hdr_metadata_infoframe_sdp for HDR Static Metadata
> > as per dp 1.4
> > spec and CTA-861-F spec.
> > As per DP 1.4 spec, 2.2.2.5 SDP Formats. It enables Dynamic Range
> > and Mastering
> > Infoframe for HDR content, which is defined in CTA-861-F spec.
> > According to DP 1.4 spec and CEA-861-F spec Table 5, in order to
> > transmit static HDR
> > metadata, we have to use Non-audio INFOFRAME SDP v1.3.
> > 
> > +--------------------------------+-------------------------------+
> > >      [ Packet Type Value ]     |       [ Packet Type ]         |
> > +--------------------------------+-------------------------------+
> > > 80h + Non-audio INFOFRAME Type | CEA-861-F Non-audio INFOFRAME |
> > +--------------------------------+-------------------------------+
> > >      [Transmission Timing]                                     |
> > +----------------------------------------------------------------+
> > > As per CEA-861-F for INFOFRAME, including CEA-861.3 within     |
> > > which Dynamic Range and Mastering INFOFRAME are defined        |
> > +----------------------------------------------------------------+
> > 
> > v2: Add a missed blank line after function declaration.
> > 
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c |  1 +
> > drivers/gpu/drm/i915/display/intel_dp.c  | 91
> > ++++++++++++++++++++++++
> > drivers/gpu/drm/i915/display/intel_dp.h  |  3 +
> > drivers/gpu/drm/i915/i915_reg.h          |  1 +
> > 4 files changed, 96 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 5c42b58c1c2f..9f5bea941bcd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3478,6 +3478,7 @@ static void intel_enable_ddi_dp(struct
> > intel_encoder
> > *encoder,
> > 	intel_edp_backlight_on(crtc_state, conn_state);
> > 	intel_psr_enable(intel_dp, crtc_state);
> > 	intel_dp_vsc_enable(intel_dp, crtc_state, conn_state);
> > +	intel_dp_hdr_metadata_enable(intel_dp, crtc_state, conn_state);
> > 	intel_edp_drrs_enable(intel_dp, crtc_state);
> > 
> > 	if (crtc_state->has_audio)
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 7218e159f55d..00da8221eaba 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4554,6 +4554,85 @@ intel_dp_setup_vsc_sdp(struct intel_dp
> > *intel_dp,
> > 			crtc_state, DP_SDP_VSC, &vsc_sdp,
> > sizeof(vsc_sdp));  }
> > 
> > +static int
> > +intel_dp_setup_hdr_metadata_infoframe_sdp(struct intel_dp
> > *intel_dp,
> > +					  const struct intel_crtc_state
> > *crtc_state,
> > +					  const struct
> > drm_connector_state
> 
> The return value is not handled, you may convert it as void.
> 
Okay, I'll remove the return values which is not handled from
intel_dp_setup_hdr_metadata_infoframe_sdp().

> > *conn_state) {
> > +	struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
> > >base.base.dev);
> > +	struct dp_sdp infoframe_sdp = {};
> > +	struct hdmi_drm_infoframe drm_infoframe = {};
> > +	const int infoframe_size = HDMI_INFOFRAME_HEADER_SIZE +
> > HDMI_DRM_INFOFRAME_SIZE;
> > +	unsigned char buf[HDMI_INFOFRAME_HEADER_SIZE +
> > HDMI_DRM_INFOFRAME_SIZE];
> > +	ssize_t len;
> > +	int ret;
> > +
> > +	ret = drm_hdmi_infoframe_set_hdr_metadata(&drm_infoframe,
> > conn_state);
> > +	if (ret) {
> > +		DRM_DEBUG_KMS("couldn't set HDR metadata in
> > infoframe\n");
> > +		return ret;
> > +	}
> > +
> > +	len = hdmi_drm_infoframe_pack_only(&drm_infoframe, buf,
> > sizeof(buf));
> > +	if (len < 0) {
> > +		DRM_DEBUG_KMS("buffer size is smaller than hdr metadata
> > infoframe\n");
> > +		return (int)len;
> 
> If made void, this will not be required.
> 
> > +	}
> > +
> > +	if (len != infoframe_size) {
> > +		DRM_DEBUG_KMS("wrong static hdr metadata size\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/*
> > +	 * Set up the infoframe sdp packet for HDR static metadata.
> > +	 * Prepare VSC Header for SU as per DP 1.4a spec,
> > +	 * Table 2-100 and Table 2-101
> > +	 */
> > +
> > +	/* Packet ID, 00h for non-Audio INFOFRAME */
> > +	infoframe_sdp.sdp_header.HB0 = 0;
> > +	/*
> > +	 * Packet Type 80h + Non-audio INFOFRAME Type value
> > +	 * HDMI_INFOFRAME_TYPE_DRM: 0x87,
> > +	 */
> > +	infoframe_sdp.sdp_header.HB1 = drm_infoframe.type;
> > +	/*
> > +	 * Least Significant Eight Bits of (Data Byte Count – 1)
> > +	 * infoframe_size - 1,
> > +	 */
> > +	infoframe_sdp.sdp_header.HB2 = 0x1D;
> > +	/* INFOFRAME SDP Version Number */
> > +	infoframe_sdp.sdp_header.HB3 = (0x13 << 2);
> > +	/* CTA Header Byte 2 (INFOFRAME Version Number) */
> > +	infoframe_sdp.db[0] = drm_infoframe.version;
> > +	/* CTA Header Byte 3 (Length of INFOFRAME):
> > HDMI_DRM_INFOFRAME_SIZE
> > */
> > +	infoframe_sdp.db[1] = drm_infoframe.length;
> > +	/*
> > +	 * Copy HDMI_DRM_INFOFRAME_SIZE size from a buffer after
> > +	 * HDMI_INFOFRAME_HEADER_SIZE
> > +	 */
> > +	memcpy(&infoframe_sdp.db[2], &buf[HDMI_INFOFRAME_HEADER_SIZE],
> > +	       HDMI_DRM_INFOFRAME_SIZE);
> > +
> > +	if (INTEL_GEN(dev_priv) >= 11)
> > +		intel_dig_port->write_infoframe(&intel_dig_port->base,
> > +						crtc_state,
> > +
> > 	HDMI_PACKET_TYPE_GAMUT_METADATA,
> > +						&infoframe_sdp,
> > +						VIDEO_DIP_GMP_DATA_SIZE
> > );
> 
> This new VIDEO_DIP_GMP_DATA_SIZE doesn't seem to be handled in
> hsw_write_infoframe
> (hsw_dip_data_size). Can you please check this.
> 
Okay, I'll add missed handling of VIDEO_DIP_GMP_DATA_SIZE on
hsw_dip_data_size().
> > +	else
> > +		/* Prior to GEN11, Header size: 4 bytes, Data size: 28
> > bytes */
> > +		intel_dig_port->write_infoframe(&intel_dig_port->base,
> > +						crtc_state,
> > +
> > 	HDMI_PACKET_TYPE_GAMUT_METADATA,
> > +						&infoframe_sdp,
> > +						VIDEO_DIP_DATA_SIZE);
> > +
> 
> Also can you update the series to handle state checking also for
> metadata sent to DP sink.
> 
Does it mean a similar logic of "intel_read_infoframe(encoder,
pipe_config,  HDMI_INFOFRAME_TYPE_DRM, &pipe_config->infoframes.drm);"

if then, because current DP packet related implementation of i915 does
not hold DP packets (ex. VSC SDP, HDR Metadata Infoframe SDP...) to the
pipe state structure. In my opinion, as a diffenent series, first, it
would be better to add a storing of infoframe packets to pipe_state
structure for later comparing call (ex.  intel_pipe_config_compare() ).
after then we can compare them.

> > +	return 0;
> > +}
> > +
> > void intel_dp_vsc_enable(struct intel_dp *intel_dp,
> > 			 const struct intel_crtc_state *crtc_state,
> > 			 const struct drm_connector_state *conn_state)
> > @@ -
> > 4565,6 +4644,18 @@ void intel_dp_vsc_enable(struct intel_dp
> > *intel_dp,
> > 	intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state);  }
> > 
> > +void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
> > +				  const struct intel_crtc_state
> > *crtc_state,
> > +				  const struct drm_connector_state
> > *conn_state) {
> > +	if (!conn_state->hdr_output_metadata)
> > +		return;
> > +
> > +	intel_dp_setup_hdr_metadata_infoframe_sdp(intel_dp,
> > +						  crtc_state,
> > +						  conn_state);
> > +}
> > +
> > static u8 intel_dp_autotest_link_training(struct intel_dp
> > *intel_dp)  {
> > 	int status = 0;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> > b/drivers/gpu/drm/i915/display/intel_dp.h
> > index b2da7c9998f7..c3593691dd38 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -115,6 +115,9 @@ bool intel_dp_needs_vsc_colorimetry(u32
> > colorspace);  void
> > intel_dp_vsc_enable(struct intel_dp *intel_dp,
> > 			 const struct intel_crtc_state *crtc_state,
> > 			 const struct drm_connector_state *conn_state);
> > +void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
> > +				  const struct intel_crtc_state
> > *crtc_state,
> > +				  const struct drm_connector_state
> > *conn_state);
> > bool intel_digital_port_connected(struct intel_encoder *encoder);
> > 
> > static inline unsigned int intel_dp_unused_lane_mask(int
> > lane_count) diff --git
> > a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index
> > ea2f0fa2402d..92885416d539 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4645,6 +4645,7 @@ enum {
> >  * (Haswell and newer) to see which VIDEO_DIP_DATA byte corresponds
> > to each byte
> >  * of the infoframe structure specified by CEA-861. */
> > #define   VIDEO_DIP_DATA_SIZE	32
> > +#define   VIDEO_DIP_GMP_DATA_SIZE	36
> > #define   VIDEO_DIP_VSC_DATA_SIZE	36
> > #define   VIDEO_DIP_PPS_DATA_SIZE	132
> > #define VIDEO_DIP_CTL		_MMIO(0x61170)
> > --
> > 2.22.0
_______________________________________________
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