diff mbox series

[2/7] drm/i915/display: Enable HDR on gen9 devices with MCA Lspcon

Message ID 20200327052357.22269-3-vipin.anand@intel.com (mailing list archive)
State New, archived
Headers show
Series Enable HDR on Gen9 devices with lspcon hdr capability | expand

Commit Message

Vipin Anand March 27, 2020, 5:23 a.m. UTC 
From: Uma Shankar <uma.shankar@intel.com>

Gen9 hardware supports HDMI2.0 through LSPCON chips.
Extending HDR support for MCA LSPCON based GEN9 devices.

SOC will drive LSPCON as DP and send HDR metadata as standard
DP SDP packets. LSPCON will be set to operate in PCON mode,
will receive the metadata and create Dynamic Range and
Mastering Infoframe (DRM packets) and send it to HDR capable
HDMI sink devices.

v2: Re-used hsw infoframe write implementation for HDR metadata
for LSPCON as per Ville's suggestion.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c   | 10 ++++++
 drivers/gpu/drm/i915/display/intel_lspcon.c | 35 +++++++++++++++------
 drivers/gpu/drm/i915/display/intel_lspcon.h |  5 ++-
 3 files changed, 39 insertions(+), 11 deletions(-)

Comments

Jani Nikula April 7, 2020, 12:34 p.m. UTC | #1 
On Fri, 27 Mar 2020, Vipin Anand <vipin.anand@intel.com> wrote:
> From: Uma Shankar <uma.shankar@intel.com>
>
> Gen9 hardware supports HDMI2.0 through LSPCON chips.
> Extending HDR support for MCA LSPCON based GEN9 devices.
>
> SOC will drive LSPCON as DP and send HDR metadata as standard
> DP SDP packets. LSPCON will be set to operate in PCON mode,
> will receive the metadata and create Dynamic Range and
> Mastering Infoframe (DRM packets) and send it to HDR capable
> HDMI sink devices.
>
> v2: Re-used hsw infoframe write implementation for HDR metadata
> for LSPCON as per Ville's suggestion.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c   | 10 ++++++
>  drivers/gpu/drm/i915/display/intel_lspcon.c | 35 +++++++++++++++------
>  drivers/gpu/drm/i915/display/intel_lspcon.h |  5 ++-
>  3 files changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 93ac0f296852..9ae2f88cc925 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -577,6 +577,16 @@ static u32 hsw_infoframes_enabled(struct intel_encoder *encoder,
>  	return val & mask;
>  }
>  
> +void lspcon_drm_write_infoframe(struct intel_encoder *encoder,
> +	const struct intel_crtc_state *crtc_state,
> +	unsigned int type,
> +	const void *frame, ssize_t len)
> +{
> +	DRM_DEBUG_KMS("Update HDR metadata for lspcon\n");
> +	/* It uses the legacy hsw implementation for the same */
> +	hsw_write_infoframe(encoder, crtc_state, type, frame, len);
> +}

This seems like an unnecessary wrapper.

> +
>  static const u8 infoframe_type_to_idx[] = {
>  	HDMI_PACKET_TYPE_GENERAL_CONTROL,
>  	HDMI_PACKET_TYPE_GAMUT_METADATA,
> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
> index 2e41ae483a23..c5ddabf903d6 100644
> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
> @@ -460,27 +460,42 @@ void lspcon_write_infoframe(struct intel_encoder *encoder,
>  			    unsigned int type,
>  			    const void *frame, ssize_t len)
>  {
> -	bool ret;
> +	bool ret = true;
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
>  
>  	/* LSPCON only needs AVI IF */
> -	if (type != HDMI_INFOFRAME_TYPE_AVI)
> +	if (!(type == HDMI_INFOFRAME_TYPE_AVI ||
> +	      type == HDMI_PACKET_TYPE_GAMUT_METADATA))
>  		return;
>  
> -	if (lspcon->vendor == LSPCON_VENDOR_MCA)
> -		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
> -						      frame, len);
> -	else
> -		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
> -							 frame, len);
> +	/*
> +	 * Supporting HDR on MCA LSPCON
> +	 * Todo: Add support for Parade later
> +	 */
> +	if (type == HDMI_PACKET_TYPE_GAMUT_METADATA &&
> +	    lspcon->vendor != LSPCON_VENDOR_MCA)
> +		return;
> +
> +	if (lspcon->vendor == LSPCON_VENDOR_MCA) {
> +		if (type == HDMI_INFOFRAME_TYPE_AVI)
> +			ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
> +							      frame, len);
> +		else if (type == HDMI_PACKET_TYPE_GAMUT_METADATA)
> +			lspcon_drm_write_infoframe(encoder, crtc_state,
> +						   HDMI_PACKET_TYPE_GAMUT_METADATA,
> +						   frame, VIDEO_DIP_DATA_SIZE);
> +	} else {
> +		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux, frame,
> +							 len);
> +	}

Seems to me it would be best to replace from the beginning of this
function with something like:

        switch (type) {
        case HDMI_INFOFRAME_TYPE_AVI:
                if (lspcon->vendor == LSPCON_VENDOR_MCA)
                        ...
                else
                        ...
                break;
        case HDMI_PACKET_TYPE_GAMUT_METADATA:
                if (lspcon->vendor == LSPCON_VENDOR_MCA)
                        ...
                else
                        break;
                break;
        default:
                return;
        }

Then it'll be straightforward to replace the actual vendor specific if
ladders with function pointers in struct intel_lspcon in the future.

>  
>  	if (!ret) {
> -		DRM_ERROR("Failed to write AVI infoframes\n");
> +		DRM_ERROR("Failed to write infoframes\n");
>  		return;
>  	}
>  
> -	DRM_DEBUG_DRIVER("AVI infoframes updated successfully\n");
> +	DRM_DEBUG_DRIVER("Infoframes updated successfully\n");
>  }
>  
>  void lspcon_read_infoframe(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.h b/drivers/gpu/drm/i915/display/intel_lspcon.h
> index 37cfddf8a9c5..b2051f236223 100644
> --- a/drivers/gpu/drm/i915/display/intel_lspcon.h
> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.h
> @@ -34,5 +34,8 @@ u32 lspcon_infoframes_enabled(struct intel_encoder *encoder,
>  			      const struct intel_crtc_state *pipe_config);
>  void lspcon_ycbcr420_config(struct drm_connector *connector,
>  			    struct intel_crtc_state *crtc_state);
> -
> +void lspcon_drm_write_infoframe(struct intel_encoder *encoder,
> +				const struct intel_crtc_state *crtc_state,
> +				unsigned int type,
> +				const void *frame, ssize_t len);
>  #endif /* __INTEL_LSPCON_H__ */
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 93ac0f296852..9ae2f88cc925 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -577,6 +577,16 @@  static u32 hsw_infoframes_enabled(struct intel_encoder *encoder,
 	return val & mask;
 }
 
+void lspcon_drm_write_infoframe(struct intel_encoder *encoder,
+	const struct intel_crtc_state *crtc_state,
+	unsigned int type,
+	const void *frame, ssize_t len)
+{
+	DRM_DEBUG_KMS("Update HDR metadata for lspcon\n");
+	/* It uses the legacy hsw implementation for the same */
+	hsw_write_infoframe(encoder, crtc_state, type, frame, len);
+}
+
 static const u8 infoframe_type_to_idx[] = {
 	HDMI_PACKET_TYPE_GENERAL_CONTROL,
 	HDMI_PACKET_TYPE_GAMUT_METADATA,
diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
index 2e41ae483a23..c5ddabf903d6 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
@@ -460,27 +460,42 @@  void lspcon_write_infoframe(struct intel_encoder *encoder,
 			    unsigned int type,
 			    const void *frame, ssize_t len)
 {
-	bool ret;
+	bool ret = true;
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
 
 	/* LSPCON only needs AVI IF */
-	if (type != HDMI_INFOFRAME_TYPE_AVI)
+	if (!(type == HDMI_INFOFRAME_TYPE_AVI ||
+	      type == HDMI_PACKET_TYPE_GAMUT_METADATA))
 		return;
 
-	if (lspcon->vendor == LSPCON_VENDOR_MCA)
-		ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
-						      frame, len);
-	else
-		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux,
-							 frame, len);
+	/*
+	 * Supporting HDR on MCA LSPCON
+	 * Todo: Add support for Parade later
+	 */
+	if (type == HDMI_PACKET_TYPE_GAMUT_METADATA &&
+	    lspcon->vendor != LSPCON_VENDOR_MCA)
+		return;
+
+	if (lspcon->vendor == LSPCON_VENDOR_MCA) {
+		if (type == HDMI_INFOFRAME_TYPE_AVI)
+			ret = _lspcon_write_avi_infoframe_mca(&intel_dp->aux,
+							      frame, len);
+		else if (type == HDMI_PACKET_TYPE_GAMUT_METADATA)
+			lspcon_drm_write_infoframe(encoder, crtc_state,
+						   HDMI_PACKET_TYPE_GAMUT_METADATA,
+						   frame, VIDEO_DIP_DATA_SIZE);
+	} else {
+		ret = _lspcon_write_avi_infoframe_parade(&intel_dp->aux, frame,
+							 len);
+	}
 
 	if (!ret) {
-		DRM_ERROR("Failed to write AVI infoframes\n");
+		DRM_ERROR("Failed to write infoframes\n");
 		return;
 	}
 
-	DRM_DEBUG_DRIVER("AVI infoframes updated successfully\n");
+	DRM_DEBUG_DRIVER("Infoframes updated successfully\n");
 }
 
 void lspcon_read_infoframe(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.h b/drivers/gpu/drm/i915/display/intel_lspcon.h
index 37cfddf8a9c5..b2051f236223 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.h
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.h
@@ -34,5 +34,8 @@  u32 lspcon_infoframes_enabled(struct intel_encoder *encoder,
 			      const struct intel_crtc_state *pipe_config);
 void lspcon_ycbcr420_config(struct drm_connector *connector,
 			    struct intel_crtc_state *crtc_state);
-
+void lspcon_drm_write_infoframe(struct intel_encoder *encoder,
+				const struct intel_crtc_state *crtc_state,
+				unsigned int type,
+				const void *frame, ssize_t len);
 #endif /* __INTEL_LSPCON_H__ */