[PATCH 0/7] drm/i915: Make infoframe code available to (e)DP ports

[PATCH 0/7] drm/i915: Make infoframe code available to (e)DP ports

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The video DIP can be used for DP SDP transmission, so I figured we should
move the vfuncs into the intel_digital_port structure. This migth help
with the LSPCON work, and it will also allow us to rip out the duplicated
video DIP routine from the PSR code.

The last patch we don't want to merge just yet. The state checker would get
angry at the VSC DIP being enabled even though the pipe config didn't indicate
that infoframes are used. I think we'll want to convert the has_infoframe into
a bitmask and then do something about the VSC DIP. Not quite sure yet.

Entire series available here:
git://github.com/vsyrjala/linux.git infoframe_dig_port_2

Ville Syrjälä (7):
  drm/dp: Add defines for DP SDP types
  drm/i915: Check has_infoframes when enabling infoframes
  drm/i915: Disable infoframes when shutting down DDI HDMI
  drm/i915: Move infoframe vfuncs into intel_digital_port
  drm/i915: Init infoframe vfuncs for DP encoders as well
  drm/i915: Remove mostly duplicated video DIP handling from PSR code
  drm/i915: Allow DP ports to set/readout infoframe state (WIP)

 drivers/gpu/drm/i915/intel_ddi.c  |  31 +++++++----
 drivers/gpu/drm/i915/intel_dp.c   |  12 +++++
 drivers/gpu/drm/i915/intel_drv.h  |  17 +++---
 drivers/gpu/drm/i915/intel_hdmi.c | 111 +++++++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_psr.c  |  41 +++-----------
 include/drm/drm_dp_helper.h       |   8 +++
 6 files changed, 122 insertions(+), 98 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/7] drm/dp: Add defines for DP SDP types

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add defines for the secondary data packet (SDP) types from the spec.
These are the DP specific ones, and in addition HDMI infoframe types
(see enum hdmi_infoframe_type) are also valid SDP types.

Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/drm/drm_dp_helper.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 5a848e734422..9ae1aa5f6713 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -630,6 +630,14 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]);
 u8 drm_dp_link_rate_to_bw_code(int link_rate);
 int drm_dp_bw_code_to_link_rate(u8 link_bw);
 
+#define DP_SDP_AUDIO_TIMESTAMP		0x01
+#define DP_SDP_AUDIO_STREAM		0x02
+#define DP_SDP_EXTENSION		0x04
+#define DP_SDP_AUDIO_COPYMANAGEMENT	0x05
+#define DP_SDP_ISRC			0x06
+#define DP_SDP_VSC			0x07
+#define DP_SDP_CAMERA_GENERIC(i)	(0x08 + (i)) /* 0x08-0x0f */
+
 struct edp_sdp_header {
 	u8 HB0; /* Secondary Data Packet ID */
 	u8 HB1; /* Secondary Data Packet Type */
-- 
2.7.4

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

[PATCH 2/7] drm/i915: Check has_infoframes when enabling infoframes

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

has_infoframe is what tells us whether infoframes should be enabled, so
let's pass that instead of has_hdmi_sink to .set_infoframes().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  | 2 +-
 drivers/gpu/drm/i915/intel_hdmi.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 022b41d422dc..2fb28d310c22 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1631,7 +1631,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 
 		intel_hdmi->set_infoframes(encoder,
-					   crtc->config->has_hdmi_sink,
+					   crtc->config->has_infoframe,
 					   &crtc->config->base.adjusted_mode);
 	}
 }
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index eb455ea6ea92..067b10a7cb04 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1665,7 +1665,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
 	intel_hdmi_prepare(encoder);
 
 	intel_hdmi->set_infoframes(&encoder->base,
-				   intel_crtc->config->has_hdmi_sink,
+				   intel_crtc->config->has_infoframe,
 				   adjusted_mode);
 }
 
@@ -1686,7 +1686,7 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
 				 0x2b247878);
 
 	intel_hdmi->set_infoframes(&encoder->base,
-				   intel_crtc->config->has_hdmi_sink,
+				   intel_crtc->config->has_infoframe,
 				   adjusted_mode);
 
 	g4x_enable_hdmi(encoder);
@@ -1749,7 +1749,7 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
 	chv_set_phy_signal_level(encoder, 128, 102, false);
 
 	intel_hdmi->set_infoframes(&encoder->base,
-				   intel_crtc->config->has_hdmi_sink,
+				   intel_crtc->config->has_infoframe,
 				   adjusted_mode);
 
 	g4x_enable_hdmi(encoder);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915: Check has_infoframes when enabling infoframes

[Sharma, Shashank]

Regards
Shashank

On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> has_infoframe is what tells us whether infoframes should be enabled, so
> let's pass that instead of has_hdmi_sink to .set_infoframes().
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c  | 2 +-
>   drivers/gpu/drm/i915/intel_hdmi.c | 6 +++---
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 022b41d422dc..2fb28d310c22 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1631,7 +1631,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
>   		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>
>   		intel_hdmi->set_infoframes(encoder,
> -					   crtc->config->has_hdmi_sink,
> +					   crtc->config->has_infoframe,
I am a bit confused here, please correct me if I am not going in right 
direction, but what if its a DVI device which still needs video/AVI 
infoframes but can drop Audio IF ? Wont it make more sense to keep using 
has_hdmi_sink ?
>   					   &crtc->config->base.adjusted_mode);
>   	}
>   }
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index eb455ea6ea92..067b10a7cb04 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1665,7 +1665,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
>   	intel_hdmi_prepare(encoder);
>
>   	intel_hdmi->set_infoframes(&encoder->base,
> -				   intel_crtc->config->has_hdmi_sink,
> +				   intel_crtc->config->has_infoframe,
Same as above

Regards
Shashank
>   				   adjusted_mode);
>   }
>
> @@ -1686,7 +1686,7 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
>   				 0x2b247878);
>
>   	intel_hdmi->set_infoframes(&encoder->base,
> -				   intel_crtc->config->has_hdmi_sink,
> +				   intel_crtc->config->has_infoframe,
>   				   adjusted_mode);
>
>   	g4x_enable_hdmi(encoder);
> @@ -1749,7 +1749,7 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
>   	chv_set_phy_signal_level(encoder, 128, 102, false);
>
>   	intel_hdmi->set_infoframes(&encoder->base,
> -				   intel_crtc->config->has_hdmi_sink,
> +				   intel_crtc->config->has_infoframe,
>   				   adjusted_mode);
>
>   	g4x_enable_hdmi(encoder);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915: Check has_infoframes when enabling infoframes

[Ville Syrjälä]

On Mon, Jun 13, 2016 at 01:40:50PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > has_infoframe is what tells us whether infoframes should be enabled, so
> > let's pass that instead of has_hdmi_sink to .set_infoframes().
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_ddi.c  | 2 +-
> >   drivers/gpu/drm/i915/intel_hdmi.c | 6 +++---
> >   2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 022b41d422dc..2fb28d310c22 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1631,7 +1631,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> >   		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> >
> >   		intel_hdmi->set_infoframes(encoder,
> > -					   crtc->config->has_hdmi_sink,
> > +					   crtc->config->has_infoframe,
> I am a bit confused here, please correct me if I am not going in right 
> direction, but what if its a DVI device which still needs video/AVI 
> infoframes but can drop Audio IF ? Wont it make more sense to keep using 
> has_hdmi_sink ?

DVI doesn't do infoframes, so not sure what you're asking here. Also we
don't deal with audio infoframes here.

> >   					   &crtc->config->base.adjusted_mode);
> >   	}
> >   }
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index eb455ea6ea92..067b10a7cb04 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1665,7 +1665,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
> >   	intel_hdmi_prepare(encoder);
> >
> >   	intel_hdmi->set_infoframes(&encoder->base,
> > -				   intel_crtc->config->has_hdmi_sink,
> > +				   intel_crtc->config->has_infoframe,
> Same as above
> 
> Regards
> Shashank
> >   				   adjusted_mode);
> >   }
> >
> > @@ -1686,7 +1686,7 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
> >   				 0x2b247878);
> >
> >   	intel_hdmi->set_infoframes(&encoder->base,
> > -				   intel_crtc->config->has_hdmi_sink,
> > +				   intel_crtc->config->has_infoframe,
> >   				   adjusted_mode);
> >
> >   	g4x_enable_hdmi(encoder);
> > @@ -1749,7 +1749,7 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
> >   	chv_set_phy_signal_level(encoder, 128, 102, false);
> >
> >   	intel_hdmi->set_infoframes(&encoder->base,
> > -				   intel_crtc->config->has_hdmi_sink,
> > +				   intel_crtc->config->has_infoframe,
> >   				   adjusted_mode);
> >
> >   	g4x_enable_hdmi(encoder);
> >

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915: Check has_infoframes when enabling infoframes

[Sharma, Shashank]

Regards
Shashank

On 6/13/2016 5:54 PM, Ville Syrjälä wrote:
> On Mon, Jun 13, 2016 at 01:40:50PM +0530, Sharma, Shashank wrote:
>> Regards
>> Shashank
>>
>> On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> has_infoframe is what tells us whether infoframes should be enabled, so
>>> let's pass that instead of has_hdmi_sink to .set_infoframes().
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_ddi.c  | 2 +-
>>>    drivers/gpu/drm/i915/intel_hdmi.c | 6 +++---
>>>    2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>> index 022b41d422dc..2fb28d310c22 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -1631,7 +1631,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
>>>    		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>>>
>>>    		intel_hdmi->set_infoframes(encoder,
>>> -					   crtc->config->has_hdmi_sink,
>>> +					   crtc->config->has_infoframe,
>> I am a bit confused here, please correct me if I am not going in right
>> direction, but what if its a DVI device which still needs video/AVI
>> infoframes but can drop Audio IF ? Wont it make more sense to keep using
>> has_hdmi_sink ?
>
> DVI doesn't do infoframes, so not sure what you're asking here. Also we
> don't deal with audio infoframes here.
>
Sorry, I guess I was under the (wrong) assumption that DVI still needs 
information(like aspect ratio) in form of infoframes, only the audio 
info frames are skipped.
>>>    					   &crtc->config->base.adjusted_mode);
>>>    	}
>>>    }
>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>>> index eb455ea6ea92..067b10a7cb04 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>> @@ -1665,7 +1665,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
>>>    	intel_hdmi_prepare(encoder);
>>>
>>>    	intel_hdmi->set_infoframes(&encoder->base,
>>> -				   intel_crtc->config->has_hdmi_sink,
>>> +				   intel_crtc->config->has_infoframe,
>> Same as above
>>
>> Regards
>> Shashank
>>>    				   adjusted_mode);
>>>    }
>>>
>>> @@ -1686,7 +1686,7 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
>>>    				 0x2b247878);
>>>
>>>    	intel_hdmi->set_infoframes(&encoder->base,
>>> -				   intel_crtc->config->has_hdmi_sink,
>>> +				   intel_crtc->config->has_infoframe,
>>>    				   adjusted_mode);
>>>
>>>    	g4x_enable_hdmi(encoder);
>>> @@ -1749,7 +1749,7 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
>>>    	chv_set_phy_signal_level(encoder, 128, 102, false);
>>>
>>>    	intel_hdmi->set_infoframes(&encoder->base,
>>> -				   intel_crtc->config->has_hdmi_sink,
>>> +				   intel_crtc->config->has_infoframe,
>>>    				   adjusted_mode);
>>>
>>>    	g4x_enable_hdmi(encoder);
>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/7] drm/i915: Disable infoframes when shutting down DDI HDMI

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Disabling the video DIP when shutting the port down seems like a good
idea.

Bspec says:
"When disabling both the DIP port and DIP transmission,
 first disable the port and then disable DIP."
and
"Restriction : GCP is only supported with HDMI when the bits per color is
 not equal to 8. GCP must be enabled prior to enabling TRANS_DDI_FUNC_CTL
 for HDMI with bits per color not equal to 8 and disabled after disabling
 TRANS_DDI_FUNC_CTL"

So let's do it in the .post_disable() hook.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 2fb28d310c22..6ff2a7b97ca6 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1661,6 +1661,12 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 	if (wait)
 		intel_wait_ddi_buf_idle(dev_priv, port);
 
+	if (type == INTEL_OUTPUT_HDMI) {
+		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+
+		intel_hdmi->set_infoframes(encoder, false, NULL);
+	}
+
 	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Disable infoframes when shutting down DDI HDMI

[Sharma, Shashank]

Regards
Shashank

On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Disabling the video DIP when shutting the port down seems like a good
> idea.
>
> Bspec says:
> "When disabling both the DIP port and DIP transmission,
>   first disable the port and then disable DIP."
> and
> "Restriction : GCP is only supported with HDMI when the bits per color is
>   not equal to 8. GCP must be enabled prior to enabling TRANS_DDI_FUNC_CTL
>   for HDMI with bits per color not equal to 8 and disabled after disabling
>   TRANS_DDI_FUNC_CTL"
>
> So let's do it in the .post_disable() hook.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 2fb28d310c22..6ff2a7b97ca6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1661,6 +1661,12 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>   	if (wait)
>   		intel_wait_ddi_buf_idle(dev_priv, port);
>
> +	if (type == INTEL_OUTPUT_HDMI) {
> +		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> +
> +		intel_hdmi->set_infoframes(encoder, false, NULL);
I have seen an assert_hdmi_port_disabled in hsw_set_infoframes, it will 
cause assert.
> +	}
> +
>   	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
>   		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>   		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Disable infoframes when shutting down DDI HDMI

[Ville Syrjälä]

On Mon, Jun 13, 2016 at 03:36:19PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Disabling the video DIP when shutting the port down seems like a good
> > idea.
> >
> > Bspec says:
> > "When disabling both the DIP port and DIP transmission,
> >   first disable the port and then disable DIP."
> > and
> > "Restriction : GCP is only supported with HDMI when the bits per color is
> >   not equal to 8. GCP must be enabled prior to enabling TRANS_DDI_FUNC_CTL
> >   for HDMI with bits per color not equal to 8 and disabled after disabling
> >   TRANS_DDI_FUNC_CTL"
> >
> > So let's do it in the .post_disable() hook.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_ddi.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 2fb28d310c22..6ff2a7b97ca6 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1661,6 +1661,12 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> >   	if (wait)
> >   		intel_wait_ddi_buf_idle(dev_priv, port);
> >
> > +	if (type == INTEL_OUTPUT_HDMI) {
> > +		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> > +
> > +		intel_hdmi->set_infoframes(encoder, false, NULL);
> I have seen an assert_hdmi_port_disabled in hsw_set_infoframes, it will 
> cause assert.

No. We've already turned off the port by this time.

> > +	}
> > +
> >   	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
> >   		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >   		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> >

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Disable infoframes when shutting down DDI HDMI

[Sharma, Shashank]

Ok, apart from this, looks good to me.
Reviewed-by: Shashank Sharma

Regards
Shashank

On 6/13/2016 5:54 PM, Ville Syrjälä wrote:
> On Mon, Jun 13, 2016 at 03:36:19PM +0530, Sharma, Shashank wrote:
>> Regards
>> Shashank
>>
>> On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Disabling the video DIP when shutting the port down seems like a good
>>> idea.
>>>
>>> Bspec says:
>>> "When disabling both the DIP port and DIP transmission,
>>>    first disable the port and then disable DIP."
>>> and
>>> "Restriction : GCP is only supported with HDMI when the bits per color is
>>>    not equal to 8. GCP must be enabled prior to enabling TRANS_DDI_FUNC_CTL
>>>    for HDMI with bits per color not equal to 8 and disabled after disabling
>>>    TRANS_DDI_FUNC_CTL"
>>>
>>> So let's do it in the .post_disable() hook.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_ddi.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>> index 2fb28d310c22..6ff2a7b97ca6 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -1661,6 +1661,12 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>>>    	if (wait)
>>>    		intel_wait_ddi_buf_idle(dev_priv, port);
>>>
>>> +	if (type == INTEL_OUTPUT_HDMI) {
>>> +		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>>> +
>>> +		intel_hdmi->set_infoframes(encoder, false, NULL);
>> I have seen an assert_hdmi_port_disabled in hsw_set_infoframes, it will
>> cause assert.
>
> No. We've already turned off the port by this time.
>
>>> +	}
>>> +
>>>    	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
>>>    		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>>>    		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/7] drm/i915: Move infoframe vfuncs into intel_digital_port

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

DP ports will also want to utilize the video DIP for SDP transmission.
So let's move the vfuncs into the dig_port.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  | 20 ++++++------
 drivers/gpu/drm/i915/intel_drv.h  | 16 +++++-----
 drivers/gpu/drm/i915/intel_hdmi.c | 64 ++++++++++++++++++++-------------------
 3 files changed, 52 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 6ff2a7b97ca6..3a882a979e5d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1628,11 +1628,12 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 		if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9)
 			intel_dp_stop_link_train(intel_dp);
 	} else if (type == INTEL_OUTPUT_HDMI) {
-		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+		struct intel_digital_port *intel_dig_port =
+			enc_to_dig_port(encoder);
 
-		intel_hdmi->set_infoframes(encoder,
-					   crtc->config->has_infoframe,
-					   &crtc->config->base.adjusted_mode);
+		intel_dig_port->set_infoframes(encoder,
+					       crtc->config->has_infoframe,
+					       &crtc->config->base.adjusted_mode);
 	}
 }
 
@@ -1662,9 +1663,10 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 		intel_wait_ddi_buf_idle(dev_priv, port);
 
 	if (type == INTEL_OUTPUT_HDMI) {
-		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+		struct intel_digital_port *intel_dig_port =
+			enc_to_dig_port(encoder);
 
-		intel_hdmi->set_infoframes(encoder, false, NULL);
+		intel_dig_port->set_infoframes(encoder, false, NULL);
 	}
 
 	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
@@ -2155,7 +2157,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
 	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
-	struct intel_hdmi *intel_hdmi;
+	struct intel_digital_port *intel_dig_port;
 	u32 temp, flags = 0;
 
 	/* XXX: DSI transcoder paranoia */
@@ -2194,9 +2196,9 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
 	case TRANS_DDI_MODE_SELECT_HDMI:
 		pipe_config->has_hdmi_sink = true;
-		intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+		intel_dig_port = enc_to_dig_port(&encoder->base);
 
-		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
+		if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
 			pipe_config->has_infoframe = true;
 		/* fall through */
 	case TRANS_DDI_MODE_SELECT_DVI:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ebe7b3427e2e..a607799b7776 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -790,14 +790,6 @@ struct intel_hdmi {
 	bool rgb_quant_range_selectable;
 	enum hdmi_picture_aspect aspect_ratio;
 	struct intel_connector *attached_connector;
-	void (*write_infoframe)(struct drm_encoder *encoder,
-				enum hdmi_infoframe_type type,
-				const void *frame, ssize_t len);
-	void (*set_infoframes)(struct drm_encoder *encoder,
-			       bool enable,
-			       const struct drm_display_mode *adjusted_mode);
-	bool (*infoframe_enabled)(struct drm_encoder *encoder,
-				  const struct intel_crtc_state *pipe_config);
 };
 
 struct intel_dp_mst_encoder;
@@ -907,6 +899,14 @@ struct intel_digital_port {
 	uint8_t max_lanes;
 	/* for communication with audio component; protected by av_mutex */
 	const struct drm_connector *audio_connector;
+	void (*write_infoframe)(struct drm_encoder *encoder,
+				enum hdmi_infoframe_type type,
+				const void *frame, ssize_t len);
+	void (*set_infoframes)(struct drm_encoder *encoder,
+			       bool enable,
+			       const struct drm_display_mode *adjusted_mode);
+	bool (*infoframe_enabled)(struct drm_encoder *encoder,
+				  const struct intel_crtc_state *pipe_config);
 };
 
 struct intel_dp_mst_encoder {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 067b10a7cb04..319f5013923c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -427,7 +427,7 @@ static bool hsw_infoframe_enabled(struct drm_encoder *encoder,
 static void intel_write_infoframe(struct drm_encoder *encoder,
 				  union hdmi_infoframe *frame)
 {
-	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
 	uint8_t buffer[VIDEO_DIP_DATA_SIZE];
 	ssize_t len;
 
@@ -443,7 +443,7 @@ static void intel_write_infoframe(struct drm_encoder *encoder,
 	buffer[3] = 0;
 	len++;
 
-	intel_hdmi->write_infoframe(encoder, frame->any.type, buffer, len);
+	intel_dig_port->write_infoframe(encoder, frame->any.type, buffer, len);
 }
 
 static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
@@ -930,6 +930,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 				  struct intel_crtc_state *pipe_config)
 {
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct intel_digital_port *intel_dig_port = hdmi_to_dig_port(intel_hdmi);
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 tmp, flags = 0;
@@ -950,7 +951,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 	if (tmp & HDMI_MODE_SELECT_HDMI)
 		pipe_config->has_hdmi_sink = true;
 
-	if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
+	if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
 		pipe_config->has_infoframe = true;
 
 	if (tmp & SDVO_AUDIO_ENABLE)
@@ -1117,6 +1118,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct intel_digital_port *intel_dig_port =
+		hdmi_to_dig_port(intel_hdmi);
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 	u32 temp;
 
@@ -1159,7 +1162,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
 		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
 	}
 
-	intel_hdmi->set_infoframes(&encoder->base, false, NULL);
+	intel_dig_port->set_infoframes(&encoder->base, false, NULL);
 
 	intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
 }
@@ -1658,21 +1661,21 @@ done:
 
 static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
 {
-	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct intel_digital_port *intel_dig_port =
+		enc_to_dig_port(&encoder->base);
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
 	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
 
 	intel_hdmi_prepare(encoder);
 
-	intel_hdmi->set_infoframes(&encoder->base,
-				   intel_crtc->config->has_infoframe,
-				   adjusted_mode);
+	intel_dig_port->set_infoframes(&encoder->base,
+				       intel_crtc->config->has_infoframe,
+				       adjusted_mode);
 }
 
 static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
 {
 	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
-	struct intel_hdmi *intel_hdmi = &dport->hdmi;
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc =
@@ -1685,9 +1688,9 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
 	vlv_set_phy_signal_level(encoder, 0x2b245f5f, 0x00002000, 0x5578b83a,
 				 0x2b247878);
 
-	intel_hdmi->set_infoframes(&encoder->base,
-				   intel_crtc->config->has_infoframe,
-				   adjusted_mode);
+	dport->set_infoframes(&encoder->base,
+			      intel_crtc->config->has_infoframe,
+			      adjusted_mode);
 
 	g4x_enable_hdmi(encoder);
 
@@ -1735,7 +1738,6 @@ static void chv_hdmi_post_disable(struct intel_encoder *encoder)
 static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
 {
 	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
-	struct intel_hdmi *intel_hdmi = &dport->hdmi;
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc =
@@ -1748,9 +1750,9 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
 	/* Use 800mV-0dB */
 	chv_set_phy_signal_level(encoder, 128, 102, false);
 
-	intel_hdmi->set_infoframes(&encoder->base,
-				   intel_crtc->config->has_infoframe,
-				   adjusted_mode);
+	dport->set_infoframes(&encoder->base,
+			      intel_crtc->config->has_infoframe,
+			      adjusted_mode);
 
 	g4x_enable_hdmi(encoder);
 
@@ -1882,25 +1884,25 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	}
 
 	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
-		intel_hdmi->write_infoframe = vlv_write_infoframe;
-		intel_hdmi->set_infoframes = vlv_set_infoframes;
-		intel_hdmi->infoframe_enabled = vlv_infoframe_enabled;
+		intel_dig_port->write_infoframe = vlv_write_infoframe;
+		intel_dig_port->set_infoframes = vlv_set_infoframes;
+		intel_dig_port->infoframe_enabled = vlv_infoframe_enabled;
 	} else if (IS_G4X(dev)) {
-		intel_hdmi->write_infoframe = g4x_write_infoframe;
-		intel_hdmi->set_infoframes = g4x_set_infoframes;
-		intel_hdmi->infoframe_enabled = g4x_infoframe_enabled;
+		intel_dig_port->write_infoframe = g4x_write_infoframe;
+		intel_dig_port->set_infoframes = g4x_set_infoframes;
+		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
 	} else if (HAS_DDI(dev)) {
-		intel_hdmi->write_infoframe = hsw_write_infoframe;
-		intel_hdmi->set_infoframes = hsw_set_infoframes;
-		intel_hdmi->infoframe_enabled = hsw_infoframe_enabled;
+		intel_dig_port->write_infoframe = hsw_write_infoframe;
+		intel_dig_port->set_infoframes = hsw_set_infoframes;
+		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
 	} else if (HAS_PCH_IBX(dev)) {
-		intel_hdmi->write_infoframe = ibx_write_infoframe;
-		intel_hdmi->set_infoframes = ibx_set_infoframes;
-		intel_hdmi->infoframe_enabled = ibx_infoframe_enabled;
+		intel_dig_port->write_infoframe = ibx_write_infoframe;
+		intel_dig_port->set_infoframes = ibx_set_infoframes;
+		intel_dig_port->infoframe_enabled = ibx_infoframe_enabled;
 	} else {
-		intel_hdmi->write_infoframe = cpt_write_infoframe;
-		intel_hdmi->set_infoframes = cpt_set_infoframes;
-		intel_hdmi->infoframe_enabled = cpt_infoframe_enabled;
+		intel_dig_port->write_infoframe = cpt_write_infoframe;
+		intel_dig_port->set_infoframes = cpt_set_infoframes;
+		intel_dig_port->infoframe_enabled = cpt_infoframe_enabled;
 	}
 
 	if (HAS_DDI(dev))
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915: Move infoframe vfuncs into intel_digital_port

[Sharma, Shashank]

Regards
Shashank

On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> DP ports will also want to utilize the video DIP for SDP transmission.
> So let's move the vfuncs into the dig_port.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c  | 20 ++++++------
>   drivers/gpu/drm/i915/intel_drv.h  | 16 +++++-----
>   drivers/gpu/drm/i915/intel_hdmi.c | 64 ++++++++++++++++++++-------------------
>   3 files changed, 52 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 6ff2a7b97ca6..3a882a979e5d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1628,11 +1628,12 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
>   		if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9)
>   			intel_dp_stop_link_train(intel_dp);
>   	} else if (type == INTEL_OUTPUT_HDMI) {
> -		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> +		struct intel_digital_port *intel_dig_port =
> +			enc_to_dig_port(encoder);
>
> -		intel_hdmi->set_infoframes(encoder,
> -					   crtc->config->has_infoframe,
> -					   &crtc->config->base.adjusted_mode);
> +		intel_dig_port->set_infoframes(encoder,
> +					       crtc->config->has_infoframe,
> +
We have kept this call still inside if (type == HDMI); now when the aim 
is to make it common for all DDI interfaces, do we need this check ?
					       &crtc->config->base.adjusted_mode);
>   	}
>   }
>
> @@ -1662,9 +1663,10 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>   		intel_wait_ddi_buf_idle(dev_priv, port);
>
>   	if (type == INTEL_OUTPUT_HDMI) {
> -		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> +		struct intel_digital_port *intel_dig_port =
> +			enc_to_dig_port(encoder);
>
> -		intel_hdmi->set_infoframes(encoder, false, NULL);
> +		intel_dig_port->set_infoframes(encoder, false, NULL);
Same as above.
>   	}
>
>   	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
> @@ -2155,7 +2157,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>   	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>   	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>   	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> -	struct intel_hdmi *intel_hdmi;
> +	struct intel_digital_port *intel_dig_port;
>   	u32 temp, flags = 0;
>
>   	/* XXX: DSI transcoder paranoia */
> @@ -2194,9 +2196,9 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>   	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
>   	case TRANS_DDI_MODE_SELECT_HDMI:
	case TRANS_DDI_MODE_SELECT_DP here: ?
>   		pipe_config->has_hdmi_sink = true;
> -		intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> +		intel_dig_port = enc_to_dig_port(&encoder->base);
>
> -		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
> +		if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
>   			pipe_config->has_infoframe = true;
>   		/* fall through */
>   	case TRANS_DDI_MODE_SELECT_DVI:
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ebe7b3427e2e..a607799b7776 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -790,14 +790,6 @@ struct intel_hdmi {
>   	bool rgb_quant_range_selectable;
>   	enum hdmi_picture_aspect aspect_ratio;
>   	struct intel_connector *attached_connector;
> -	void (*write_infoframe)(struct drm_encoder *encoder,
> -				enum hdmi_infoframe_type type,
> -				const void *frame, ssize_t len);
> -	void (*set_infoframes)(struct drm_encoder *encoder,
> -			       bool enable,
> -			       const struct drm_display_mode *adjusted_mode);
> -	bool (*infoframe_enabled)(struct drm_encoder *encoder,
> -				  const struct intel_crtc_state *pipe_config);
>   };
>
>   struct intel_dp_mst_encoder;
> @@ -907,6 +899,14 @@ struct intel_digital_port {
>   	uint8_t max_lanes;
>   	/* for communication with audio component; protected by av_mutex */
>   	const struct drm_connector *audio_connector;
> +	void (*write_infoframe)(struct drm_encoder *encoder,
> +				enum hdmi_infoframe_type type,
> +				const void *frame, ssize_t len);
> +	void (*set_infoframes)(struct drm_encoder *encoder,
> +			       bool enable,
> +			       const struct drm_display_mode *adjusted_mode);
> +	bool (*infoframe_enabled)(struct drm_encoder *encoder,
> +				  const struct intel_crtc_state *pipe_config);
>   };
>
>   struct intel_dp_mst_encoder {
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 067b10a7cb04..319f5013923c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -427,7 +427,7 @@ static bool hsw_infoframe_enabled(struct drm_encoder *encoder,
>   static void intel_write_infoframe(struct drm_encoder *encoder,
>   				  union hdmi_infoframe *frame)
>   {
> -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> +	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
>   	uint8_t buffer[VIDEO_DIP_DATA_SIZE];
>   	ssize_t len;
>
> @@ -443,7 +443,7 @@ static void intel_write_infoframe(struct drm_encoder *encoder,
>   	buffer[3] = 0;
>   	len++;
>
> -	intel_hdmi->write_infoframe(encoder, frame->any.type, buffer, len);
> +	intel_dig_port->write_infoframe(encoder, frame->any.type, buffer, len);
>   }
>
>   static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
> @@ -930,6 +930,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>   				  struct intel_crtc_state *pipe_config)
>   {
>   	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> +	struct intel_digital_port *intel_dig_port = hdmi_to_dig_port(intel_hdmi);
>   	struct drm_device *dev = encoder->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	u32 tmp, flags = 0;
> @@ -950,7 +951,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>   	if (tmp & HDMI_MODE_SELECT_HDMI)
>   		pipe_config->has_hdmi_sink = true;
>
> -	if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
> +	if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
>   		pipe_config->has_infoframe = true;
>
>   	if (tmp & SDVO_AUDIO_ENABLE)
> @@ -1117,6 +1118,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
>   	struct drm_device *dev = encoder->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> +	struct intel_digital_port *intel_dig_port =
> +		hdmi_to_dig_port(intel_hdmi);
>   	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
>   	u32 temp;
>
> @@ -1159,7 +1162,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
>   		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
>   	}
>
> -	intel_hdmi->set_infoframes(&encoder->base, false, NULL);
> +	intel_dig_port->set_infoframes(&encoder->base, false, NULL);
>
I guess there is a separate patch where you are adding the code which 
uses these functions for DP, and will going further in this series.
- Shashank
>   	intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
>   }
> @@ -1658,21 +1661,21 @@ done:
>
>   static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
>   {
> -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> +	struct intel_digital_port *intel_dig_port =
> +		enc_to_dig_port(&encoder->base);
>   	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>   	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
>
>   	intel_hdmi_prepare(encoder);
>
> -	intel_hdmi->set_infoframes(&encoder->base,
> -				   intel_crtc->config->has_infoframe,
> -				   adjusted_mode);
> +	intel_dig_port->set_infoframes(&encoder->base,
> +				       intel_crtc->config->has_infoframe,
> +				       adjusted_mode);
>   }
>
>   static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
>   {
>   	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
> -	struct intel_hdmi *intel_hdmi = &dport->hdmi;
>   	struct drm_device *dev = encoder->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_crtc *intel_crtc =
> @@ -1685,9 +1688,9 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
>   	vlv_set_phy_signal_level(encoder, 0x2b245f5f, 0x00002000, 0x5578b83a,
>   				 0x2b247878);
>
> -	intel_hdmi->set_infoframes(&encoder->base,
> -				   intel_crtc->config->has_infoframe,
> -				   adjusted_mode);
> +	dport->set_infoframes(&encoder->base,
> +			      intel_crtc->config->has_infoframe,
> +			      adjusted_mode);
>
>   	g4x_enable_hdmi(encoder);
>
> @@ -1735,7 +1738,6 @@ static void chv_hdmi_post_disable(struct intel_encoder *encoder)
>   static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
>   {
>   	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
> -	struct intel_hdmi *intel_hdmi = &dport->hdmi;
>   	struct drm_device *dev = encoder->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_crtc *intel_crtc =
> @@ -1748,9 +1750,9 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
>   	/* Use 800mV-0dB */
>   	chv_set_phy_signal_level(encoder, 128, 102, false);
>
> -	intel_hdmi->set_infoframes(&encoder->base,
> -				   intel_crtc->config->has_infoframe,
> -				   adjusted_mode);
> +	dport->set_infoframes(&encoder->base,
> +			      intel_crtc->config->has_infoframe,
> +			      adjusted_mode);
>
>   	g4x_enable_hdmi(encoder);
>
> @@ -1882,25 +1884,25 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>   	}
>
>   	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> -		intel_hdmi->write_infoframe = vlv_write_infoframe;
> -		intel_hdmi->set_infoframes = vlv_set_infoframes;
> -		intel_hdmi->infoframe_enabled = vlv_infoframe_enabled;
> +		intel_dig_port->write_infoframe = vlv_write_infoframe;
> +		intel_dig_port->set_infoframes = vlv_set_infoframes;
> +		intel_dig_port->infoframe_enabled = vlv_infoframe_enabled;
>   	} else if (IS_G4X(dev)) {
> -		intel_hdmi->write_infoframe = g4x_write_infoframe;
> -		intel_hdmi->set_infoframes = g4x_set_infoframes;
> -		intel_hdmi->infoframe_enabled = g4x_infoframe_enabled;
> +		intel_dig_port->write_infoframe = g4x_write_infoframe;
> +		intel_dig_port->set_infoframes = g4x_set_infoframes;
> +		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
>   	} else if (HAS_DDI(dev)) {
> -		intel_hdmi->write_infoframe = hsw_write_infoframe;
> -		intel_hdmi->set_infoframes = hsw_set_infoframes;
> -		intel_hdmi->infoframe_enabled = hsw_infoframe_enabled;
> +		intel_dig_port->write_infoframe = hsw_write_infoframe;
> +		intel_dig_port->set_infoframes = hsw_set_infoframes;
> +		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
>   	} else if (HAS_PCH_IBX(dev)) {
> -		intel_hdmi->write_infoframe = ibx_write_infoframe;
> -		intel_hdmi->set_infoframes = ibx_set_infoframes;
> -		intel_hdmi->infoframe_enabled = ibx_infoframe_enabled;
> +		intel_dig_port->write_infoframe = ibx_write_infoframe;
> +		intel_dig_port->set_infoframes = ibx_set_infoframes;
> +		intel_dig_port->infoframe_enabled = ibx_infoframe_enabled;
>   	} else {
> -		intel_hdmi->write_infoframe = cpt_write_infoframe;
> -		intel_hdmi->set_infoframes = cpt_set_infoframes;
> -		intel_hdmi->infoframe_enabled = cpt_infoframe_enabled;
> +		intel_dig_port->write_infoframe = cpt_write_infoframe;
> +		intel_dig_port->set_infoframes = cpt_set_infoframes;
> +		intel_dig_port->infoframe_enabled = cpt_infoframe_enabled;
>   	}
>
>   	if (HAS_DDI(dev))
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915: Move infoframe vfuncs into intel_digital_port

[Ville Syrjälä]

On Mon, Jun 13, 2016 at 03:47:13PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > DP ports will also want to utilize the video DIP for SDP transmission.
> > So let's move the vfuncs into the dig_port.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_ddi.c  | 20 ++++++------
> >   drivers/gpu/drm/i915/intel_drv.h  | 16 +++++-----
> >   drivers/gpu/drm/i915/intel_hdmi.c | 64 ++++++++++++++++++++-------------------
> >   3 files changed, 52 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 6ff2a7b97ca6..3a882a979e5d 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1628,11 +1628,12 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> >   		if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9)
> >   			intel_dp_stop_link_train(intel_dp);
> >   	} else if (type == INTEL_OUTPUT_HDMI) {
> > -		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> > +		struct intel_digital_port *intel_dig_port =
> > +			enc_to_dig_port(encoder);
> >
> > -		intel_hdmi->set_infoframes(encoder,
> > -					   crtc->config->has_infoframe,
> > -					   &crtc->config->base.adjusted_mode);
> > +		intel_dig_port->set_infoframes(encoder,
> > +					       crtc->config->has_infoframe,
> > +
> We have kept this call still inside if (type == HDMI); now when the aim 
> is to make it common for all DDI interfaces, do we need this check ?
> 					       &crtc->config->base.adjusted_mode);

This is a purely mechanical change. No change in behaviour.

> >   	}
> >   }
> >
> > @@ -1662,9 +1663,10 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> >   		intel_wait_ddi_buf_idle(dev_priv, port);
> >
> >   	if (type == INTEL_OUTPUT_HDMI) {
> > -		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> > +		struct intel_digital_port *intel_dig_port =
> > +			enc_to_dig_port(encoder);
> >
> > -		intel_hdmi->set_infoframes(encoder, false, NULL);
> > +		intel_dig_port->set_infoframes(encoder, false, NULL);
> Same as above.
> >   	}
> >
> >   	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
> > @@ -2155,7 +2157,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >   	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> >   	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> >   	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > -	struct intel_hdmi *intel_hdmi;
> > +	struct intel_digital_port *intel_dig_port;
> >   	u32 temp, flags = 0;
> >
> >   	/* XXX: DSI transcoder paranoia */
> > @@ -2194,9 +2196,9 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >   	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
> >   	case TRANS_DDI_MODE_SELECT_HDMI:
> 	case TRANS_DDI_MODE_SELECT_DP here: ?
> >   		pipe_config->has_hdmi_sink = true;
> > -		intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> > +		intel_dig_port = enc_to_dig_port(&encoder->base);
> >
> > -		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
> > +		if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
> >   			pipe_config->has_infoframe = true;
> >   		/* fall through */
> >   	case TRANS_DDI_MODE_SELECT_DVI:
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index ebe7b3427e2e..a607799b7776 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -790,14 +790,6 @@ struct intel_hdmi {
> >   	bool rgb_quant_range_selectable;
> >   	enum hdmi_picture_aspect aspect_ratio;
> >   	struct intel_connector *attached_connector;
> > -	void (*write_infoframe)(struct drm_encoder *encoder,
> > -				enum hdmi_infoframe_type type,
> > -				const void *frame, ssize_t len);
> > -	void (*set_infoframes)(struct drm_encoder *encoder,
> > -			       bool enable,
> > -			       const struct drm_display_mode *adjusted_mode);
> > -	bool (*infoframe_enabled)(struct drm_encoder *encoder,
> > -				  const struct intel_crtc_state *pipe_config);
> >   };
> >
> >   struct intel_dp_mst_encoder;
> > @@ -907,6 +899,14 @@ struct intel_digital_port {
> >   	uint8_t max_lanes;
> >   	/* for communication with audio component; protected by av_mutex */
> >   	const struct drm_connector *audio_connector;
> > +	void (*write_infoframe)(struct drm_encoder *encoder,
> > +				enum hdmi_infoframe_type type,
> > +				const void *frame, ssize_t len);
> > +	void (*set_infoframes)(struct drm_encoder *encoder,
> > +			       bool enable,
> > +			       const struct drm_display_mode *adjusted_mode);
> > +	bool (*infoframe_enabled)(struct drm_encoder *encoder,
> > +				  const struct intel_crtc_state *pipe_config);
> >   };
> >
> >   struct intel_dp_mst_encoder {
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 067b10a7cb04..319f5013923c 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -427,7 +427,7 @@ static bool hsw_infoframe_enabled(struct drm_encoder *encoder,
> >   static void intel_write_infoframe(struct drm_encoder *encoder,
> >   				  union hdmi_infoframe *frame)
> >   {
> > -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> > +	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
> >   	uint8_t buffer[VIDEO_DIP_DATA_SIZE];
> >   	ssize_t len;
> >
> > @@ -443,7 +443,7 @@ static void intel_write_infoframe(struct drm_encoder *encoder,
> >   	buffer[3] = 0;
> >   	len++;
> >
> > -	intel_hdmi->write_infoframe(encoder, frame->any.type, buffer, len);
> > +	intel_dig_port->write_infoframe(encoder, frame->any.type, buffer, len);
> >   }
> >
> >   static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
> > @@ -930,6 +930,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
> >   				  struct intel_crtc_state *pipe_config)
> >   {
> >   	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> > +	struct intel_digital_port *intel_dig_port = hdmi_to_dig_port(intel_hdmi);
> >   	struct drm_device *dev = encoder->base.dev;
> >   	struct drm_i915_private *dev_priv = dev->dev_private;
> >   	u32 tmp, flags = 0;
> > @@ -950,7 +951,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
> >   	if (tmp & HDMI_MODE_SELECT_HDMI)
> >   		pipe_config->has_hdmi_sink = true;
> >
> > -	if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
> > +	if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
> >   		pipe_config->has_infoframe = true;
> >
> >   	if (tmp & SDVO_AUDIO_ENABLE)
> > @@ -1117,6 +1118,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
> >   	struct drm_device *dev = encoder->base.dev;
> >   	struct drm_i915_private *dev_priv = dev->dev_private;
> >   	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> > +	struct intel_digital_port *intel_dig_port =
> > +		hdmi_to_dig_port(intel_hdmi);
> >   	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> >   	u32 temp;
> >
> > @@ -1159,7 +1162,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
> >   		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> >   	}
> >
> > -	intel_hdmi->set_infoframes(&encoder->base, false, NULL);
> > +	intel_dig_port->set_infoframes(&encoder->base, false, NULL);
> >
> I guess there is a separate patch where you are adding the code which 
> uses these functions for DP, and will going further in this series.
> - Shashank
> >   	intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
> >   }
> > @@ -1658,21 +1661,21 @@ done:
> >
> >   static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
> >   {
> > -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> > +	struct intel_digital_port *intel_dig_port =
> > +		enc_to_dig_port(&encoder->base);
> >   	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> >   	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
> >
> >   	intel_hdmi_prepare(encoder);
> >
> > -	intel_hdmi->set_infoframes(&encoder->base,
> > -				   intel_crtc->config->has_infoframe,
> > -				   adjusted_mode);
> > +	intel_dig_port->set_infoframes(&encoder->base,
> > +				       intel_crtc->config->has_infoframe,
> > +				       adjusted_mode);
> >   }
> >
> >   static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
> >   {
> >   	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
> > -	struct intel_hdmi *intel_hdmi = &dport->hdmi;
> >   	struct drm_device *dev = encoder->base.dev;
> >   	struct drm_i915_private *dev_priv = dev->dev_private;
> >   	struct intel_crtc *intel_crtc =
> > @@ -1685,9 +1688,9 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
> >   	vlv_set_phy_signal_level(encoder, 0x2b245f5f, 0x00002000, 0x5578b83a,
> >   				 0x2b247878);
> >
> > -	intel_hdmi->set_infoframes(&encoder->base,
> > -				   intel_crtc->config->has_infoframe,
> > -				   adjusted_mode);
> > +	dport->set_infoframes(&encoder->base,
> > +			      intel_crtc->config->has_infoframe,
> > +			      adjusted_mode);
> >
> >   	g4x_enable_hdmi(encoder);
> >
> > @@ -1735,7 +1738,6 @@ static void chv_hdmi_post_disable(struct intel_encoder *encoder)
> >   static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
> >   {
> >   	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
> > -	struct intel_hdmi *intel_hdmi = &dport->hdmi;
> >   	struct drm_device *dev = encoder->base.dev;
> >   	struct drm_i915_private *dev_priv = dev->dev_private;
> >   	struct intel_crtc *intel_crtc =
> > @@ -1748,9 +1750,9 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
> >   	/* Use 800mV-0dB */
> >   	chv_set_phy_signal_level(encoder, 128, 102, false);
> >
> > -	intel_hdmi->set_infoframes(&encoder->base,
> > -				   intel_crtc->config->has_infoframe,
> > -				   adjusted_mode);
> > +	dport->set_infoframes(&encoder->base,
> > +			      intel_crtc->config->has_infoframe,
> > +			      adjusted_mode);
> >
> >   	g4x_enable_hdmi(encoder);
> >
> > @@ -1882,25 +1884,25 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >   	}
> >
> >   	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > -		intel_hdmi->write_infoframe = vlv_write_infoframe;
> > -		intel_hdmi->set_infoframes = vlv_set_infoframes;
> > -		intel_hdmi->infoframe_enabled = vlv_infoframe_enabled;
> > +		intel_dig_port->write_infoframe = vlv_write_infoframe;
> > +		intel_dig_port->set_infoframes = vlv_set_infoframes;
> > +		intel_dig_port->infoframe_enabled = vlv_infoframe_enabled;
> >   	} else if (IS_G4X(dev)) {
> > -		intel_hdmi->write_infoframe = g4x_write_infoframe;
> > -		intel_hdmi->set_infoframes = g4x_set_infoframes;
> > -		intel_hdmi->infoframe_enabled = g4x_infoframe_enabled;
> > +		intel_dig_port->write_infoframe = g4x_write_infoframe;
> > +		intel_dig_port->set_infoframes = g4x_set_infoframes;
> > +		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
> >   	} else if (HAS_DDI(dev)) {
> > -		intel_hdmi->write_infoframe = hsw_write_infoframe;
> > -		intel_hdmi->set_infoframes = hsw_set_infoframes;
> > -		intel_hdmi->infoframe_enabled = hsw_infoframe_enabled;
> > +		intel_dig_port->write_infoframe = hsw_write_infoframe;
> > +		intel_dig_port->set_infoframes = hsw_set_infoframes;
> > +		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
> >   	} else if (HAS_PCH_IBX(dev)) {
> > -		intel_hdmi->write_infoframe = ibx_write_infoframe;
> > -		intel_hdmi->set_infoframes = ibx_set_infoframes;
> > -		intel_hdmi->infoframe_enabled = ibx_infoframe_enabled;
> > +		intel_dig_port->write_infoframe = ibx_write_infoframe;
> > +		intel_dig_port->set_infoframes = ibx_set_infoframes;
> > +		intel_dig_port->infoframe_enabled = ibx_infoframe_enabled;
> >   	} else {
> > -		intel_hdmi->write_infoframe = cpt_write_infoframe;
> > -		intel_hdmi->set_infoframes = cpt_set_infoframes;
> > -		intel_hdmi->infoframe_enabled = cpt_infoframe_enabled;
> > +		intel_dig_port->write_infoframe = cpt_write_infoframe;
> > +		intel_dig_port->set_infoframes = cpt_set_infoframes;
> > +		intel_dig_port->infoframe_enabled = cpt_infoframe_enabled;
> >   	}
> >
> >   	if (HAS_DDI(dev))
> >

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915: Move infoframe vfuncs into intel_digital_port

[Sharma, Shashank]

Reviewed-by: Shashank Sharma

Regards
Shashank
On 6/13/2016 5:55 PM, Ville Syrjälä wrote:
> On Mon, Jun 13, 2016 at 03:47:13PM +0530, Sharma, Shashank wrote:
>> Regards
>> Shashank
>>
>> On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> DP ports will also want to utilize the video DIP for SDP transmission.
>>> So let's move the vfuncs into the dig_port.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_ddi.c  | 20 ++++++------
>>>    drivers/gpu/drm/i915/intel_drv.h  | 16 +++++-----
>>>    drivers/gpu/drm/i915/intel_hdmi.c | 64 ++++++++++++++++++++-------------------
>>>    3 files changed, 52 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>> index 6ff2a7b97ca6..3a882a979e5d 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -1628,11 +1628,12 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
>>>    		if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9)
>>>    			intel_dp_stop_link_train(intel_dp);
>>>    	} else if (type == INTEL_OUTPUT_HDMI) {
>>> -		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>>> +		struct intel_digital_port *intel_dig_port =
>>> +			enc_to_dig_port(encoder);
>>>
>>> -		intel_hdmi->set_infoframes(encoder,
>>> -					   crtc->config->has_infoframe,
>>> -					   &crtc->config->base.adjusted_mode);
>>> +		intel_dig_port->set_infoframes(encoder,
>>> +					       crtc->config->has_infoframe,
>>> +
>> We have kept this call still inside if (type == HDMI); now when the aim
>> is to make it common for all DDI interfaces, do we need this check ?
>> 					       &crtc->config->base.adjusted_mode);
>
> This is a purely mechanical change. No change in behaviour.
>
>>>    	}
>>>    }
>>>
>>> @@ -1662,9 +1663,10 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>>>    		intel_wait_ddi_buf_idle(dev_priv, port);
>>>
>>>    	if (type == INTEL_OUTPUT_HDMI) {
>>> -		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>>> +		struct intel_digital_port *intel_dig_port =
>>> +			enc_to_dig_port(encoder);
>>>
>>> -		intel_hdmi->set_infoframes(encoder, false, NULL);
>>> +		intel_dig_port->set_infoframes(encoder, false, NULL);
>> Same as above.
>>>    	}
>>>
>>>    	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
>>> @@ -2155,7 +2157,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>>>    	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>>>    	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>>>    	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
>>> -	struct intel_hdmi *intel_hdmi;
>>> +	struct intel_digital_port *intel_dig_port;
>>>    	u32 temp, flags = 0;
>>>
>>>    	/* XXX: DSI transcoder paranoia */
>>> @@ -2194,9 +2196,9 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>>>    	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
>>>    	case TRANS_DDI_MODE_SELECT_HDMI:
>> 	case TRANS_DDI_MODE_SELECT_DP here: ?
>>>    		pipe_config->has_hdmi_sink = true;
>>> -		intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>>> +		intel_dig_port = enc_to_dig_port(&encoder->base);
>>>
>>> -		if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
>>> +		if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
>>>    			pipe_config->has_infoframe = true;
>>>    		/* fall through */
>>>    	case TRANS_DDI_MODE_SELECT_DVI:
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index ebe7b3427e2e..a607799b7776 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -790,14 +790,6 @@ struct intel_hdmi {
>>>    	bool rgb_quant_range_selectable;
>>>    	enum hdmi_picture_aspect aspect_ratio;
>>>    	struct intel_connector *attached_connector;
>>> -	void (*write_infoframe)(struct drm_encoder *encoder,
>>> -				enum hdmi_infoframe_type type,
>>> -				const void *frame, ssize_t len);
>>> -	void (*set_infoframes)(struct drm_encoder *encoder,
>>> -			       bool enable,
>>> -			       const struct drm_display_mode *adjusted_mode);
>>> -	bool (*infoframe_enabled)(struct drm_encoder *encoder,
>>> -				  const struct intel_crtc_state *pipe_config);
>>>    };
>>>
>>>    struct intel_dp_mst_encoder;
>>> @@ -907,6 +899,14 @@ struct intel_digital_port {
>>>    	uint8_t max_lanes;
>>>    	/* for communication with audio component; protected by av_mutex */
>>>    	const struct drm_connector *audio_connector;
>>> +	void (*write_infoframe)(struct drm_encoder *encoder,
>>> +				enum hdmi_infoframe_type type,
>>> +				const void *frame, ssize_t len);
>>> +	void (*set_infoframes)(struct drm_encoder *encoder,
>>> +			       bool enable,
>>> +			       const struct drm_display_mode *adjusted_mode);
>>> +	bool (*infoframe_enabled)(struct drm_encoder *encoder,
>>> +				  const struct intel_crtc_state *pipe_config);
>>>    };
>>>
>>>    struct intel_dp_mst_encoder {
>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>>> index 067b10a7cb04..319f5013923c 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>> @@ -427,7 +427,7 @@ static bool hsw_infoframe_enabled(struct drm_encoder *encoder,
>>>    static void intel_write_infoframe(struct drm_encoder *encoder,
>>>    				  union hdmi_infoframe *frame)
>>>    {
>>> -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>>> +	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
>>>    	uint8_t buffer[VIDEO_DIP_DATA_SIZE];
>>>    	ssize_t len;
>>>
>>> @@ -443,7 +443,7 @@ static void intel_write_infoframe(struct drm_encoder *encoder,
>>>    	buffer[3] = 0;
>>>    	len++;
>>>
>>> -	intel_hdmi->write_infoframe(encoder, frame->any.type, buffer, len);
>>> +	intel_dig_port->write_infoframe(encoder, frame->any.type, buffer, len);
>>>    }
>>>
>>>    static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>>> @@ -930,6 +930,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>>>    				  struct intel_crtc_state *pipe_config)
>>>    {
>>>    	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>>> +	struct intel_digital_port *intel_dig_port = hdmi_to_dig_port(intel_hdmi);
>>>    	struct drm_device *dev = encoder->base.dev;
>>>    	struct drm_i915_private *dev_priv = dev->dev_private;
>>>    	u32 tmp, flags = 0;
>>> @@ -950,7 +951,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>>>    	if (tmp & HDMI_MODE_SELECT_HDMI)
>>>    		pipe_config->has_hdmi_sink = true;
>>>
>>> -	if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
>>> +	if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
>>>    		pipe_config->has_infoframe = true;
>>>
>>>    	if (tmp & SDVO_AUDIO_ENABLE)
>>> @@ -1117,6 +1118,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
>>>    	struct drm_device *dev = encoder->base.dev;
>>>    	struct drm_i915_private *dev_priv = dev->dev_private;
>>>    	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>>> +	struct intel_digital_port *intel_dig_port =
>>> +		hdmi_to_dig_port(intel_hdmi);
>>>    	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
>>>    	u32 temp;
>>>
>>> @@ -1159,7 +1162,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
>>>    		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
>>>    	}
>>>
>>> -	intel_hdmi->set_infoframes(&encoder->base, false, NULL);
>>> +	intel_dig_port->set_infoframes(&encoder->base, false, NULL);
>>>
>> I guess there is a separate patch where you are adding the code which
>> uses these functions for DP, and will going further in this series.
>> - Shashank
>>>    	intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
>>>    }
>>> @@ -1658,21 +1661,21 @@ done:
>>>
>>>    static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
>>>    {
>>> -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>>> +	struct intel_digital_port *intel_dig_port =
>>> +		enc_to_dig_port(&encoder->base);
>>>    	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>>>    	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
>>>
>>>    	intel_hdmi_prepare(encoder);
>>>
>>> -	intel_hdmi->set_infoframes(&encoder->base,
>>> -				   intel_crtc->config->has_infoframe,
>>> -				   adjusted_mode);
>>> +	intel_dig_port->set_infoframes(&encoder->base,
>>> +				       intel_crtc->config->has_infoframe,
>>> +				       adjusted_mode);
>>>    }
>>>
>>>    static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
>>>    {
>>>    	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
>>> -	struct intel_hdmi *intel_hdmi = &dport->hdmi;
>>>    	struct drm_device *dev = encoder->base.dev;
>>>    	struct drm_i915_private *dev_priv = dev->dev_private;
>>>    	struct intel_crtc *intel_crtc =
>>> @@ -1685,9 +1688,9 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
>>>    	vlv_set_phy_signal_level(encoder, 0x2b245f5f, 0x00002000, 0x5578b83a,
>>>    				 0x2b247878);
>>>
>>> -	intel_hdmi->set_infoframes(&encoder->base,
>>> -				   intel_crtc->config->has_infoframe,
>>> -				   adjusted_mode);
>>> +	dport->set_infoframes(&encoder->base,
>>> +			      intel_crtc->config->has_infoframe,
>>> +			      adjusted_mode);
>>>
>>>    	g4x_enable_hdmi(encoder);
>>>
>>> @@ -1735,7 +1738,6 @@ static void chv_hdmi_post_disable(struct intel_encoder *encoder)
>>>    static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
>>>    {
>>>    	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
>>> -	struct intel_hdmi *intel_hdmi = &dport->hdmi;
>>>    	struct drm_device *dev = encoder->base.dev;
>>>    	struct drm_i915_private *dev_priv = dev->dev_private;
>>>    	struct intel_crtc *intel_crtc =
>>> @@ -1748,9 +1750,9 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
>>>    	/* Use 800mV-0dB */
>>>    	chv_set_phy_signal_level(encoder, 128, 102, false);
>>>
>>> -	intel_hdmi->set_infoframes(&encoder->base,
>>> -				   intel_crtc->config->has_infoframe,
>>> -				   adjusted_mode);
>>> +	dport->set_infoframes(&encoder->base,
>>> +			      intel_crtc->config->has_infoframe,
>>> +			      adjusted_mode);
>>>
>>>    	g4x_enable_hdmi(encoder);
>>>
>>> @@ -1882,25 +1884,25 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>>>    	}
>>>
>>>    	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
>>> -		intel_hdmi->write_infoframe = vlv_write_infoframe;
>>> -		intel_hdmi->set_infoframes = vlv_set_infoframes;
>>> -		intel_hdmi->infoframe_enabled = vlv_infoframe_enabled;
>>> +		intel_dig_port->write_infoframe = vlv_write_infoframe;
>>> +		intel_dig_port->set_infoframes = vlv_set_infoframes;
>>> +		intel_dig_port->infoframe_enabled = vlv_infoframe_enabled;
>>>    	} else if (IS_G4X(dev)) {
>>> -		intel_hdmi->write_infoframe = g4x_write_infoframe;
>>> -		intel_hdmi->set_infoframes = g4x_set_infoframes;
>>> -		intel_hdmi->infoframe_enabled = g4x_infoframe_enabled;
>>> +		intel_dig_port->write_infoframe = g4x_write_infoframe;
>>> +		intel_dig_port->set_infoframes = g4x_set_infoframes;
>>> +		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
>>>    	} else if (HAS_DDI(dev)) {
>>> -		intel_hdmi->write_infoframe = hsw_write_infoframe;
>>> -		intel_hdmi->set_infoframes = hsw_set_infoframes;
>>> -		intel_hdmi->infoframe_enabled = hsw_infoframe_enabled;
>>> +		intel_dig_port->write_infoframe = hsw_write_infoframe;
>>> +		intel_dig_port->set_infoframes = hsw_set_infoframes;
>>> +		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
>>>    	} else if (HAS_PCH_IBX(dev)) {
>>> -		intel_hdmi->write_infoframe = ibx_write_infoframe;
>>> -		intel_hdmi->set_infoframes = ibx_set_infoframes;
>>> -		intel_hdmi->infoframe_enabled = ibx_infoframe_enabled;
>>> +		intel_dig_port->write_infoframe = ibx_write_infoframe;
>>> +		intel_dig_port->set_infoframes = ibx_set_infoframes;
>>> +		intel_dig_port->infoframe_enabled = ibx_infoframe_enabled;
>>>    	} else {
>>> -		intel_hdmi->write_infoframe = cpt_write_infoframe;
>>> -		intel_hdmi->set_infoframes = cpt_set_infoframes;
>>> -		intel_hdmi->infoframe_enabled = cpt_infoframe_enabled;
>>> +		intel_dig_port->write_infoframe = cpt_write_infoframe;
>>> +		intel_dig_port->set_infoframes = cpt_set_infoframes;
>>> +		intel_dig_port->infoframe_enabled = cpt_infoframe_enabled;
>>>    	}
>>>
>>>    	if (HAS_DDI(dev))
>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/7] drm/i915: Init infoframe vfuncs for DP encoders as well

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

DP ports may want to use the video DIP for SDP transmission, so let's
initiialize the vfuncs for DP encoders as well. The only exception is
port A eDP prior to HSW as that one doesn't have a video DIP instance.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  |  2 ++
 drivers/gpu/drm/i915/intel_dp.c   |  3 +++
 drivers/gpu/drm/i915/intel_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c | 51 ++++++++++++++++++++++-----------------
 4 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 3a882a979e5d..c5611e9d9b9c 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2392,6 +2392,8 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
 	intel_encoder->cloneable = 0;
 
+	intel_infoframe_init(intel_dig_port);
+
 	if (init_dp) {
 		if (!intel_ddi_init_dp_connector(intel_dig_port))
 			goto err;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f97cd5305e4c..a2d0ee363307 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5637,6 +5637,9 @@ bool intel_dp_init(struct drm_device *dev,
 	intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
 	dev_priv->hotplug.irq_port[port] = intel_dig_port;
 
+	if (port != PORT_A)
+		intel_infoframe_init(intel_dig_port);
+
 	if (!intel_dp_init_connector(intel_dig_port, intel_connector))
 		goto err_init_connector;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a607799b7776..4c8451e3d8f1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1445,6 +1445,7 @@ struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
 bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			       struct intel_crtc_state *pipe_config);
 void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
+void intel_infoframe_init(struct intel_digital_port *intel_dig_port);
 
 
 /* intel_lvds.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 319f5013923c..637b17baf798 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1801,6 +1801,33 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
+void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
+{
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+
+	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
+		intel_dig_port->write_infoframe = vlv_write_infoframe;
+		intel_dig_port->set_infoframes = vlv_set_infoframes;
+		intel_dig_port->infoframe_enabled = vlv_infoframe_enabled;
+	} else if (IS_G4X(dev)) {
+		intel_dig_port->write_infoframe = g4x_write_infoframe;
+		intel_dig_port->set_infoframes = g4x_set_infoframes;
+		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
+	} else if (HAS_DDI(dev)) {
+		intel_dig_port->write_infoframe = hsw_write_infoframe;
+		intel_dig_port->set_infoframes = hsw_set_infoframes;
+		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
+	} else if (HAS_PCH_IBX(dev)) {
+		intel_dig_port->write_infoframe = ibx_write_infoframe;
+		intel_dig_port->set_infoframes = ibx_set_infoframes;
+		intel_dig_port->infoframe_enabled = ibx_infoframe_enabled;
+	} else {
+		intel_dig_port->write_infoframe = cpt_write_infoframe;
+		intel_dig_port->set_infoframes = cpt_set_infoframes;
+		intel_dig_port->infoframe_enabled = cpt_infoframe_enabled;
+	}
+}
+
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 			       struct intel_connector *intel_connector)
 {
@@ -1883,28 +1910,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 		BUG();
 	}
 
-	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
-		intel_dig_port->write_infoframe = vlv_write_infoframe;
-		intel_dig_port->set_infoframes = vlv_set_infoframes;
-		intel_dig_port->infoframe_enabled = vlv_infoframe_enabled;
-	} else if (IS_G4X(dev)) {
-		intel_dig_port->write_infoframe = g4x_write_infoframe;
-		intel_dig_port->set_infoframes = g4x_set_infoframes;
-		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
-	} else if (HAS_DDI(dev)) {
-		intel_dig_port->write_infoframe = hsw_write_infoframe;
-		intel_dig_port->set_infoframes = hsw_set_infoframes;
-		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
-	} else if (HAS_PCH_IBX(dev)) {
-		intel_dig_port->write_infoframe = ibx_write_infoframe;
-		intel_dig_port->set_infoframes = ibx_set_infoframes;
-		intel_dig_port->infoframe_enabled = ibx_infoframe_enabled;
-	} else {
-		intel_dig_port->write_infoframe = cpt_write_infoframe;
-		intel_dig_port->set_infoframes = cpt_set_infoframes;
-		intel_dig_port->infoframe_enabled = cpt_infoframe_enabled;
-	}
-
 	if (HAS_DDI(dev))
 		intel_connector->get_hw_state = intel_ddi_connector_get_hw_state;
 	else
@@ -2002,5 +2007,7 @@ void intel_hdmi_init(struct drm_device *dev,
 	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
 	intel_dig_port->max_lanes = 4;
 
+	intel_infoframe_init(intel_dig_port);
+
 	intel_hdmi_init_connector(intel_dig_port, intel_connector);
 }
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: Init infoframe vfuncs for DP encoders as well

[Sharma, Shashank]

Reviewed-by: Shashank Sharma

Regards
Shashank

On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> DP ports may want to use the video DIP for SDP transmission, so let's
> initiialize the vfuncs for DP encoders as well. The only exception is
> port A eDP prior to HSW as that one doesn't have a video DIP instance.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c  |  2 ++
>   drivers/gpu/drm/i915/intel_dp.c   |  3 +++
>   drivers/gpu/drm/i915/intel_drv.h  |  1 +
>   drivers/gpu/drm/i915/intel_hdmi.c | 51 ++++++++++++++++++++++-----------------
>   4 files changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 3a882a979e5d..c5611e9d9b9c 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2392,6 +2392,8 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>   	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>   	intel_encoder->cloneable = 0;
>
> +	intel_infoframe_init(intel_dig_port);
> +
>   	if (init_dp) {
>   		if (!intel_ddi_init_dp_connector(intel_dig_port))
>   			goto err;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f97cd5305e4c..a2d0ee363307 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5637,6 +5637,9 @@ bool intel_dp_init(struct drm_device *dev,
>   	intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
>   	dev_priv->hotplug.irq_port[port] = intel_dig_port;
>
> +	if (port != PORT_A)
> +		intel_infoframe_init(intel_dig_port);
> +
>   	if (!intel_dp_init_connector(intel_dig_port, intel_connector))
>   		goto err_init_connector;
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a607799b7776..4c8451e3d8f1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1445,6 +1445,7 @@ struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
>   bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>   			       struct intel_crtc_state *pipe_config);
>   void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
> +void intel_infoframe_init(struct intel_digital_port *intel_dig_port);
>
>
>   /* intel_lvds.c */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 319f5013923c..637b17baf798 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1801,6 +1801,33 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
>   	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>   }
>
> +void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
> +{
> +	struct drm_device *dev = intel_dig_port->base.base.dev;
> +
> +	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> +		intel_dig_port->write_infoframe = vlv_write_infoframe;
> +		intel_dig_port->set_infoframes = vlv_set_infoframes;
> +		intel_dig_port->infoframe_enabled = vlv_infoframe_enabled;
> +	} else if (IS_G4X(dev)) {
> +		intel_dig_port->write_infoframe = g4x_write_infoframe;
> +		intel_dig_port->set_infoframes = g4x_set_infoframes;
> +		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
> +	} else if (HAS_DDI(dev)) {
> +		intel_dig_port->write_infoframe = hsw_write_infoframe;
> +		intel_dig_port->set_infoframes = hsw_set_infoframes;
> +		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
> +	} else if (HAS_PCH_IBX(dev)) {
> +		intel_dig_port->write_infoframe = ibx_write_infoframe;
> +		intel_dig_port->set_infoframes = ibx_set_infoframes;
> +		intel_dig_port->infoframe_enabled = ibx_infoframe_enabled;
> +	} else {
> +		intel_dig_port->write_infoframe = cpt_write_infoframe;
> +		intel_dig_port->set_infoframes = cpt_set_infoframes;
> +		intel_dig_port->infoframe_enabled = cpt_infoframe_enabled;
> +	}
> +}
> +
>   void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>   			       struct intel_connector *intel_connector)
>   {
> @@ -1883,28 +1910,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>   		BUG();
>   	}
>
> -	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> -		intel_dig_port->write_infoframe = vlv_write_infoframe;
> -		intel_dig_port->set_infoframes = vlv_set_infoframes;
> -		intel_dig_port->infoframe_enabled = vlv_infoframe_enabled;
> -	} else if (IS_G4X(dev)) {
> -		intel_dig_port->write_infoframe = g4x_write_infoframe;
> -		intel_dig_port->set_infoframes = g4x_set_infoframes;
> -		intel_dig_port->infoframe_enabled = g4x_infoframe_enabled;
> -	} else if (HAS_DDI(dev)) {
> -		intel_dig_port->write_infoframe = hsw_write_infoframe;
> -		intel_dig_port->set_infoframes = hsw_set_infoframes;
> -		intel_dig_port->infoframe_enabled = hsw_infoframe_enabled;
> -	} else if (HAS_PCH_IBX(dev)) {
> -		intel_dig_port->write_infoframe = ibx_write_infoframe;
> -		intel_dig_port->set_infoframes = ibx_set_infoframes;
> -		intel_dig_port->infoframe_enabled = ibx_infoframe_enabled;
> -	} else {
> -		intel_dig_port->write_infoframe = cpt_write_infoframe;
> -		intel_dig_port->set_infoframes = cpt_set_infoframes;
> -		intel_dig_port->infoframe_enabled = cpt_infoframe_enabled;
> -	}
> -
>   	if (HAS_DDI(dev))
>   		intel_connector->get_hw_state = intel_ddi_connector_get_hw_state;
>   	else
> @@ -2002,5 +2007,7 @@ void intel_hdmi_init(struct drm_device *dev,
>   	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
>   	intel_dig_port->max_lanes = 4;
>
> +	intel_infoframe_init(intel_dig_port);
> +
>   	intel_hdmi_init_connector(intel_dig_port, intel_connector);
>   }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/7] drm/i915: Remove mostly duplicated video DIP handling from PSR code

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that the infoframe hooks are part of the intel_dig_port, we can use
the normal .write_infoframe() hook to update the VSC SDP. We do need to
deal with the size difference between the VSC DIP and the others though.

Another minor snag is that the compiler will complain to use if we keep
using enum hdmi_infoframe_type type and passing in the DP define instead,
so et's just change to unsigned int all over for the inforframe type.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  |  2 +-
 drivers/gpu/drm/i915/intel_hdmi.c | 26 +++++++++++++++----------
 drivers/gpu/drm/i915/intel_psr.c  | 41 ++++++++-------------------------------
 3 files changed, 25 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4c8451e3d8f1..5dcaa14ff90d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -900,7 +900,7 @@ struct intel_digital_port {
 	/* for communication with audio component; protected by av_mutex */
 	const struct drm_connector *audio_connector;
 	void (*write_infoframe)(struct drm_encoder *encoder,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len);
 	void (*set_infoframes)(struct drm_encoder *encoder,
 			       bool enable,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 637b17baf798..600a58210450 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -68,7 +68,7 @@ static struct intel_hdmi *intel_attached_hdmi(struct drm_connector *connector)
 	return enc_to_intel_hdmi(&intel_attached_encoder(connector)->base);
 }
 
-static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
+static u32 g4x_infoframe_index(unsigned int type)
 {
 	switch (type) {
 	case HDMI_INFOFRAME_TYPE_AVI:
@@ -83,7 +83,7 @@ static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
 	}
 }
 
-static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
+static u32 g4x_infoframe_enable(unsigned int type)
 {
 	switch (type) {
 	case HDMI_INFOFRAME_TYPE_AVI:
@@ -98,9 +98,11 @@ static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
 	}
 }
 
-static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
+static u32 hsw_infoframe_enable(unsigned int type)
 {
 	switch (type) {
+	case DP_SDP_VSC:
+		return VIDEO_DIP_ENABLE_VSC_HSW;
 	case HDMI_INFOFRAME_TYPE_AVI:
 		return VIDEO_DIP_ENABLE_AVI_HSW;
 	case HDMI_INFOFRAME_TYPE_SPD:
@@ -116,10 +118,12 @@ static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
 static i915_reg_t
 hsw_dip_data_reg(struct drm_i915_private *dev_priv,
 		 enum transcoder cpu_transcoder,
-		 enum hdmi_infoframe_type type,
+		 unsigned int type,
 		 int i)
 {
 	switch (type) {
+	case DP_SDP_VSC:
+		return HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder, i);
 	case HDMI_INFOFRAME_TYPE_AVI:
 		return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder, i);
 	case HDMI_INFOFRAME_TYPE_SPD:
@@ -133,7 +137,7 @@ hsw_dip_data_reg(struct drm_i915_private *dev_priv,
 }
 
 static void g4x_write_infoframe(struct drm_encoder *encoder,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len)
 {
 	const uint32_t *data = frame;
@@ -187,7 +191,7 @@ static bool g4x_infoframe_enabled(struct drm_encoder *encoder,
 }
 
 static void ibx_write_infoframe(struct drm_encoder *encoder,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len)
 {
 	const uint32_t *data = frame;
@@ -246,7 +250,7 @@ static bool ibx_infoframe_enabled(struct drm_encoder *encoder,
 }
 
 static void cpt_write_infoframe(struct drm_encoder *encoder,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len)
 {
 	const uint32_t *data = frame;
@@ -303,7 +307,7 @@ static bool cpt_infoframe_enabled(struct drm_encoder *encoder,
 }
 
 static void vlv_write_infoframe(struct drm_encoder *encoder,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len)
 {
 	const uint32_t *data = frame;
@@ -361,7 +365,7 @@ static bool vlv_infoframe_enabled(struct drm_encoder *encoder,
 }
 
 static void hsw_write_infoframe(struct drm_encoder *encoder,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len)
 {
 	const uint32_t *data = frame;
@@ -371,6 +375,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
 	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
 	i915_reg_t data_reg;
+	int data_size = type == DP_SDP_VSC ?
+		VIDEO_DIP_VSC_DATA_SIZE : VIDEO_DIP_DATA_SIZE;
 	int i;
 	u32 val = I915_READ(ctl_reg);
 
@@ -386,7 +392,7 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
 		data++;
 	}
 	/* Write every possible data byte to force correct ECC calculation. */
-	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
+	for (; i < data_size; i += 4)
 		I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
 					    type, i >> 2), 0);
 	mmiowb();
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 29a09bf6bd18..904994dd1c9e 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -72,37 +72,6 @@ static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
 	       (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
 }
 
-static void intel_psr_write_vsc(struct intel_dp *intel_dp,
-				const struct edp_vsc_psr *vsc_psr)
-{
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = dig_port->base.base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
-	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
-	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
-	uint32_t *data = (uint32_t *) vsc_psr;
-	unsigned int i;
-
-	/* As per BSPec (Pipe Video Data Island Packet), we need to disable
-	   the video DIP being updated before program video DIP data buffer
-	   registers for DIP being updated. */
-	I915_WRITE(ctl_reg, 0);
-	POSTING_READ(ctl_reg);
-
-	for (i = 0; i < sizeof(*vsc_psr); i += 4) {
-		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
-						   i >> 2), *data);
-		data++;
-	}
-	for (; i < VIDEO_DIP_VSC_DATA_SIZE; i += 4)
-		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
-						   i >> 2), 0);
-
-	I915_WRITE(ctl_reg, VIDEO_DIP_ENABLE_VSC_HSW);
-	POSTING_READ(ctl_reg);
-}
-
 static void vlv_psr_setup_vsc(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -121,6 +90,7 @@ static void vlv_psr_setup_vsc(struct intel_dp *intel_dp)
 
 static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp)
 {
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct edp_vsc_psr psr_vsc;
 
 	/* Prepare VSC Header for SU as per EDP 1.4 spec, Table 6.11 */
@@ -129,11 +99,14 @@ static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp)
 	psr_vsc.sdp_header.HB1 = 0x7;
 	psr_vsc.sdp_header.HB2 = 0x3;
 	psr_vsc.sdp_header.HB3 = 0xb;
-	intel_psr_write_vsc(intel_dp, &psr_vsc);
+
+	intel_dig_port->write_infoframe(&intel_dig_port->base.base,
+					DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc));
 }
 
 static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
 {
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct edp_vsc_psr psr_vsc;
 
 	/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
@@ -142,7 +115,9 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
 	psr_vsc.sdp_header.HB1 = 0x7;
 	psr_vsc.sdp_header.HB2 = 0x2;
 	psr_vsc.sdp_header.HB3 = 0x8;
-	intel_psr_write_vsc(intel_dp, &psr_vsc);
+
+	intel_dig_port->write_infoframe(&intel_dig_port->base.base,
+					DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc));
 }
 
 static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/i915: Remove mostly duplicated video DIP handling from PSR code

[Sharma, Shashank]

Regards
Shashank

On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that the infoframe hooks are part of the intel_dig_port, we can use
> the normal .write_infoframe() hook to update the VSC SDP. We do need to
> deal with the size difference between the VSC DIP and the others though.
>
> Another minor snag is that the compiler will complain to use if we keep
> using enum hdmi_infoframe_type type and passing in the DP define instead,
> so et's just change to unsigned int all over for the inforframe type.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_drv.h  |  2 +-
>   drivers/gpu/drm/i915/intel_hdmi.c | 26 +++++++++++++++----------
>   drivers/gpu/drm/i915/intel_psr.c  | 41 ++++++++-------------------------------
>   3 files changed, 25 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4c8451e3d8f1..5dcaa14ff90d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -900,7 +900,7 @@ struct intel_digital_port {
>   	/* for communication with audio component; protected by av_mutex */
>   	const struct drm_connector *audio_connector;
>   	void (*write_infoframe)(struct drm_encoder *encoder,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>   				const void *frame, ssize_t len);
>   	void (*set_infoframes)(struct drm_encoder *encoder,
>   			       bool enable,
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 637b17baf798..600a58210450 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -68,7 +68,7 @@ static struct intel_hdmi *intel_attached_hdmi(struct drm_connector *connector)
>   	return enc_to_intel_hdmi(&intel_attached_encoder(connector)->base);
>   }
>
> -static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
> +static u32 g4x_infoframe_index(unsigned int type)
>   {
>   	switch (type) {
>   	case HDMI_INFOFRAME_TYPE_AVI:
> @@ -83,7 +83,7 @@ static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
>   	}
>   }
>
> -static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
> +static u32 g4x_infoframe_enable(unsigned int type)
>   {
>   	switch (type) {
>   	case HDMI_INFOFRAME_TYPE_AVI:
> @@ -98,9 +98,11 @@ static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
>   	}
>   }
>
> -static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
> +static u32 hsw_infoframe_enable(unsigned int type)
>   {
>   	switch (type) {
> +	case DP_SDP_VSC:
Why do we need a new field like this ? Would it make sense to set the 
type itself as "HDMI_INFOFRAME_TYPE_AVI" ?
> +		return VIDEO_DIP_ENABLE_VSC_HSW;
>   	case HDMI_INFOFRAME_TYPE_AVI:
>   		return VIDEO_DIP_ENABLE_AVI_HSW;
>   	case HDMI_INFOFRAME_TYPE_SPD:
> @@ -116,10 +118,12 @@ static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
>   static i915_reg_t
>   hsw_dip_data_reg(struct drm_i915_private *dev_priv,
>   		 enum transcoder cpu_transcoder,
> -		 enum hdmi_infoframe_type type,
> +		 unsigned int type,
>   		 int i)
>   {
>   	switch (type) {
> +	case DP_SDP_VSC:
> +		return HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder, i);
>   	case HDMI_INFOFRAME_TYPE_AVI:
>   		return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder, i);
>   	case HDMI_INFOFRAME_TYPE_SPD:
> @@ -133,7 +137,7 @@ hsw_dip_data_reg(struct drm_i915_private *dev_priv,
>   }
>
>   static void g4x_write_infoframe(struct drm_encoder *encoder,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>   				const void *frame, ssize_t len)
>   {
>   	const uint32_t *data = frame;
> @@ -187,7 +191,7 @@ static bool g4x_infoframe_enabled(struct drm_encoder *encoder,
>   }
>
>   static void ibx_write_infoframe(struct drm_encoder *encoder,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>   				const void *frame, ssize_t len)
>   {
>   	const uint32_t *data = frame;
> @@ -246,7 +250,7 @@ static bool ibx_infoframe_enabled(struct drm_encoder *encoder,
>   }
>
>   static void cpt_write_infoframe(struct drm_encoder *encoder,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>   				const void *frame, ssize_t len)
>   {
>   	const uint32_t *data = frame;
> @@ -303,7 +307,7 @@ static bool cpt_infoframe_enabled(struct drm_encoder *encoder,
>   }
>
>   static void vlv_write_infoframe(struct drm_encoder *encoder,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>   				const void *frame, ssize_t len)
>   {
>   	const uint32_t *data = frame;
> @@ -361,7 +365,7 @@ static bool vlv_infoframe_enabled(struct drm_encoder *encoder,
>   }
>
>   static void hsw_write_infoframe(struct drm_encoder *encoder,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>   				const void *frame, ssize_t len)
>   {
>   	const uint32_t *data = frame;
> @@ -371,6 +375,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
>   	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>   	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
>   	i915_reg_t data_reg;
> +	int data_size = type == DP_SDP_VSC ?
Ok, I think may be this is the reason why we need a separate filed for 
DP_SDP_VSC, is it so ?

- Shashank
> +		VIDEO_DIP_VSC_DATA_SIZE : VIDEO_DIP_DATA_SIZE;
>   	int i;
>   	u32 val = I915_READ(ctl_reg);
>
> @@ -386,7 +392,7 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
>   		data++;
>   	}
>   	/* Write every possible data byte to force correct ECC calculation. */
> -	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
> +	for (; i < data_size; i += 4)
>   		I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
>   					    type, i >> 2), 0);
>   	mmiowb();
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 29a09bf6bd18..904994dd1c9e 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -72,37 +72,6 @@ static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
>   	       (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
>   }
>
> -static void intel_psr_write_vsc(struct intel_dp *intel_dp,
> -				const struct edp_vsc_psr *vsc_psr)
> -{
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_device *dev = dig_port->base.base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
> -	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> -	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
> -	uint32_t *data = (uint32_t *) vsc_psr;
> -	unsigned int i;
> -
> -	/* As per BSPec (Pipe Video Data Island Packet), we need to disable
> -	   the video DIP being updated before program video DIP data buffer
> -	   registers for DIP being updated. */
> -	I915_WRITE(ctl_reg, 0);
> -	POSTING_READ(ctl_reg);
> -
> -	for (i = 0; i < sizeof(*vsc_psr); i += 4) {
> -		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
> -						   i >> 2), *data);
> -		data++;
> -	}
> -	for (; i < VIDEO_DIP_VSC_DATA_SIZE; i += 4)
> -		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
> -						   i >> 2), 0);
> -
> -	I915_WRITE(ctl_reg, VIDEO_DIP_ENABLE_VSC_HSW);
> -	POSTING_READ(ctl_reg);
> -}
> -
>   static void vlv_psr_setup_vsc(struct intel_dp *intel_dp)
>   {
>   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> @@ -121,6 +90,7 @@ static void vlv_psr_setup_vsc(struct intel_dp *intel_dp)
>
>   static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp)
>   {
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   	struct edp_vsc_psr psr_vsc;
>
>   	/* Prepare VSC Header for SU as per EDP 1.4 spec, Table 6.11 */
> @@ -129,11 +99,14 @@ static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp)
>   	psr_vsc.sdp_header.HB1 = 0x7;
>   	psr_vsc.sdp_header.HB2 = 0x3;
>   	psr_vsc.sdp_header.HB3 = 0xb;
> -	intel_psr_write_vsc(intel_dp, &psr_vsc);
> +
> +	intel_dig_port->write_infoframe(&intel_dig_port->base.base,
> +					DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc));
>   }
>
>   static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
>   {
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   	struct edp_vsc_psr psr_vsc;
>
>   	/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
> @@ -142,7 +115,9 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
>   	psr_vsc.sdp_header.HB1 = 0x7;
>   	psr_vsc.sdp_header.HB2 = 0x2;
>   	psr_vsc.sdp_header.HB3 = 0x8;
> -	intel_psr_write_vsc(intel_dp, &psr_vsc);
> +
> +	intel_dig_port->write_infoframe(&intel_dig_port->base.base,
> +					DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc));
>   }
>
>   static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/i915: Remove mostly duplicated video DIP handling from PSR code

[Ville Syrjälä]

On Mon, Jun 13, 2016 at 05:39:47PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Now that the infoframe hooks are part of the intel_dig_port, we can use
> > the normal .write_infoframe() hook to update the VSC SDP. We do need to
> > deal with the size difference between the VSC DIP and the others though.
> >
> > Another minor snag is that the compiler will complain to use if we keep
> > using enum hdmi_infoframe_type type and passing in the DP define instead,
> > so et's just change to unsigned int all over for the inforframe type.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_drv.h  |  2 +-
> >   drivers/gpu/drm/i915/intel_hdmi.c | 26 +++++++++++++++----------
> >   drivers/gpu/drm/i915/intel_psr.c  | 41 ++++++++-------------------------------
> >   3 files changed, 25 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 4c8451e3d8f1..5dcaa14ff90d 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -900,7 +900,7 @@ struct intel_digital_port {
> >   	/* for communication with audio component; protected by av_mutex */
> >   	const struct drm_connector *audio_connector;
> >   	void (*write_infoframe)(struct drm_encoder *encoder,
> > -				enum hdmi_infoframe_type type,
> > +				unsigned int type,
> >   				const void *frame, ssize_t len);
> >   	void (*set_infoframes)(struct drm_encoder *encoder,
> >   			       bool enable,
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 637b17baf798..600a58210450 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -68,7 +68,7 @@ static struct intel_hdmi *intel_attached_hdmi(struct drm_connector *connector)
> >   	return enc_to_intel_hdmi(&intel_attached_encoder(connector)->base);
> >   }
> >
> > -static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
> > +static u32 g4x_infoframe_index(unsigned int type)
> >   {
> >   	switch (type) {
> >   	case HDMI_INFOFRAME_TYPE_AVI:
> > @@ -83,7 +83,7 @@ static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
> >   	}
> >   }
> >
> > -static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
> > +static u32 g4x_infoframe_enable(unsigned int type)
> >   {
> >   	switch (type) {
> >   	case HDMI_INFOFRAME_TYPE_AVI:
> > @@ -98,9 +98,11 @@ static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
> >   	}
> >   }
> >
> > -static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
> > +static u32 hsw_infoframe_enable(unsigned int type)
> >   {
> >   	switch (type) {
> > +	case DP_SDP_VSC:
> Why do we need a new field like this ? Would it make sense to set the 
> type itself as "HDMI_INFOFRAME_TYPE_AVI" ?

We want to enable/write the VSC, not the AVI.

> > +		return VIDEO_DIP_ENABLE_VSC_HSW;
> >   	case HDMI_INFOFRAME_TYPE_AVI:
> >   		return VIDEO_DIP_ENABLE_AVI_HSW;
> >   	case HDMI_INFOFRAME_TYPE_SPD:
> > @@ -116,10 +118,12 @@ static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
> >   static i915_reg_t
> >   hsw_dip_data_reg(struct drm_i915_private *dev_priv,
> >   		 enum transcoder cpu_transcoder,
> > -		 enum hdmi_infoframe_type type,
> > +		 unsigned int type,
> >   		 int i)
> >   {
> >   	switch (type) {
> > +	case DP_SDP_VSC:
> > +		return HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder, i);
> >   	case HDMI_INFOFRAME_TYPE_AVI:
> >   		return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder, i);
> >   	case HDMI_INFOFRAME_TYPE_SPD:
> > @@ -133,7 +137,7 @@ hsw_dip_data_reg(struct drm_i915_private *dev_priv,
> >   }
> >
> >   static void g4x_write_infoframe(struct drm_encoder *encoder,
> > -				enum hdmi_infoframe_type type,
> > +				unsigned int type,
> >   				const void *frame, ssize_t len)
> >   {
> >   	const uint32_t *data = frame;
> > @@ -187,7 +191,7 @@ static bool g4x_infoframe_enabled(struct drm_encoder *encoder,
> >   }
> >
> >   static void ibx_write_infoframe(struct drm_encoder *encoder,
> > -				enum hdmi_infoframe_type type,
> > +				unsigned int type,
> >   				const void *frame, ssize_t len)
> >   {
> >   	const uint32_t *data = frame;
> > @@ -246,7 +250,7 @@ static bool ibx_infoframe_enabled(struct drm_encoder *encoder,
> >   }
> >
> >   static void cpt_write_infoframe(struct drm_encoder *encoder,
> > -				enum hdmi_infoframe_type type,
> > +				unsigned int type,
> >   				const void *frame, ssize_t len)
> >   {
> >   	const uint32_t *data = frame;
> > @@ -303,7 +307,7 @@ static bool cpt_infoframe_enabled(struct drm_encoder *encoder,
> >   }
> >
> >   static void vlv_write_infoframe(struct drm_encoder *encoder,
> > -				enum hdmi_infoframe_type type,
> > +				unsigned int type,
> >   				const void *frame, ssize_t len)
> >   {
> >   	const uint32_t *data = frame;
> > @@ -361,7 +365,7 @@ static bool vlv_infoframe_enabled(struct drm_encoder *encoder,
> >   }
> >
> >   static void hsw_write_infoframe(struct drm_encoder *encoder,
> > -				enum hdmi_infoframe_type type,
> > +				unsigned int type,
> >   				const void *frame, ssize_t len)
> >   {
> >   	const uint32_t *data = frame;
> > @@ -371,6 +375,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
> >   	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> >   	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
> >   	i915_reg_t data_reg;
> > +	int data_size = type == DP_SDP_VSC ?
> Ok, I think may be this is the reason why we need a separate filed for 
> DP_SDP_VSC, is it so ?

No, we need to regarless of the size of the buffer.

> 
> - Shashank
> > +		VIDEO_DIP_VSC_DATA_SIZE : VIDEO_DIP_DATA_SIZE;
> >   	int i;
> >   	u32 val = I915_READ(ctl_reg);
> >
> > @@ -386,7 +392,7 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
> >   		data++;
> >   	}
> >   	/* Write every possible data byte to force correct ECC calculation. */
> > -	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
> > +	for (; i < data_size; i += 4)
> >   		I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
> >   					    type, i >> 2), 0);
> >   	mmiowb();
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 29a09bf6bd18..904994dd1c9e 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -72,37 +72,6 @@ static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> >   	       (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
> >   }
> >
> > -static void intel_psr_write_vsc(struct intel_dp *intel_dp,
> > -				const struct edp_vsc_psr *vsc_psr)
> > -{
> > -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > -	struct drm_device *dev = dig_port->base.base.dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
> > -	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> > -	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
> > -	uint32_t *data = (uint32_t *) vsc_psr;
> > -	unsigned int i;
> > -
> > -	/* As per BSPec (Pipe Video Data Island Packet), we need to disable
> > -	   the video DIP being updated before program video DIP data buffer
> > -	   registers for DIP being updated. */
> > -	I915_WRITE(ctl_reg, 0);
> > -	POSTING_READ(ctl_reg);
> > -
> > -	for (i = 0; i < sizeof(*vsc_psr); i += 4) {
> > -		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
> > -						   i >> 2), *data);
> > -		data++;
> > -	}
> > -	for (; i < VIDEO_DIP_VSC_DATA_SIZE; i += 4)
> > -		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
> > -						   i >> 2), 0);
> > -
> > -	I915_WRITE(ctl_reg, VIDEO_DIP_ENABLE_VSC_HSW);
> > -	POSTING_READ(ctl_reg);
> > -}
> > -
> >   static void vlv_psr_setup_vsc(struct intel_dp *intel_dp)
> >   {
> >   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > @@ -121,6 +90,7 @@ static void vlv_psr_setup_vsc(struct intel_dp *intel_dp)
> >
> >   static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp)
> >   {
> > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >   	struct edp_vsc_psr psr_vsc;
> >
> >   	/* Prepare VSC Header for SU as per EDP 1.4 spec, Table 6.11 */
> > @@ -129,11 +99,14 @@ static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp)
> >   	psr_vsc.sdp_header.HB1 = 0x7;
> >   	psr_vsc.sdp_header.HB2 = 0x3;
> >   	psr_vsc.sdp_header.HB3 = 0xb;
> > -	intel_psr_write_vsc(intel_dp, &psr_vsc);
> > +
> > +	intel_dig_port->write_infoframe(&intel_dig_port->base.base,
> > +					DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc));
> >   }
> >
> >   static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
> >   {
> > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >   	struct edp_vsc_psr psr_vsc;
> >
> >   	/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
> > @@ -142,7 +115,9 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
> >   	psr_vsc.sdp_header.HB1 = 0x7;
> >   	psr_vsc.sdp_header.HB2 = 0x2;
> >   	psr_vsc.sdp_header.HB3 = 0x8;
> > -	intel_psr_write_vsc(intel_dp, &psr_vsc);
> > +
> > +	intel_dig_port->write_infoframe(&intel_dig_port->base.base,
> > +					DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc));
> >   }
> >
> >   static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
> >

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/i915: Remove mostly duplicated video DIP handling from PSR code

[Sharma, Shashank]

Regards
Shashank

On 6/13/2016 5:57 PM, Ville Syrjälä wrote:
> On Mon, Jun 13, 2016 at 05:39:47PM +0530, Sharma, Shashank wrote:
>> Regards
>> Shashank
>>
>> On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Now that the infoframe hooks are part of the intel_dig_port, we can use
>>> the normal .write_infoframe() hook to update the VSC SDP. We do need to
>>> deal with the size difference between the VSC DIP and the others though.
>>>
>>> Another minor snag is that the compiler will complain to use if we keep
>>> using enum hdmi_infoframe_type type and passing in the DP define instead,
>>> so et's just change to unsigned int all over for the inforframe type.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_drv.h  |  2 +-
>>>    drivers/gpu/drm/i915/intel_hdmi.c | 26 +++++++++++++++----------
>>>    drivers/gpu/drm/i915/intel_psr.c  | 41 ++++++++-------------------------------
>>>    3 files changed, 25 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index 4c8451e3d8f1..5dcaa14ff90d 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -900,7 +900,7 @@ struct intel_digital_port {
>>>    	/* for communication with audio component; protected by av_mutex */
>>>    	const struct drm_connector *audio_connector;
>>>    	void (*write_infoframe)(struct drm_encoder *encoder,
>>> -				enum hdmi_infoframe_type type,
>>> +				unsigned int type,
>>>    				const void *frame, ssize_t len);
>>>    	void (*set_infoframes)(struct drm_encoder *encoder,
>>>    			       bool enable,
>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>>> index 637b17baf798..600a58210450 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>> @@ -68,7 +68,7 @@ static struct intel_hdmi *intel_attached_hdmi(struct drm_connector *connector)
>>>    	return enc_to_intel_hdmi(&intel_attached_encoder(connector)->base);
>>>    }
>>>
>>> -static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
>>> +static u32 g4x_infoframe_index(unsigned int type)
>>>    {
>>>    	switch (type) {
>>>    	case HDMI_INFOFRAME_TYPE_AVI:
>>> @@ -83,7 +83,7 @@ static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
>>>    	}
>>>    }
>>>
>>> -static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
>>> +static u32 g4x_infoframe_enable(unsigned int type)
>>>    {
>>>    	switch (type) {
>>>    	case HDMI_INFOFRAME_TYPE_AVI:
>>> @@ -98,9 +98,11 @@ static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
>>>    	}
>>>    }
>>>
>>> -static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
>>> +static u32 hsw_infoframe_enable(unsigned int type)
>>>    {
>>>    	switch (type) {
>>> +	case DP_SDP_VSC:
>> Why do we need a new field like this ? Would it make sense to set the
>> type itself as "HDMI_INFOFRAME_TYPE_AVI" ?
>
> We want to enable/write the VSC, not the AVI.
>
Ok, This patch generally looks good, but I am not sure I am the right 
guy to check this patch further, so I will pass it from here. We might 
need another opinion on this.
>>> +		return VIDEO_DIP_ENABLE_VSC_HSW;
>>>    	case HDMI_INFOFRAME_TYPE_AVI:
>>>    		return VIDEO_DIP_ENABLE_AVI_HSW;
>>>    	case HDMI_INFOFRAME_TYPE_SPD:
>>> @@ -116,10 +118,12 @@ static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
>>>    static i915_reg_t
>>>    hsw_dip_data_reg(struct drm_i915_private *dev_priv,
>>>    		 enum transcoder cpu_transcoder,
>>> -		 enum hdmi_infoframe_type type,
>>> +		 unsigned int type,
>>>    		 int i)
>>>    {
>>>    	switch (type) {
>>> +	case DP_SDP_VSC:
>>> +		return HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder, i);
>>>    	case HDMI_INFOFRAME_TYPE_AVI:
>>>    		return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder, i);
>>>    	case HDMI_INFOFRAME_TYPE_SPD:
>>> @@ -133,7 +137,7 @@ hsw_dip_data_reg(struct drm_i915_private *dev_priv,
>>>    }
>>>
>>>    static void g4x_write_infoframe(struct drm_encoder *encoder,
>>> -				enum hdmi_infoframe_type type,
>>> +				unsigned int type,
>>>    				const void *frame, ssize_t len)
>>>    {
>>>    	const uint32_t *data = frame;
>>> @@ -187,7 +191,7 @@ static bool g4x_infoframe_enabled(struct drm_encoder *encoder,
>>>    }
>>>
>>>    static void ibx_write_infoframe(struct drm_encoder *encoder,
>>> -				enum hdmi_infoframe_type type,
>>> +				unsigned int type,
>>>    				const void *frame, ssize_t len)
>>>    {
>>>    	const uint32_t *data = frame;
>>> @@ -246,7 +250,7 @@ static bool ibx_infoframe_enabled(struct drm_encoder *encoder,
>>>    }
>>>
>>>    static void cpt_write_infoframe(struct drm_encoder *encoder,
>>> -				enum hdmi_infoframe_type type,
>>> +				unsigned int type,
>>>    				const void *frame, ssize_t len)
>>>    {
>>>    	const uint32_t *data = frame;
>>> @@ -303,7 +307,7 @@ static bool cpt_infoframe_enabled(struct drm_encoder *encoder,
>>>    }
>>>
>>>    static void vlv_write_infoframe(struct drm_encoder *encoder,
>>> -				enum hdmi_infoframe_type type,
>>> +				unsigned int type,
>>>    				const void *frame, ssize_t len)
>>>    {
>>>    	const uint32_t *data = frame;
>>> @@ -361,7 +365,7 @@ static bool vlv_infoframe_enabled(struct drm_encoder *encoder,
>>>    }
>>>
>>>    static void hsw_write_infoframe(struct drm_encoder *encoder,
>>> -				enum hdmi_infoframe_type type,
>>> +				unsigned int type,
>>>    				const void *frame, ssize_t len)
>>>    {
>>>    	const uint32_t *data = frame;
>>> @@ -371,6 +375,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
>>>    	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>>>    	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
>>>    	i915_reg_t data_reg;
>>> +	int data_size = type == DP_SDP_VSC ?
>> Ok, I think may be this is the reason why we need a separate filed for
>> DP_SDP_VSC, is it so ?
>
> No, we need to regarless of the size of the buffer.
>
>>
>> - Shashank
>>> +		VIDEO_DIP_VSC_DATA_SIZE : VIDEO_DIP_DATA_SIZE;
>>>    	int i;
>>>    	u32 val = I915_READ(ctl_reg);
>>>
>>> @@ -386,7 +392,7 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
>>>    		data++;
>>>    	}
>>>    	/* Write every possible data byte to force correct ECC calculation. */
>>> -	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
>>> +	for (; i < data_size; i += 4)
>>>    		I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
>>>    					    type, i >> 2), 0);
>>>    	mmiowb();
>>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>>> index 29a09bf6bd18..904994dd1c9e 100644
>>> --- a/drivers/gpu/drm/i915/intel_psr.c
>>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>>> @@ -72,37 +72,6 @@ static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
>>>    	       (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
>>>    }
>>>
>>> -static void intel_psr_write_vsc(struct intel_dp *intel_dp,
>>> -				const struct edp_vsc_psr *vsc_psr)
>>> -{
>>> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>>> -	struct drm_device *dev = dig_port->base.base.dev;
>>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>>> -	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
>>> -	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
>>> -	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
>>> -	uint32_t *data = (uint32_t *) vsc_psr;
>>> -	unsigned int i;
>>> -
>>> -	/* As per BSPec (Pipe Video Data Island Packet), we need to disable
>>> -	   the video DIP being updated before program video DIP data buffer
>>> -	   registers for DIP being updated. */
>>> -	I915_WRITE(ctl_reg, 0);
>>> -	POSTING_READ(ctl_reg);
>>> -
>>> -	for (i = 0; i < sizeof(*vsc_psr); i += 4) {
>>> -		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
>>> -						   i >> 2), *data);
>>> -		data++;
>>> -	}
>>> -	for (; i < VIDEO_DIP_VSC_DATA_SIZE; i += 4)
>>> -		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
>>> -						   i >> 2), 0);
>>> -
>>> -	I915_WRITE(ctl_reg, VIDEO_DIP_ENABLE_VSC_HSW);
>>> -	POSTING_READ(ctl_reg);
>>> -}
>>> -
>>>    static void vlv_psr_setup_vsc(struct intel_dp *intel_dp)
>>>    {
>>>    	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>> @@ -121,6 +90,7 @@ static void vlv_psr_setup_vsc(struct intel_dp *intel_dp)
>>>
>>>    static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp)
>>>    {
>>> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>>    	struct edp_vsc_psr psr_vsc;
>>>
>>>    	/* Prepare VSC Header for SU as per EDP 1.4 spec, Table 6.11 */
>>> @@ -129,11 +99,14 @@ static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp)
>>>    	psr_vsc.sdp_header.HB1 = 0x7;
>>>    	psr_vsc.sdp_header.HB2 = 0x3;
>>>    	psr_vsc.sdp_header.HB3 = 0xb;
>>> -	intel_psr_write_vsc(intel_dp, &psr_vsc);
>>> +
>>> +	intel_dig_port->write_infoframe(&intel_dig_port->base.base,
>>> +					DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc));
>>>    }
>>>
>>>    static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
>>>    {
>>> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>>    	struct edp_vsc_psr psr_vsc;
>>>
>>>    	/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
>>> @@ -142,7 +115,9 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
>>>    	psr_vsc.sdp_header.HB1 = 0x7;
>>>    	psr_vsc.sdp_header.HB2 = 0x2;
>>>    	psr_vsc.sdp_header.HB3 = 0x8;
>>> -	intel_psr_write_vsc(intel_dp, &psr_vsc);
>>> +
>>> +	intel_dig_port->write_infoframe(&intel_dig_port->base.base,
>>> +					DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc));
>>>    }
>>>
>>>    static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/7] drm/i915: Allow DP ports to set/readout infoframe state (WIP)

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The video DIP can be used with DP ports as well. So let's at least read
out the state, and disable all infoframes when disabling the port.
Otherwise we might get left with whatever the previous guy was doing.

If we were totally paranaoid, I suppose we might consider doing this
for FDI too on DDI platforms. But that would require first decoupling
the infoframe code from intel_digital_port. So leave it be for now at
least.

FIXME need to figure out how to handle the PSR VSC SDP usage before
doing this, as that might make the state checker unhappy.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 13 ++++++++-----
 drivers/gpu/drm/i915/intel_dp.c  |  9 +++++++++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c5611e9d9b9c..6543feeb58f2 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2157,7 +2157,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
 	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
-	struct intel_digital_port *intel_dig_port;
 	u32 temp, flags = 0;
 
 	/* XXX: DSI transcoder paranoia */
@@ -2193,13 +2192,17 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 		break;
 	}
 
-	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
-	case TRANS_DDI_MODE_SELECT_HDMI:
-		pipe_config->has_hdmi_sink = true;
-		intel_dig_port = enc_to_dig_port(&encoder->base);
+	if (encoder->type != INTEL_OUTPUT_ANALOG) {
+		struct intel_digital_port *intel_dig_port =
+			enc_to_dig_port(&encoder->base);
 
 		if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
 			pipe_config->has_infoframe = true;
+	}
+
+	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
+	case TRANS_DDI_MODE_SELECT_HDMI:
+		pipe_config->has_hdmi_sink = true;
 		/* fall through */
 	case TRANS_DDI_MODE_SELECT_DVI:
 		pipe_config->lane_count = 4;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a2d0ee363307..b43009ed1dab 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2355,6 +2355,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 				struct intel_crtc_state *pipe_config)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	u32 tmp, flags = 0;
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2395,6 +2396,10 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 	    !IS_CHERRYVIEW(dev) && tmp & DP_COLOR_RANGE_16_235)
 		pipe_config->limited_color_range = true;
 
+	if (intel_dig_port->infoframe_enabled &&
+	    intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
+		pipe_config->has_infoframe = true;
+
 	pipe_config->has_dp_encoder = true;
 
 	pipe_config->lane_count =
@@ -3343,6 +3348,10 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
 	}
 
+	if (intel_dig_port->set_infoframes)
+		intel_dig_port->set_infoframes(&intel_dig_port->base.base,
+					       NULL, false);
+
 	msleep(intel_dp->panel_power_down_delay);
 
 	intel_dp->DP = DP;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915: Allow DP ports to set/readout infoframe state (WIP)

[Sharma, Shashank]

Regards
Shashank

On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The video DIP can be used with DP ports as well. So let's at least read
> out the state, and disable all infoframes when disabling the port.
> Otherwise we might get left with whatever the previous guy was doing.
>
> If we were totally paranaoid, I suppose we might consider doing this
> for FDI too on DDI platforms. But that would require first decoupling
> the infoframe code from intel_digital_port. So leave it be for now at
> least.
>
> FIXME need to figure out how to handle the PSR VSC SDP usage before
> doing this, as that might make the state checker unhappy.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c | 13 ++++++++-----
>   drivers/gpu/drm/i915/intel_dp.c  |  9 +++++++++
>   2 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index c5611e9d9b9c..6543feeb58f2 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2157,7 +2157,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>   	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>   	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>   	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> -	struct intel_digital_port *intel_dig_port;
>   	u32 temp, flags = 0;
>
>   	/* XXX: DSI transcoder paranoia */
> @@ -2193,13 +2192,17 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>   		break;
>   	}
>
> -	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
> -	case TRANS_DDI_MODE_SELECT_HDMI:
> -		pipe_config->has_hdmi_sink = true;
> -		intel_dig_port = enc_to_dig_port(&encoder->base);
> +	if (encoder->type != INTEL_OUTPUT_ANALOG) {
This will be true for INTEL_OUTPUT_UNUSED, UNKNOWN, eDP, and bunch of 
more encoder types. Should we add a bit mask for valid encoder types here ?
> +		struct intel_digital_port *intel_dig_port =
> +			enc_to_dig_port(&encoder->base);
>
>   		if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
>   			pipe_config->has_infoframe = true;
> +	}
> +
> +	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
> +	case TRANS_DDI_MODE_SELECT_HDMI:
> +		pipe_config->has_hdmi_sink = true;
>   		/* fall through */
>   	case TRANS_DDI_MODE_SELECT_DVI:
>   		pipe_config->lane_count = 4;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a2d0ee363307..b43009ed1dab 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2355,6 +2355,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>   				struct intel_crtc_state *pipe_config)
>   {
>   	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   	u32 tmp, flags = 0;
>   	struct drm_device *dev = encoder->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2395,6 +2396,10 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>   	    !IS_CHERRYVIEW(dev) && tmp & DP_COLOR_RANGE_16_235)
>   		pipe_config->limited_color_range = true;
>
> +	if (intel_dig_port->infoframe_enabled &&
> +	    intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
> +		pipe_config->has_infoframe = true;
> +
>   	pipe_config->has_dp_encoder = true;
>
>   	pipe_config->lane_count =
> @@ -3343,6 +3348,10 @@ intel_dp_link_down(struct intel_dp *intel_dp)
>   		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
>   	}
>
> +	if (intel_dig_port->set_infoframes)
> +		intel_dig_port->set_infoframes(&intel_dig_port->base.base,
> +					       NULL, false);
> +
>   	msleep(intel_dp->panel_power_down_delay);
>
>   	intel_dp->DP = DP;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915: Allow DP ports to set/readout infoframe state (WIP)

[Ville Syrjälä]

On Mon, Jun 13, 2016 at 06:58:19PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The video DIP can be used with DP ports as well. So let's at least read
> > out the state, and disable all infoframes when disabling the port.
> > Otherwise we might get left with whatever the previous guy was doing.
> >
> > If we were totally paranaoid, I suppose we might consider doing this
> > for FDI too on DDI platforms. But that would require first decoupling
> > the infoframe code from intel_digital_port. So leave it be for now at
> > least.
> >
> > FIXME need to figure out how to handle the PSR VSC SDP usage before
> > doing this, as that might make the state checker unhappy.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_ddi.c | 13 ++++++++-----
> >   drivers/gpu/drm/i915/intel_dp.c  |  9 +++++++++
> >   2 files changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index c5611e9d9b9c..6543feeb58f2 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2157,7 +2157,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >   	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> >   	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> >   	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > -	struct intel_digital_port *intel_dig_port;
> >   	u32 temp, flags = 0;
> >
> >   	/* XXX: DSI transcoder paranoia */
> > @@ -2193,13 +2192,17 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >   		break;
> >   	}
> >
> > -	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
> > -	case TRANS_DDI_MODE_SELECT_HDMI:
> > -		pipe_config->has_hdmi_sink = true;
> > -		intel_dig_port = enc_to_dig_port(&encoder->base);
> > +	if (encoder->type != INTEL_OUTPUT_ANALOG) {
> This will be true for INTEL_OUTPUT_UNUSED, UNKNOWN, eDP, and bunch of 
> more encoder types. Should we add a bit mask for valid encoder types here ?

We only need to care about FDI vs. not. If it's not FDI, then it'll be
a DDI encoder which has struct intel_digital_port as its base.

> > +		struct intel_digital_port *intel_dig_port =
> > +			enc_to_dig_port(&encoder->base);
> >
> >   		if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
> >   			pipe_config->has_infoframe = true;
> > +	}
> > +
> > +	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
> > +	case TRANS_DDI_MODE_SELECT_HDMI:
> > +		pipe_config->has_hdmi_sink = true;
> >   		/* fall through */
> >   	case TRANS_DDI_MODE_SELECT_DVI:
> >   		pipe_config->lane_count = 4;
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index a2d0ee363307..b43009ed1dab 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2355,6 +2355,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> >   				struct intel_crtc_state *pipe_config)
> >   {
> >   	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >   	u32 tmp, flags = 0;
> >   	struct drm_device *dev = encoder->base.dev;
> >   	struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -2395,6 +2396,10 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> >   	    !IS_CHERRYVIEW(dev) && tmp & DP_COLOR_RANGE_16_235)
> >   		pipe_config->limited_color_range = true;
> >
> > +	if (intel_dig_port->infoframe_enabled &&
> > +	    intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
> > +		pipe_config->has_infoframe = true;
> > +
> >   	pipe_config->has_dp_encoder = true;
> >
> >   	pipe_config->lane_count =
> > @@ -3343,6 +3348,10 @@ intel_dp_link_down(struct intel_dp *intel_dp)
> >   		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> >   	}
> >
> > +	if (intel_dig_port->set_infoframes)
> > +		intel_dig_port->set_infoframes(&intel_dig_port->base.base,
> > +					       NULL, false);
> > +
> >   	msleep(intel_dp->panel_power_down_delay);
> >
> >   	intel_dp->DP = DP;
> >

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915: Allow DP ports to set/readout infoframe state (WIP)

[Sharma, Shashank]

Regards
Shashank

On 6/14/2016 7:46 PM, Ville Syrjälä wrote:
> On Mon, Jun 13, 2016 at 06:58:19PM +0530, Sharma, Shashank wrote:
>> Regards
>> Shashank
>>
>> On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> The video DIP can be used with DP ports as well. So let's at least read
>>> out the state, and disable all infoframes when disabling the port.
>>> Otherwise we might get left with whatever the previous guy was doing.
>>>
>>> If we were totally paranaoid, I suppose we might consider doing this
>>> for FDI too on DDI platforms. But that would require first decoupling
>>> the infoframe code from intel_digital_port. So leave it be for now at
>>> least.
>>>
>>> FIXME need to figure out how to handle the PSR VSC SDP usage before
>>> doing this, as that might make the state checker unhappy.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_ddi.c | 13 ++++++++-----
>>>    drivers/gpu/drm/i915/intel_dp.c  |  9 +++++++++
>>>    2 files changed, 17 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>> index c5611e9d9b9c..6543feeb58f2 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -2157,7 +2157,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>>>    	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>>>    	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>>>    	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
>>> -	struct intel_digital_port *intel_dig_port;
>>>    	u32 temp, flags = 0;
>>>
>>>    	/* XXX: DSI transcoder paranoia */
>>> @@ -2193,13 +2192,17 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>>>    		break;
>>>    	}
>>>
>>> -	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
>>> -	case TRANS_DDI_MODE_SELECT_HDMI:
>>> -		pipe_config->has_hdmi_sink = true;
>>> -		intel_dig_port = enc_to_dig_port(&encoder->base);
>>> +	if (encoder->type != INTEL_OUTPUT_ANALOG) {
>> This will be true for INTEL_OUTPUT_UNUSED, UNKNOWN, eDP, and bunch of
>> more encoder types. Should we add a bit mask for valid encoder types here ?
>
> We only need to care about FDI vs. not. If it's not FDI, then it'll be
> a DDI encoder which has struct intel_digital_port as its base.
But again, what if I got a INTEL_OUTPUT_UNKNOWN here, should we even 
bother to call a infoframe_enabled() for that ? Wont it be better if we 
can just allow (DP | HDMI | eDP etc) ?
>
>>> +		struct intel_digital_port *intel_dig_port =
>>> +			enc_to_dig_port(&encoder->base);
>>>
>>>    		if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
>>>    			pipe_config->has_infoframe = true;
>>> +	}
>>> +
>>> +	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
>>> +	case TRANS_DDI_MODE_SELECT_HDMI:
>>> +		pipe_config->has_hdmi_sink = true;
>>>    		/* fall through */
>>>    	case TRANS_DDI_MODE_SELECT_DVI:
>>>    		pipe_config->lane_count = 4;
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index a2d0ee363307..b43009ed1dab 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -2355,6 +2355,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>>>    				struct intel_crtc_state *pipe_config)
>>>    {
>>>    	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>>> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>>    	u32 tmp, flags = 0;
>>>    	struct drm_device *dev = encoder->base.dev;
>>>    	struct drm_i915_private *dev_priv = dev->dev_private;
>>> @@ -2395,6 +2396,10 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>>>    	    !IS_CHERRYVIEW(dev) && tmp & DP_COLOR_RANGE_16_235)
>>>    		pipe_config->limited_color_range = true;
>>>
>>> +	if (intel_dig_port->infoframe_enabled &&
>>> +	    intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
>>> +		pipe_config->has_infoframe = true;
>>> +
>>>    	pipe_config->has_dp_encoder = true;
>>>
>>>    	pipe_config->lane_count =
>>> @@ -3343,6 +3348,10 @@ intel_dp_link_down(struct intel_dp *intel_dp)
>>>    		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
>>>    	}
>>>
>>> +	if (intel_dig_port->set_infoframes)
>>> +		intel_dig_port->set_infoframes(&intel_dig_port->base.base,
>>> +					       NULL, false);
>>> +
>>>    	msleep(intel_dp->panel_power_down_delay);
>>>
>>>    	intel_dp->DP = DP;
>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915: Allow DP ports to set/readout infoframe state (WIP)

[Ville Syrjälä]

On Tue, Jun 14, 2016 at 08:10:52PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 6/14/2016 7:46 PM, Ville Syrjälä wrote:
> > On Mon, Jun 13, 2016 at 06:58:19PM +0530, Sharma, Shashank wrote:
> >> Regards
> >> Shashank
> >>
> >> On 6/3/2016 1:25 AM, ville.syrjala@linux.intel.com wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> The video DIP can be used with DP ports as well. So let's at least read
> >>> out the state, and disable all infoframes when disabling the port.
> >>> Otherwise we might get left with whatever the previous guy was doing.
> >>>
> >>> If we were totally paranaoid, I suppose we might consider doing this
> >>> for FDI too on DDI platforms. But that would require first decoupling
> >>> the infoframe code from intel_digital_port. So leave it be for now at
> >>> least.
> >>>
> >>> FIXME need to figure out how to handle the PSR VSC SDP usage before
> >>> doing this, as that might make the state checker unhappy.
> >>>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>    drivers/gpu/drm/i915/intel_ddi.c | 13 ++++++++-----
> >>>    drivers/gpu/drm/i915/intel_dp.c  |  9 +++++++++
> >>>    2 files changed, 17 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >>> index c5611e9d9b9c..6543feeb58f2 100644
> >>> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >>> @@ -2157,7 +2157,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >>>    	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> >>>    	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> >>>    	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> >>> -	struct intel_digital_port *intel_dig_port;
> >>>    	u32 temp, flags = 0;
> >>>
> >>>    	/* XXX: DSI transcoder paranoia */
> >>> @@ -2193,13 +2192,17 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >>>    		break;
> >>>    	}
> >>>
> >>> -	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
> >>> -	case TRANS_DDI_MODE_SELECT_HDMI:
> >>> -		pipe_config->has_hdmi_sink = true;
> >>> -		intel_dig_port = enc_to_dig_port(&encoder->base);
> >>> +	if (encoder->type != INTEL_OUTPUT_ANALOG) {
> >> This will be true for INTEL_OUTPUT_UNUSED, UNKNOWN, eDP, and bunch of
> >> more encoder types. Should we add a bit mask for valid encoder types here ?
> >
> > We only need to care about FDI vs. not. If it's not FDI, then it'll be
> > a DDI encoder which has struct intel_digital_port as its base.
> But again, what if I got a INTEL_OUTPUT_UNKNOWN here, should we even 
> bother to call a infoframe_enabled() for that ? Wont it be better if we 
> can just allow (DP | HDMI | eDP etc) ?

UNKNOWN is the same thing as DP/eDP/HDMI.

> >
> >>> +		struct intel_digital_port *intel_dig_port =
> >>> +			enc_to_dig_port(&encoder->base);
> >>>
> >>>    		if (intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
> >>>    			pipe_config->has_infoframe = true;
> >>> +	}
> >>> +
> >>> +	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
> >>> +	case TRANS_DDI_MODE_SELECT_HDMI:
> >>> +		pipe_config->has_hdmi_sink = true;
> >>>    		/* fall through */
> >>>    	case TRANS_DDI_MODE_SELECT_DVI:
> >>>    		pipe_config->lane_count = 4;
> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >>> index a2d0ee363307..b43009ed1dab 100644
> >>> --- a/drivers/gpu/drm/i915/intel_dp.c
> >>> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >>> @@ -2355,6 +2355,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> >>>    				struct intel_crtc_state *pipe_config)
> >>>    {
> >>>    	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> >>> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >>>    	u32 tmp, flags = 0;
> >>>    	struct drm_device *dev = encoder->base.dev;
> >>>    	struct drm_i915_private *dev_priv = dev->dev_private;
> >>> @@ -2395,6 +2396,10 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> >>>    	    !IS_CHERRYVIEW(dev) && tmp & DP_COLOR_RANGE_16_235)
> >>>    		pipe_config->limited_color_range = true;
> >>>
> >>> +	if (intel_dig_port->infoframe_enabled &&
> >>> +	    intel_dig_port->infoframe_enabled(&encoder->base, pipe_config))
> >>> +		pipe_config->has_infoframe = true;
> >>> +
> >>>    	pipe_config->has_dp_encoder = true;
> >>>
> >>>    	pipe_config->lane_count =
> >>> @@ -3343,6 +3348,10 @@ intel_dp_link_down(struct intel_dp *intel_dp)
> >>>    		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> >>>    	}
> >>>
> >>> +	if (intel_dig_port->set_infoframes)
> >>> +		intel_dig_port->set_infoframes(&intel_dig_port->base.base,
> >>> +					       NULL, false);
> >>> +
> >>>    	msleep(intel_dp->panel_power_down_delay);
> >>>
> >>>    	intel_dp->DP = DP;
> >>>
> >

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: warning for drm/i915: Make infoframe code available to (e)DP ports

[Patchwork]

== Series Details ==

Series: drm/i915: Make infoframe code available to (e)DP ports
URL   : https://patchwork.freedesktop.org/series/8183/
State : warning

== Summary ==

Series 8183v1 drm/i915: Make infoframe code available to (e)DP ports
http://patchwork.freedesktop.org/api/1.0/series/8183/revisions/1/mbox

Test drv_module_reload_basic:
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-uc-pro-default:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup basic-uc-ro-default:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup basic-wb-pro-default:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-wb-ro-default:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test gem_mmap_gtt:
        Subgroup basic-write-cpu-read-gtt:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-write-gtt:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test gem_pwrite:
        Subgroup basic:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_storedw_loop:
        Subgroup basic-bsd:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-vebox:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test kms_addfb_basic:
        Subgroup bad-pitch-0:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup bad-pitch-63:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup bo-too-small:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup small-bo:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup tile-pitch-mismatch:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup hang-read-crc-pipe-b:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup hang-read-crc-pipe-c:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup nonblocking-crc-pipe-a:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup nonblocking-crc-pipe-b:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup nonblocking-crc-pipe-c:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup read-crc-pipe-a:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup read-crc-pipe-c:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup read-crc-pipe-c-frame-sequence:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup basic-rte:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)

fi-hsw-i7-4770k  total:209  pass:187  dwarn:0   dfail:0   fail:3   skip:19 
fi-snb-i7-2600   total:209  pass:167  dwarn:0   dfail:0   fail:3   skip:39 
ro-bdw-i5-5250u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8  
ro-bdw-i7-5600u  total:102  pass:74   dwarn:1   dfail:0   fail:0   skip:26 
ro-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:102  pass:82   dwarn:0   dfail:0   fail:0   skip:19 
WARNING: Long output truncated
ro-bdw-i7-5557U failed to connect after reboot
ro-ivb-i7-3770 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1087/

cbc3c4a drm-intel-nightly: 2016y-06m-02d-22h-03m-59s UTC integration manifest
74ca187 drm/i915: Allow DP ports to set/readout infoframe state (WIP)
4b8ed9e drm/i915: Remove mostly duplicated video DIP handling from PSR code
2385312 drm/i915: Init infoframe vfuncs for DP encoders as well
26f8d36 drm/i915: Move infoframe vfuncs into intel_digital_port
7a68977 drm/i915: Disable infoframes when shutting down DDI HDMI
871b088 drm/i915: Check has_infoframes when enabling infoframes
a0c3313 drm/dp: Add defines for DP SDP types

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx