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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Sep 16, 2019 at 10:11:49AM +0300, Gwan-gyeong Mun wrote:
> 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.
> v3: Remove not handled return values from
>     intel_dp_setup_hdr_metadata_infoframe_sdp(). [Uma]
> 
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c |  1 +
>  drivers/gpu/drm/i915/display/intel_dp.c  | 89 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dp.h  |  3 +
>  3 files changed, 93 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 8dc030650801..306f6f9f0204 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3625,6 +3625,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 7fe22b37474d..abbf1d5c54c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4599,6 +4599,83 @@ intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
>  			crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp));
>  }
>  
> +static void
> +intel_dp_setup_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp,
> +					  const struct intel_crtc_state *crtc_state,
> +					  const struct drm_connector_state *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;
> +	}
> +
> +	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;
> +	}
> +
> +	if (len != infoframe_size) {
> +		DRM_DEBUG_KMS("wrong static hdr metadata size\n");
> +		return;
> +	}
> +
> +	/*
> +	 * 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);
> +	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);

This looks suspicious. Why is this not just something like sizeof(sdp)?

> +}
> +
>  void intel_dp_vsc_enable(struct intel_dp *intel_dp,
>  			 const struct intel_crtc_state *crtc_state,
>  			 const struct drm_connector_state *conn_state)
> @@ -4609,6 +4686,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 87883d0d5977..5613073d1dd5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -116,6 +116,9 @@ bool intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state);
>  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)
> -- 
> 2.23.0

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel