-----Original Message----- From: Mun, Gwan-gyeong Sent: Tuesday, September 3, 2019 10:54 AM To: Shankar, Uma uma.shankar@intel.com; intel-gfx@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org; Sharma, Shashank shashank.sharma@intel.com Subject: Re: [PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP Header and DB for HDR Static Metadata
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@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org; Shankar, Uma < uma.shankar@intel.com>; Sharma, Shashank shashank.sharma@intel.com 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@intel.com
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 structdrm_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 ininfoframe\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 metadatainfoframe\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: 28bytes */
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.
Yeah that’s the idea. We should be having some kind of state checking for DP SDP data as well. However, we can get this series in and later work on the DP state checking as a separate activity.
- 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