RE: [v6 12/13] drm/i915: Set Infoframe for non modeset case for HDR

2019-04-02 Thread Shankar, Uma 


>-Original Message-
>From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of 
>Ville
>Syrjälä
>Sent: Friday, March 29, 2019 7:35 PM
>To: Shankar, Uma 
>Cc: Syrjala, Ville ; liviu.du...@arm.com; intel-
>g...@lists.freedesktop.org; emil.l.veli...@gmail.com; dri-
>de...@lists.freedesktop.org; Lankhorst, Maarten 
>Subject: Re: [v6 12/13] drm/i915: Set Infoframe for non modeset case for HDR
>
>On Wed, Mar 20, 2019 at 04:18:25PM +0530, Uma Shankar wrote:
>> HDR metadata requires a infoframe to be set. Due to fastset, full
>> modeset is not performed hence adding it to update_pipe to handle
>> that.
>>
>> Signed-off-by: Uma Shankar 
>> ---
>>  drivers/gpu/drm/i915/intel_ddi.c | 13 +
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> b/drivers/gpu/drm/i915/intel_ddi.c
>> index 69aa0d1..a27aab9 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -3566,6 +3566,10 @@ static void intel_ddi_update_pipe(struct intel_encoder
>*encoder,
>>const struct intel_crtc_state *crtc_state,
>>const struct drm_connector_state *conn_state) 
>>  {
>> +struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> +struct intel_digital_port *intel_dig_port =
>> +enc_to_dig_port(>base);
>> +
>>  if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>>  intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
>>
>> @@ -3575,6 +3579,15 @@ static void intel_ddi_update_pipe(struct intel_encoder
>*encoder,
>>  else if (conn_state->content_protection ==
>>   DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
>>  intel_hdcp_disable(to_intel_connector(conn_state->connector));
>> +
>> +/* Set the infoframe for NON modeset cases as well */
>> +if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
>> +if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
>> +conn_state->hdr_metadata_changed)
>> +intel_dig_port->set_infoframes(encoder,
>> +   
>> crtc_state->has_infoframe,
>> +   crtc_state, conn_state);
>
>I don't think this should be tied to the drm infoframe. It should be totally 
>generic. IIRC
>there is also some magic dance we may have to perform when updating infoframes
>with the port enabled. But the details escape right now.

Hi Ville,
I agree, currently limiting it to DRM Infoframes so that we don't break the 
legacy
usecase/non HDR cases. We will have to enable this for all kind of infoframes 
and
validate if things are fine.

Regards,
Uma Shankar

>> +}
>>  }
>>
>>  static void intel_ddi_set_fia_lane_count(struct intel_encoder
>> *encoder,
>> --
>> 1.9.1
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>--
>Ville Syrjälä
>Intel
>___
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v6 12/13] drm/i915: Set Infoframe for non modeset case for HDR

2019-03-29 Thread Ville Syrjälä 
On Wed, Mar 20, 2019 at 04:18:25PM +0530, Uma Shankar wrote:
> HDR metadata requires a infoframe to be set. Due to fastset,
> full modeset is not performed hence adding it to update_pipe
> to handle that.
> 
> Signed-off-by: Uma Shankar 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 69aa0d1..a27aab9 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3566,6 +3566,10 @@ static void intel_ddi_update_pipe(struct intel_encoder 
> *encoder,
> const struct intel_crtc_state *crtc_state,
> const struct drm_connector_state *conn_state)
>  {
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_digital_port *intel_dig_port =
> + enc_to_dig_port(>base);
> +
>   if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>   intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
>  
> @@ -3575,6 +3579,15 @@ static void intel_ddi_update_pipe(struct intel_encoder 
> *encoder,
>   else if (conn_state->content_protection ==
>DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
>   intel_hdcp_disable(to_intel_connector(conn_state->connector));
> +
> + /* Set the infoframe for NON modeset cases as well */
> + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> + if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
> + conn_state->hdr_metadata_changed)
> + intel_dig_port->set_infoframes(encoder,
> +
> crtc_state->has_infoframe,
> +crtc_state, conn_state);

I don't think this should be tied to the drm infoframe. It should
be totally generic. IIRC there is also some magic dance we may have
to perform when updating infoframes with the port enabled. But the
details escape right now.

> + }
>  }
>  
>  static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
> -- 
> 1.9.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [v6 12/13] drm/i915: Set Infoframe for non modeset case for HDR

2019-03-29 Thread Sharma, Shashank 

On 3/20/2019 4:18 PM, Uma Shankar wrote:

HDR metadata requires a infoframe to be set. Due to fastset,
full modeset is not performed hence adding it to update_pipe
to handle that.

Signed-off-by: Uma Shankar 
---
  drivers/gpu/drm/i915/intel_ddi.c | 13 +
  1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 69aa0d1..a27aab9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3566,6 +3566,10 @@ static void intel_ddi_update_pipe(struct intel_encoder 
*encoder,
  const struct intel_crtc_state *crtc_state,
  const struct drm_connector_state *conn_state)
  {
+   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+   struct intel_digital_port *intel_dig_port =
+   enc_to_dig_port(>base);
+
if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
  
@@ -3575,6 +3579,15 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,

else if (conn_state->content_protection ==
 DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
intel_hdcp_disable(to_intel_connector(conn_state->connector));
+
+   /* Set the infoframe for NON modeset cases as well */
+   if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
+   if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
+   conn_state->hdr_metadata_changed)
+   intel_dig_port->set_infoframes(encoder,
+  
crtc_state->has_infoframe,
+  crtc_state, conn_state);
+   }
  }
  

Looks good to me,  Please feel free to use:

Reviewed-by: Shashank Sharma 



  static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[v6 12/13] drm/i915: Set Infoframe for non modeset case for HDR

2019-03-20 Thread Uma Shankar 
HDR metadata requires a infoframe to be set. Due to fastset,
full modeset is not performed hence adding it to update_pipe
to handle that.

Signed-off-by: Uma Shankar 
---
 drivers/gpu/drm/i915/intel_ddi.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 69aa0d1..a27aab9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3566,6 +3566,10 @@ static void intel_ddi_update_pipe(struct intel_encoder 
*encoder,
  const struct intel_crtc_state *crtc_state,
  const struct drm_connector_state *conn_state)
 {
+   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+   struct intel_digital_port *intel_dig_port =
+   enc_to_dig_port(>base);
+
if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
 
@@ -3575,6 +3579,15 @@ static void intel_ddi_update_pipe(struct intel_encoder 
*encoder,
else if (conn_state->content_protection ==
 DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
intel_hdcp_disable(to_intel_connector(conn_state->connector));
+
+   /* Set the infoframe for NON modeset cases as well */
+   if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
+   if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) &&
+   conn_state->hdr_metadata_changed)
+   intel_dig_port->set_infoframes(encoder,
+  
crtc_state->has_infoframe,
+  crtc_state, conn_state);
+   }
 }
 
 static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel