[PATCH 1/2] drm/i915: Add psr_enabled on pipe_config

[PATCH 1/2] drm/i915: Add psr_enabled on pipe_config

[Rodrigo Vivi]

Let's pre-compute it on pipe_config compute to let it exported there but also
to use to see if we can enable DRRS.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_psr.c     | 19 +++++--------------
 3 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 35cdb48..91dd7bf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5750,6 +5750,38 @@ retry:
 	return setup_ok ? 0 : -EINVAL;
 }
 
+static void intel_compute_psr_config(struct intel_crtc *crtc,
+				     struct intel_crtc_state *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_encoder *intel_encoder;
+	struct intel_digital_port *dig_port;
+
+	pipe_config->psr_enabled = false;
+
+	if (!HAS_PSR(dev)) {
+		DRM_DEBUG_KMS("PSR not supported on this platform\n");
+		return;
+	}
+
+	for_each_encoder_on_crtc(dev, &crtc->base, intel_encoder)
+		if (intel_encoder->type == INTEL_OUTPUT_EDP)
+			break;
+
+	if (intel_encoder->type != INTEL_OUTPUT_EDP)
+		return;
+
+	if (!dev_priv->psr.sink_support) {
+		DRM_DEBUG_KMS("PSR not supported by this panel\n");
+		return;
+	}
+
+	dig_port = enc_to_dig_port(&intel_encoder->base);
+
+	pipe_config->psr_enabled = intel_psr_match_conditions(dig_port);
+}
+
 static void hsw_compute_ips_config(struct intel_crtc *crtc,
 				   struct intel_crtc_state *pipe_config)
 {
@@ -5812,6 +5844,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		pipe_config->pipe_bpp = 8*3;
 	}
 
+	intel_compute_psr_config(crtc, pipe_config);
+
 	if (HAS_IPS(dev))
 		hsw_compute_ips_config(crtc, pipe_config);
 
@@ -10367,6 +10401,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 		      pipe_config->pch_pfit.pos,
 		      pipe_config->pch_pfit.size,
 		      pipe_config->pch_pfit.enabled ? "enabled" : "disabled");
+	DRM_DEBUG_KMS("psr: %i\n", pipe_config->psr_enabled);
 	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
 	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c0fa1cb..d79852f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -384,6 +384,7 @@ struct intel_crtc_state {
 	int fdi_lanes;
 	struct intel_link_m_n fdi_m_n;
 
+	bool psr_enabled;
 	bool ips_enabled;
 
 	bool double_wide;
@@ -1206,6 +1207,7 @@ void intel_backlight_unregister(struct drm_device *dev);
 
 
 /* intel_psr.c */
+bool intel_psr_match_conditions(struct intel_digital_port *dig_port);
 void intel_psr_enable(struct intel_dp *intel_dp);
 void intel_psr_disable(struct intel_dp *intel_dp);
 void intel_psr_invalidate(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index c1ca923..34854af 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -249,15 +249,13 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 		   EDP_PSR_ENABLE);
 }
 
-static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
+bool intel_psr_match_conditions(struct intel_digital_port *dig_port)
 {
-	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 drm_crtc *crtc = dig_port->base.base.crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	lockdep_assert_held(&dev_priv->psr.lock);
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
 
@@ -296,6 +294,7 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 	return true;
 }
 
+
 static void intel_psr_activate(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -330,16 +329,11 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc =
+		to_intel_crtc(intel_dig_port->base.base.crtc);
 
-	if (!HAS_PSR(dev)) {
-		DRM_DEBUG_KMS("PSR not supported on this platform\n");
-		return;
-	}
-
-	if (!is_edp_psr(intel_dp)) {
-		DRM_DEBUG_KMS("PSR not supported by this panel\n");
+	if (!intel_crtc->config->psr_enabled)
 		return;
-	}
 
 	mutex_lock(&dev_priv->psr.lock);
 	if (dev_priv->psr.enabled) {
@@ -347,9 +341,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		goto unlock;
 	}
 
-	if (!intel_psr_match_conditions(intel_dp))
-		goto unlock;
-
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
 	if (HAS_DDI(dev)) {
-- 
2.1.0

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

[PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Rodrigo Vivi]

With PSR enabled being pre computed on pipe_config we can now
prevent DRRS to be enabled along with PSR.

Cc: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 637dd53..af28833 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4992,11 +4992,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 		return;
 	}
 
-	/*
-	 * FIXME: This needs proper synchronization with psr state for some
-	 * platforms that cannot have PSR and DRRS enabled at the same time.
-	 */
-
 	dig_port = dp_to_dig_port(intel_dp);
 	encoder = &dig_port->base;
 	intel_crtc = encoder->new_crtc;
@@ -5082,6 +5077,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
 		return;
 	}
 
+	if (intel_crtc->config->psr_enabled) {
+		DRM_DEBUG_KMS("DRRS: PSR enabled on this crtc\n");
+		return;
+	}
+
 	mutex_lock(&dev_priv->drrs.mutex);
 	if (WARN_ON(dev_priv->drrs.dp)) {
 		DRM_ERROR("DRRS already enabled\n");
-- 
2.1.0

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

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6043
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              269/269              268/269
ILK                                  303/303              303/303
SNB                                  304/304              304/304
IVB                                  338/338              338/338
BYT                                  287/287              287/287
HSW                                  362/362              362/362
BDW                                  310/310              310/310
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt@gem_userptr_blits@coherency-sync      CRASH(1)PASS(3)      CRASH(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Add psr_enabled on pipe_config

[Sivakumar Thulasimani]

> _crtc_state *pipe_config)
>   {
> @@ -5812,6 +5844,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>   		pipe_config->pipe_bpp = 8*3;
>   	}
>   
> +	intel_compute_psr_config(crtc, pipe_config);
> +
>   	if (HAS_IPS(dev))
>   		hsw_compute_ips_config(crtc, pipe_config);
>   
is it required to update psr_enabled during every call to 
intel_crtc_compute_config ? all conditions being checked will never 
change post init so it will be better to do this as part of eDP init 
rather than here.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Ramalingam C]

On Wednesday 25 March 2015 12:42 AM, Rodrigo Vivi wrote:
> With PSR enabled being pre computed on pipe_config we can now
> prevent DRRS to be enabled along with PSR.
>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 637dd53..af28833 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4992,11 +4992,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
>   		return;
>   	}
>   
> -	/*
> -	 * FIXME: This needs proper synchronization with psr state for some
> -	 * platforms that cannot have PSR and DRRS enabled at the same time.
> -	 */
> -
>   	dig_port = dp_to_dig_port(intel_dp);
>   	encoder = &dig_port->base;
>   	intel_crtc = encoder->new_crtc;
> @@ -5082,6 +5077,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
>   		return;
>   	}
>   
> +	if (intel_crtc->config->psr_enabled) {
> +		DRM_DEBUG_KMS("DRRS: PSR enabled on this crtc\n");
> +		return;
> +	}
> +
But you might want to explicitly disable the DRRS when PSR is supported. 
Because incase DRRS is enabled before PSR,
then blocking the enable call wont help us to stop DRRS.
In fact I would have suggested to place this in DRRS init path.  But a 
feature called media playback DRRS(in RFC stage)
needs DRRS active even when PSR is enabled/supported. So dont want to 
block the DRRS init path because PSR is supported.
Disabling alone will be sufficient.

So as soon as PSR support is detected you would like to explicitly 
disable the DRRS(if it is supported)
along with blocking further drrs_enable calls.
>   	mutex_lock(&dev_priv->drrs.mutex);
>   	if (WARN_ON(dev_priv->drrs.dp)) {
>   		DRM_ERROR("DRRS already enabled\n");

-- 
Ram

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

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Vivi, Rodrigo]

On Wed, 2015-03-25 at 16:08 +0530, Ramalingam C wrote:
> On Wednesday 25 March 2015 12:42 AM, Rodrigo Vivi wrote:
> > With PSR enabled being pre computed on pipe_config we can now
> > prevent DRRS to be enabled along with PSR.
> >
> > Cc: Ramalingam C <ramalingam.c@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
> >   1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 637dd53..af28833 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4992,11 +4992,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
> >   		return;
> >   	}
> >   
> > -	/*
> > -	 * FIXME: This needs proper synchronization with psr state for some
> > -	 * platforms that cannot have PSR and DRRS enabled at the same time.
> > -	 */
> > -
> >   	dig_port = dp_to_dig_port(intel_dp);
> >   	encoder = &dig_port->base;
> >   	intel_crtc = encoder->new_crtc;
> > @@ -5082,6 +5077,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
> >   		return;
> >   	}
> >   
> > +	if (intel_crtc->config->psr_enabled) {
> > +		DRM_DEBUG_KMS("DRRS: PSR enabled on this crtc\n");
> > +		return;
> > +	}
> > +
> But you might want to explicitly disable the DRRS when PSR is supported. 
> Because incase DRRS is enabled before PSR,
> then blocking the enable call wont help us to stop DRRS.
> In fact I would have suggested to place this in DRRS init path.  But a 
> feature called media playback DRRS(in RFC stage)
> needs DRRS active even when PSR is enabled/supported. So dont want to 
> block the DRRS init path because PSR is supported.
> Disabling alone will be sufficient.
> 
> So as soon as PSR support is detected you would like to explicitly 
> disable the DRRS(if it is supported)
> along with blocking further drrs_enable calls.

Not actually. Please take a look on the other patch on this series.
If possible please review both.

intel_crtc->config->psr_enabled is pre-computed and comes before DRRS
enabling. So with this pre-computed psr enable flag we don't need to
switch other states because DRRS won't be enabled never.

> >   	mutex_lock(&dev_priv->drrs.mutex);
> >   	if (WARN_ON(dev_priv->drrs.dp)) {
> >   		DRM_ERROR("DRRS already enabled\n");
> 

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

Re: [PATCH 1/2] drm/i915: Add psr_enabled on pipe_config

[Rodrigo Vivi]

On Wed, Mar 25, 2015 at 12:04 AM, Sivakumar Thulasimani
<sivakumar.thulasimani@intel.com> wrote:
>
>> _crtc_state *pipe_config)
>>   {
>> @@ -5812,6 +5844,8 @@ static int intel_crtc_compute_config(struct
>> intel_crtc *crtc,
>>                 pipe_config->pipe_bpp = 8*3;
>>         }
>>   +     intel_compute_psr_config(crtc, pipe_config);
>> +
>>         if (HAS_IPS(dev))
>>                 hsw_compute_ips_config(crtc, pipe_config);
>>
>
> is it required to update psr_enabled during every call to
> intel_crtc_compute_config ? all conditions being checked will never change
> post init so it will be better to do this as part of eDP init rather than
> here.

Actually some of them depend on adjusted mode and other might depend
on s3d but also it is here to make sure all interdependencies and
pre-compute will be together. Besides, afaik this compute func
shouldn't be called many times.

Daniel, any thoughts?


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



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Ramalingam C]

On Wednesday 25 March 2015 09:11 PM, Vivi, Rodrigo wrote:
> On Wed, 2015-03-25 at 16:08 +0530, Ramalingam C wrote:
>> On Wednesday 25 March 2015 12:42 AM, Rodrigo Vivi wrote:
>>> With PSR enabled being pre computed on pipe_config we can now
>>> prevent DRRS to be enabled along with PSR.
>>>
>>> Cc: Ramalingam C <ramalingam.c@intel.com>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
>>>    1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 637dd53..af28833 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -4992,11 +4992,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
>>>    		return;
>>>    	}
>>>    
>>> -	/*
>>> -	 * FIXME: This needs proper synchronization with psr state for some
>>> -	 * platforms that cannot have PSR and DRRS enabled at the same time.
>>> -	 */
>>> -
>>>    	dig_port = dp_to_dig_port(intel_dp);
>>>    	encoder = &dig_port->base;
>>>    	intel_crtc = encoder->new_crtc;
>>> @@ -5082,6 +5077,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
>>>    		return;
>>>    	}
>>>    
>>> +	if (intel_crtc->config->psr_enabled) {
>>> +		DRM_DEBUG_KMS("DRRS: PSR enabled on this crtc\n");
>>> +		return;
>>> +	}
>>> +
>> But you might want to explicitly disable the DRRS when PSR is supported.
>> Because incase DRRS is enabled before PSR,
>> then blocking the enable call wont help us to stop DRRS.
>> In fact I would have suggested to place this in DRRS init path.  But a
>> feature called media playback DRRS(in RFC stage)
>> needs DRRS active even when PSR is enabled/supported. So dont want to
>> block the DRRS init path because PSR is supported.
>> Disabling alone will be sufficient.
>>
>> So as soon as PSR support is detected you would like to explicitly
>> disable the DRRS(if it is supported)
>> along with blocking further drrs_enable calls.
> Not actually. Please take a look on the other patch on this series.
> If possible please review both.
>
> intel_crtc->config->psr_enabled is pre-computed and comes before DRRS
> enabling. So with this pre-computed psr enable flag we don't need to
> switch other states because DRRS won't be enabled never.

Thanks for the explanation. Gone through the previous patch in this 
series. As this flag psr_enabled is
getting computed well before the PSR and DRRS enable, then this patch 
will be sufficient to
disable the DRRS.

>
>>>    	mutex_lock(&dev_priv->drrs.mutex);
>>>    	if (WARN_ON(dev_priv->drrs.dp)) {
>>>    		DRM_ERROR("DRRS already enabled\n");

Thanks,
--Ram

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

Re: [PATCH 1/2] drm/i915: Add psr_enabled on pipe_config

[Ramalingam C]

On Wednesday 25 March 2015 12:42 AM, Rodrigo Vivi wrote:
> Let's pre-compute it on pipe_config compute to let it exported there but also
> to use to see if we can enable DRRS.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>   drivers/gpu/drm/i915/intel_psr.c     | 19 +++++--------------
>   3 files changed, 42 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 35cdb48..91dd7bf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5750,6 +5750,38 @@ retry:
>   	return setup_ok ? 0 : -EINVAL;
>   }
>   
> +static void intel_compute_psr_config(struct intel_crtc *crtc,
> +				     struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_encoder *intel_encoder;
> +	struct intel_digital_port *dig_port;
> +
> +	pipe_config->psr_enabled = false;
> +
> +	if (!HAS_PSR(dev)) {
> +		DRM_DEBUG_KMS("PSR not supported on this platform\n");
> +		return;
> +	}
> +
> +	for_each_encoder_on_crtc(dev, &crtc->base, intel_encoder)
> +		if (intel_encoder->type == INTEL_OUTPUT_EDP)
> +			break;
> +
> +	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> +		return;
> +
> +	if (!dev_priv->psr.sink_support) {
> +		DRM_DEBUG_KMS("PSR not supported by this panel\n");
> +		return;
> +	}
> +
> +	dig_port = enc_to_dig_port(&intel_encoder->base);
> +
> +	pipe_config->psr_enabled = intel_psr_match_conditions(dig_port);
> +}
> +
>   static void hsw_compute_ips_config(struct intel_crtc *crtc,
>   				   struct intel_crtc_state *pipe_config)
>   {
> @@ -5812,6 +5844,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>   		pipe_config->pipe_bpp = 8*3;
>   	}
>   
> +	intel_compute_psr_config(crtc, pipe_config);
> +
>   	if (HAS_IPS(dev))
>   		hsw_compute_ips_config(crtc, pipe_config);
>   
> @@ -10367,6 +10401,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>   		      pipe_config->pch_pfit.pos,
>   		      pipe_config->pch_pfit.size,
>   		      pipe_config->pch_pfit.enabled ? "enabled" : "disabled");
> +	DRM_DEBUG_KMS("psr: %i\n", pipe_config->psr_enabled);
>   	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
>   	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
>   }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c0fa1cb..d79852f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -384,6 +384,7 @@ struct intel_crtc_state {
>   	int fdi_lanes;
>   	struct intel_link_m_n fdi_m_n;
>   
> +	bool psr_enabled;
flag is indicating the status of the PSR capability of PSR source and 
sink, not the status of PSR enabling.
Naming is misleading?
>   	bool ips_enabled;
>   
>   	bool double_wide;
> @@ -1206,6 +1207,7 @@ void intel_backlight_unregister(struct drm_device *dev);
>   
>   
>   /* intel_psr.c */
> +bool intel_psr_match_conditions(struct intel_digital_port *dig_port);
>   void intel_psr_enable(struct intel_dp *intel_dp);
>   void intel_psr_disable(struct intel_dp *intel_dp);
>   void intel_psr_invalidate(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index c1ca923..34854af 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -249,15 +249,13 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>   		   EDP_PSR_ENABLE);
>   }
>   
> -static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
> +bool intel_psr_match_conditions(struct intel_digital_port *dig_port)
>   {
> -	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 drm_crtc *crtc = dig_port->base.base.crtc;
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   
> -	lockdep_assert_held(&dev_priv->psr.lock);
>   	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>   	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
>   
> @@ -296,6 +294,7 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
>   	return true;
>   }
>   
> +
>   static void intel_psr_activate(struct intel_dp *intel_dp)
>   {
>   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> @@ -330,16 +329,11 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   	struct drm_device *dev = intel_dig_port->base.base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc =
> +		to_intel_crtc(intel_dig_port->base.base.crtc);
>   
> -	if (!HAS_PSR(dev)) {
> -		DRM_DEBUG_KMS("PSR not supported on this platform\n");
> -		return;
> -	}
> -
> -	if (!is_edp_psr(intel_dp)) {
> -		DRM_DEBUG_KMS("PSR not supported by this panel\n");
> +	if (!intel_crtc->config->psr_enabled)
>   		return;
> -	}
>   
>   	mutex_lock(&dev_priv->psr.lock);
>   	if (dev_priv->psr.enabled) {
> @@ -347,9 +341,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   		goto unlock;
>   	}
>   
> -	if (!intel_psr_match_conditions(intel_dp))
> -		goto unlock;
> -
>   	dev_priv->psr.busy_frontbuffer_bits = 0;
>   
>   	if (HAS_DDI(dev)) {

-- 
Ram

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

[PATCH 1/2] drm/i915: Add psr_ready on pipe_config

[Rodrigo Vivi]

Let's know beforehand if PSR is ready and will be enabled so we can
prevent DRRS to get enabled.

v2: Removing is_edp_psr func that is not used after this patch.
    Rename match_conditions and document it since it is now external.
    Moving to a propper place as pointed out by Sivakumar.
    Use a better name as pointed out by Ram.

Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Cc: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  1 +
 drivers/gpu/drm/i915/intel_dp.c      |  2 ++
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_psr.c     | 50 +++++++++++++++++++++---------------
 4 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 35cdb48..9112ad9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10367,6 +10367,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 		      pipe_config->pch_pfit.pos,
 		      pipe_config->pch_pfit.size,
 		      pipe_config->pch_pfit.enabled ? "enabled" : "disabled");
+	DRM_DEBUG_KMS("psr ready: %i\n", pipe_config->psr_ready);
 	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
 	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
 }
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 637dd53..e6b1c42 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1388,6 +1388,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		 */
 		min_lane_count = max_lane_count;
 		min_clock = max_clock;
+
+		pipe_config->psr_ready = intel_psr_ready(intel_dp);
 	}
 
 	for (; bpp >= 6*3; bpp -= 2*3) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c0fa1cb..250d70c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -384,6 +384,7 @@ struct intel_crtc_state {
 	int fdi_lanes;
 	struct intel_link_m_n fdi_m_n;
 
+	bool psr_ready;
 	bool ips_enabled;
 
 	bool double_wide;
@@ -1206,6 +1207,7 @@ void intel_backlight_unregister(struct drm_device *dev);
 
 
 /* intel_psr.c */
+bool intel_psr_ready(struct intel_dp *intel_dp);
 void intel_psr_enable(struct intel_dp *intel_dp);
 void intel_psr_disable(struct intel_dp *intel_dp);
 void intel_psr_invalidate(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index c1ca923..2f22ab6 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,11 +56,6 @@
 #include "intel_drv.h"
 #include "i915_drv.h"
 
-static bool is_edp_psr(struct intel_dp *intel_dp)
-{
-	return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
-}
-
 static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -249,7 +244,19 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 		   EDP_PSR_ENABLE);
 }
 
-static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
+/**
+ * intel_psr_ready - PSR ready
+ * @intel_dp: Intel DP
+ *
+ * This function Checks if PSR is supported by Hardware/Source and
+ * Panel/Sink and if all conditions to be enabled are fulfilled.
+ *
+ * It is used to know beforehand if PSR is going to be enabled.
+ *
+ * Returns:
+ * True when PSR is ready to be enabled, false otherwise.
+ */
+bool intel_psr_ready(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = dig_port->base.base.dev;
@@ -257,12 +264,14 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 	struct drm_crtc *crtc = dig_port->base.base.crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	lockdep_assert_held(&dev_priv->psr.lock);
+	if (!HAS_PSR(dev)) {
+		DRM_DEBUG_KMS("PSR not supported on this platform\n");
+		return false;
+	}
+
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
 
-	dev_priv->psr.source_ok = false;
-
 	if (IS_HASWELL(dev) && dig_port->port != PORT_A) {
 		DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
 		return false;
@@ -292,10 +301,19 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 		return false;
 	}
 
+	/* At this point we can tell HW/Source supports PSR */
 	dev_priv->psr.source_ok = true;
+
+	/* Now check if Panel/Sink supports it */
+	if (!dev_priv->psr.sink_support) {
+		DRM_DEBUG_KMS("PSR not supported by this panel\n");
+		return false;
+	}
+
 	return true;
 }
 
+
 static void intel_psr_activate(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -330,16 +348,11 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc =
+		to_intel_crtc(intel_dig_port->base.base.crtc);
 
-	if (!HAS_PSR(dev)) {
-		DRM_DEBUG_KMS("PSR not supported on this platform\n");
-		return;
-	}
-
-	if (!is_edp_psr(intel_dp)) {
-		DRM_DEBUG_KMS("PSR not supported by this panel\n");
+	if (!intel_crtc->config->psr_ready)
 		return;
-	}
 
 	mutex_lock(&dev_priv->psr.lock);
 	if (dev_priv->psr.enabled) {
@@ -347,9 +360,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		goto unlock;
 	}
 
-	if (!intel_psr_match_conditions(intel_dp))
-		goto unlock;
-
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
 	if (HAS_DDI(dev)) {
-- 
2.1.0

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

[PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Rodrigo Vivi]

With PSR enabled being pre computed on pipe_config we can now
prevent DRRS to be enabled along with PSR.

v2: Rebase after changing previous patch

Cc: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e6b1c42..5551b3c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4994,11 +4994,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 		return;
 	}
 
-	/*
-	 * FIXME: This needs proper synchronization with psr state for some
-	 * platforms that cannot have PSR and DRRS enabled at the same time.
-	 */
-
 	dig_port = dp_to_dig_port(intel_dp);
 	encoder = &dig_port->base;
 	intel_crtc = encoder->new_crtc;
@@ -5084,6 +5079,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
 		return;
 	}
 
+	if (intel_crtc->config->psr_ready) {
+		DRM_DEBUG_KMS("DRRS: PSR will be enabled on this crtc\n");
+		return;
+	}
+
 	mutex_lock(&dev_priv->drrs.mutex);
 	if (WARN_ON(dev_priv->drrs.dp)) {
 		DRM_ERROR("DRRS already enabled\n");
-- 
2.1.0

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

Re: [PATCH 1/2] drm/i915: Add psr_ready on pipe_config

[Sivakumar Thulasimani]

On 3/27/2015 12:50 AM, Rodrigo Vivi wrote:
> Let's know beforehand if PSR is ready and will be enabled so we can
> prevent DRRS to get enabled.
>
> v2: Removing is_edp_psr func that is not used after this patch.
>      Rename match_conditions and document it since it is now external.
>      Moving to a propper place as pointed out by Sivakumar.
>      Use a better name as pointed out by Ram.
>
> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c |  1 +
>   drivers/gpu/drm/i915/intel_dp.c      |  2 ++
>   drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>   drivers/gpu/drm/i915/intel_psr.c     | 50 +++++++++++++++++++++---------------
>   4 files changed, 35 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 35cdb48..9112ad9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10367,6 +10367,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>   		      pipe_config->pch_pfit.pos,
>   		      pipe_config->pch_pfit.size,
>   		      pipe_config->pch_pfit.enabled ? "enabled" : "disabled");
> +	DRM_DEBUG_KMS("psr ready: %i\n", pipe_config->psr_ready);
>   	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
>   	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
>   }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 637dd53..e6b1c42 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1388,6 +1388,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>   		 */
>   		min_lane_count = max_lane_count;
>   		min_clock = max_clock;
> +
> +		pipe_config->psr_ready = intel_psr_ready(intel_dp);
>   	}
>   
>   	for (; bpp >= 6*3; bpp -= 2*3) {
This is still updated during every modeset. since PSR is specific to 
eDP, can this new variable be stored inside intel_dp and be updated in 
intel_edp_init_connector itself ? that way we will initialize this 
during driver load and can reuse it forever.

regards,
Sivakumar Thulasimani
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Add psr_ready on pipe_config

[Daniel Vetter]

On Fri, Mar 27, 2015 at 11:37:09AM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 3/27/2015 12:50 AM, Rodrigo Vivi wrote:
> >Let's know beforehand if PSR is ready and will be enabled so we can
> >prevent DRRS to get enabled.
> >
> >v2: Removing is_edp_psr func that is not used after this patch.
> >     Rename match_conditions and document it since it is now external.
> >     Moving to a propper place as pointed out by Sivakumar.
> >     Use a better name as pointed out by Ram.
> >
> >Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> >Cc: Ramalingam C <ramalingam.c@intel.com>
> >Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_display.c |  1 +
> >  drivers/gpu/drm/i915/intel_dp.c      |  2 ++
> >  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
> >  drivers/gpu/drm/i915/intel_psr.c     | 50 +++++++++++++++++++++---------------
> >  4 files changed, 35 insertions(+), 20 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >index 35cdb48..9112ad9 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -10367,6 +10367,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
> >  		      pipe_config->pch_pfit.pos,
> >  		      pipe_config->pch_pfit.size,
> >  		      pipe_config->pch_pfit.enabled ? "enabled" : "disabled");
> >+	DRM_DEBUG_KMS("psr ready: %i\n", pipe_config->psr_ready);
> >  	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
> >  	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
> >  }
> >diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >index 637dd53..e6b1c42 100644
> >--- a/drivers/gpu/drm/i915/intel_dp.c
> >+++ b/drivers/gpu/drm/i915/intel_dp.c
> >@@ -1388,6 +1388,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  		 */
> >  		min_lane_count = max_lane_count;
> >  		min_clock = max_clock;
> >+
> >+		pipe_config->psr_ready = intel_psr_ready(intel_dp);
> >  	}
> >  	for (; bpp >= 6*3; bpp -= 2*3) {
> This is still updated during every modeset. since PSR is specific to eDP,
> can this new variable be stored inside intel_dp and be updated in
> intel_edp_init_connector itself ? that way we will initialize this during
> driver load and can reuse it forever.

Imo that's totally fine. I actually prefer to recompute state instead of
storing it, since that cuts out one indirection. I'm totally fine with
this approach here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Add psr_ready on pipe_config

[Sivakumar Thulasimani]

On 3/27/2015 2:02 PM, Daniel Vetter wrote:
> On Fri, Mar 27, 2015 at 11:37:09AM +0530, Sivakumar Thulasimani wrote:
>>
>> On 3/27/2015 12:50 AM, Rodrigo Vivi wrote:
>>> Let's know beforehand if PSR is ready and will be enabled so we can
>>> prevent DRRS to get enabled.
>>>
>>> v2: Removing is_edp_psr func that is not used after this patch.
>>>      Rename match_conditions and document it since it is now external.
>>>      Moving to a propper place as pointed out by Sivakumar.
>>>      Use a better name as pointed out by Ram.
>>>
>>> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>>> Cc: Ramalingam C <ramalingam.c@intel.com>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_display.c |  1 +
>>>   drivers/gpu/drm/i915/intel_dp.c      |  2 ++
>>>   drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>>>   drivers/gpu/drm/i915/intel_psr.c     | 50 +++++++++++++++++++++---------------
>>>   4 files changed, 35 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 35cdb48..9112ad9 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -10367,6 +10367,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>>>   		      pipe_config->pch_pfit.pos,
>>>   		      pipe_config->pch_pfit.size,
>>>   		      pipe_config->pch_pfit.enabled ? "enabled" : "disabled");
>>> +	DRM_DEBUG_KMS("psr ready: %i\n", pipe_config->psr_ready);
>>>   	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
>>>   	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
>>>   }
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 637dd53..e6b1c42 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -1388,6 +1388,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>>   		 */
>>>   		min_lane_count = max_lane_count;
>>>   		min_clock = max_clock;
>>> +
>>> +		pipe_config->psr_ready = intel_psr_ready(intel_dp);
>>>   	}
>>>   	for (; bpp >= 6*3; bpp -= 2*3) {
>> This is still updated during every modeset. since PSR is specific to eDP,
>> can this new variable be stored inside intel_dp and be updated in
>> intel_edp_init_connector itself ? that way we will initialize this during
>> driver load and can reuse it forever.
> Imo that's totally fine. I actually prefer to recompute state instead of
> storing it, since that cuts out one indirection. I'm totally fine with
> this approach here.
> -Daniel
just checked out intel_psr_match_conditions (which is renamed here) and 
it is checking for S3D modes, so yes in that case it is better to have 
this check here. but the same function also has check for interlaced 
mode. i am yet to come across an edp panel with Interlaced mode, that 
can be removed may be sometime later in a different patch.

regards,
Sivakumar Thulasimani
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Ramalingam C]

Looks good to me..

Reviewed-by: Ramalingam C <ramalingam.c@intel.com>

On Friday 27 March 2015 12:51 AM, Rodrigo Vivi wrote:
> With PSR enabled being pre computed on pipe_config we can now
> prevent DRRS to be enabled along with PSR.
>
> v2: Rebase after changing previous patch
>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e6b1c42..5551b3c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4994,11 +4994,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
>   		return;
>   	}
>   
> -	/*
> -	 * FIXME: This needs proper synchronization with psr state for some
> -	 * platforms that cannot have PSR and DRRS enabled at the same time.
> -	 */
> -
>   	dig_port = dp_to_dig_port(intel_dp);
>   	encoder = &dig_port->base;
>   	intel_crtc = encoder->new_crtc;
> @@ -5084,6 +5079,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
>   		return;
>   	}
>   
> +	if (intel_crtc->config->psr_ready) {
> +		DRM_DEBUG_KMS("DRRS: PSR will be enabled on this crtc\n");
> +		return;
> +	}
> +
>   	mutex_lock(&dev_priv->drrs.mutex);
>   	if (WARN_ON(dev_priv->drrs.dp)) {
>   		DRM_ERROR("DRRS already enabled\n");

-- 
Thanks,
--Ram

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

Re: [PATCH 1/2] drm/i915: Add psr_ready on pipe_config

[Ramalingam C]

Looks good to me..

Reviewed-by: Ramalingam C <ramalingam.c@intel.com>

On Friday 27 March 2015 12:50 AM, Rodrigo Vivi wrote:
> Let's know beforehand if PSR is ready and will be enabled so we can
> prevent DRRS to get enabled.
>
> v2: Removing is_edp_psr func that is not used after this patch.
>      Rename match_conditions and document it since it is now external.
>      Moving to a propper place as pointed out by Sivakumar.
>      Use a better name as pointed out by Ram.
>
> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c |  1 +
>   drivers/gpu/drm/i915/intel_dp.c      |  2 ++
>   drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>   drivers/gpu/drm/i915/intel_psr.c     | 50 +++++++++++++++++++++---------------
>   4 files changed, 35 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 35cdb48..9112ad9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10367,6 +10367,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>   		      pipe_config->pch_pfit.pos,
>   		      pipe_config->pch_pfit.size,
>   		      pipe_config->pch_pfit.enabled ? "enabled" : "disabled");
> +	DRM_DEBUG_KMS("psr ready: %i\n", pipe_config->psr_ready);
>   	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
>   	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
>   }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 637dd53..e6b1c42 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1388,6 +1388,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>   		 */
>   		min_lane_count = max_lane_count;
>   		min_clock = max_clock;
> +
> +		pipe_config->psr_ready = intel_psr_ready(intel_dp);
>   	}
>   
>   	for (; bpp >= 6*3; bpp -= 2*3) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c0fa1cb..250d70c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -384,6 +384,7 @@ struct intel_crtc_state {
>   	int fdi_lanes;
>   	struct intel_link_m_n fdi_m_n;
>   
> +	bool psr_ready;
>   	bool ips_enabled;
>   
>   	bool double_wide;
> @@ -1206,6 +1207,7 @@ void intel_backlight_unregister(struct drm_device *dev);
>   
>   
>   /* intel_psr.c */
> +bool intel_psr_ready(struct intel_dp *intel_dp);
>   void intel_psr_enable(struct intel_dp *intel_dp);
>   void intel_psr_disable(struct intel_dp *intel_dp);
>   void intel_psr_invalidate(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index c1ca923..2f22ab6 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -56,11 +56,6 @@
>   #include "intel_drv.h"
>   #include "i915_drv.h"
>   
> -static bool is_edp_psr(struct intel_dp *intel_dp)
> -{
> -	return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
> -}
> -
>   static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -249,7 +244,19 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>   		   EDP_PSR_ENABLE);
>   }
>   
> -static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
> +/**
> + * intel_psr_ready - PSR ready
> + * @intel_dp: Intel DP
> + *
> + * This function Checks if PSR is supported by Hardware/Source and
> + * Panel/Sink and if all conditions to be enabled are fulfilled.
> + *
> + * It is used to know beforehand if PSR is going to be enabled.
> + *
> + * Returns:
> + * True when PSR is ready to be enabled, false otherwise.
> + */
> +bool intel_psr_ready(struct intel_dp *intel_dp)
>   {
>   	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>   	struct drm_device *dev = dig_port->base.base.dev;
> @@ -257,12 +264,14 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
>   	struct drm_crtc *crtc = dig_port->base.base.crtc;
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   
> -	lockdep_assert_held(&dev_priv->psr.lock);
> +	if (!HAS_PSR(dev)) {
> +		DRM_DEBUG_KMS("PSR not supported on this platform\n");
> +		return false;
> +	}
> +
>   	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>   	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
>   
> -	dev_priv->psr.source_ok = false;
> -
>   	if (IS_HASWELL(dev) && dig_port->port != PORT_A) {
>   		DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
>   		return false;
> @@ -292,10 +301,19 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
>   		return false;
>   	}
>   
> +	/* At this point we can tell HW/Source supports PSR */
>   	dev_priv->psr.source_ok = true;
> +
> +	/* Now check if Panel/Sink supports it */
> +	if (!dev_priv->psr.sink_support) {
> +		DRM_DEBUG_KMS("PSR not supported by this panel\n");
> +		return false;
> +	}
> +
>   	return true;
>   }
>   
> +
>   static void intel_psr_activate(struct intel_dp *intel_dp)
>   {
>   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> @@ -330,16 +348,11 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   	struct drm_device *dev = intel_dig_port->base.base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc =
> +		to_intel_crtc(intel_dig_port->base.base.crtc);
>   
> -	if (!HAS_PSR(dev)) {
> -		DRM_DEBUG_KMS("PSR not supported on this platform\n");
> -		return;
> -	}
> -
> -	if (!is_edp_psr(intel_dp)) {
> -		DRM_DEBUG_KMS("PSR not supported by this panel\n");
> +	if (!intel_crtc->config->psr_ready)
>   		return;
> -	}
>   
>   	mutex_lock(&dev_priv->psr.lock);
>   	if (dev_priv->psr.enabled) {
> @@ -347,9 +360,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   		goto unlock;
>   	}
>   
> -	if (!intel_psr_match_conditions(intel_dp))
> -		goto unlock;
> -
>   	dev_priv->psr.busy_frontbuffer_bits = 0;
>   
>   	if (HAS_DDI(dev)) {

-- 
Thanks,
--Ram

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

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Daniel Vetter]

On Mon, Mar 30, 2015 at 03:19:29PM +0530, Ramalingam C wrote:
> Looks good to me..
> 
> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> 
> On Friday 27 March 2015 12:51 AM, Rodrigo Vivi wrote:
> >With PSR enabled being pre computed on pipe_config we can now
> >prevent DRRS to be enabled along with PSR.
> >
> >v2: Rebase after changing previous patch
> >
> >Cc: Ramalingam C <ramalingam.c@intel.com>
> >Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Both patches applied, thanks.
-Daniel

> >---
> >  drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >index e6b1c42..5551b3c 100644
> >--- a/drivers/gpu/drm/i915/intel_dp.c
> >+++ b/drivers/gpu/drm/i915/intel_dp.c
> >@@ -4994,11 +4994,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
> >  		return;
> >  	}
> >-	/*
> >-	 * FIXME: This needs proper synchronization with psr state for some
> >-	 * platforms that cannot have PSR and DRRS enabled at the same time.
> >-	 */
> >-
> >  	dig_port = dp_to_dig_port(intel_dp);
> >  	encoder = &dig_port->base;
> >  	intel_crtc = encoder->new_crtc;
> >@@ -5084,6 +5079,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
> >  		return;
> >  	}
> >+	if (intel_crtc->config->psr_ready) {
> >+		DRM_DEBUG_KMS("DRRS: PSR will be enabled on this crtc\n");
> >+		return;
> >+	}
> >+
> >  	mutex_lock(&dev_priv->drrs.mutex);
> >  	if (WARN_ON(dev_priv->drrs.dp)) {
> >  		DRM_ERROR("DRRS already enabled\n");
> 
> -- 
> Thanks,
> --Ram
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Daniel Vetter]

On Mon, Mar 30, 2015 at 05:39:54PM +0200, Daniel Vetter wrote:
> On Mon, Mar 30, 2015 at 03:19:29PM +0530, Ramalingam C wrote:
> > Looks good to me..
> > 
> > Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> > 
> > On Friday 27 March 2015 12:51 AM, Rodrigo Vivi wrote:
> > >With PSR enabled being pre computed on pipe_config we can now
> > >prevent DRRS to be enabled along with PSR.
> > >
> > >v2: Rebase after changing previous patch
> > >
> > >Cc: Ramalingam C <ramalingam.c@intel.com>
> > >Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Both patches applied, thanks.

Both dropped again since it blows up - in compute_config functions you are
not allowed to look at the current configuration ever. And you do that in
two places with this code:
- deref drm_encoder->crtc, which results in oopses
- deref intel_crtc->config, which is just semantically wrong.

See Chris patch for how to fix this (although it's slightly incomplete
since it doesn't replace all access to intel_crtc->config with the
explicitly passed-in pipe_config pointer).
-Daniel

> -Daniel
> 
> > >---
> > >  drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > >diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > >index e6b1c42..5551b3c 100644
> > >--- a/drivers/gpu/drm/i915/intel_dp.c
> > >+++ b/drivers/gpu/drm/i915/intel_dp.c
> > >@@ -4994,11 +4994,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
> > >  		return;
> > >  	}
> > >-	/*
> > >-	 * FIXME: This needs proper synchronization with psr state for some
> > >-	 * platforms that cannot have PSR and DRRS enabled at the same time.
> > >-	 */
> > >-
> > >  	dig_port = dp_to_dig_port(intel_dp);
> > >  	encoder = &dig_port->base;
> > >  	intel_crtc = encoder->new_crtc;
> > >@@ -5084,6 +5079,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
> > >  		return;
> > >  	}
> > >+	if (intel_crtc->config->psr_ready) {
> > >+		DRM_DEBUG_KMS("DRRS: PSR will be enabled on this crtc\n");
> > >+		return;
> > >+	}
> > >+
> > >  	mutex_lock(&dev_priv->drrs.mutex);
> > >  	if (WARN_ON(dev_priv->drrs.dp)) {
> > >  		DRM_ERROR("DRRS already enabled\n");
> > 
> > -- 
> > Thanks,
> > --Ram
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915: Add psr_ready on pipe_config

[Rodrigo Vivi]

Let's know beforehand if PSR is ready and will be enabled so we can
prevent DRRS to get enabled.

v2: Removing is_edp_psr func that is not used after this patch.
    Rename match_conditions and document it since it is now external.
    Moving to a propper place as pointed out by Sivakumar.
    Use a better name as pointed out by Ram.

v3: Don't dereferrence drm_encoder->crtc and intel_crtc->config on psr_ready check.
    Fix a opps caused with previous versions.

Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Cc: Ramalingam C <ramalingam.c@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: Ramalingam C <ramalingam.c@intel.com> (v2)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  1 +
 drivers/gpu/drm/i915/intel_dp.c      |  2 ++
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++
 drivers/gpu/drm/i915/intel_psr.c     | 57 ++++++++++++++++++++----------------
 4 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 75afa6e..50f2db7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10895,6 +10895,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 		      pipe_config->pch_pfit.pos,
 		      pipe_config->pch_pfit.size,
 		      pipe_config->pch_pfit.enabled ? "enabled" : "disabled");
+	DRM_DEBUG_KMS("psr ready: %i\n", pipe_config->psr_ready);
 	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
 	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 14cdd00..94bbdf4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1394,6 +1394,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		 */
 		min_lane_count = max_lane_count;
 		min_clock = max_clock;
+
+		pipe_config->psr_ready = intel_psr_ready(intel_dp, pipe_config);
 	}
 
 	for (; bpp >= 6*3; bpp -= 2*3) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 082be71..9895772 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -448,6 +448,7 @@ struct intel_crtc_state {
 	int fdi_lanes;
 	struct intel_link_m_n fdi_m_n;
 
+	bool psr_ready;
 	bool ips_enabled;
 
 	bool double_wide;
@@ -1287,6 +1288,8 @@ void intel_backlight_unregister(struct drm_device *dev);
 
 
 /* intel_psr.c */
+bool intel_psr_ready(struct intel_dp *intel_dp,
+		     struct intel_crtc_state *pipe_config);
 void intel_psr_enable(struct intel_dp *intel_dp);
 void intel_psr_disable(struct intel_dp *intel_dp);
 void intel_psr_invalidate(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 5ee0fa5..61d582b 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,11 +56,6 @@
 #include "intel_drv.h"
 #include "i915_drv.h"
 
-static bool is_edp_psr(struct intel_dp *intel_dp)
-{
-	return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
-}
-
 static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -282,19 +277,32 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
 }
 
-static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
+/**
+ * intel_psr_ready - PSR ready
+ * @intel_dp: Intel DP
+ * @pipe_config: Pipe Config
+ *
+ * This function Checks if PSR is supported by Hardware/Source and
+ * Panel/Sink and if all conditions to be enabled are fulfilled.
+ *
+ * It is used to know beforehand if PSR is going to be enabled.
+ *
+ * Returns:
+ * True when PSR is ready to be enabled, false otherwise.
+ */
+bool intel_psr_ready(struct intel_dp *intel_dp,
+		     struct intel_crtc_state *pipe_config)
 {
 	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 drm_crtc *crtc = dig_port->base.base.crtc;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	lockdep_assert_held(&dev_priv->psr.lock);
-	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+	if (!HAS_PSR(dev)) {
+		DRM_DEBUG_KMS("PSR not supported on this platform\n");
+		return false;
+	}
 
-	dev_priv->psr.source_ok = false;
+	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 
 	if (IS_HASWELL(dev) && dig_port->port != PORT_A) {
 		DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
@@ -307,14 +315,14 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 	}
 
 	if (IS_HASWELL(dev) &&
-	    I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config->cpu_transcoder)) &
+	    I915_READ(HSW_STEREO_3D_CTL(pipe_config->cpu_transcoder)) &
 		      S3D_ENABLE) {
 		DRM_DEBUG_KMS("PSR condition failed: Stereo 3D is Enabled\n");
 		return false;
 	}
 
 	if (IS_HASWELL(dev) &&
-	    intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
+	    pipe_config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
 		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
 		return false;
 	}
@@ -325,10 +333,19 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 		return false;
 	}
 
+	/* At this point we can tell HW/Source supports PSR */
 	dev_priv->psr.source_ok = true;
+
+	/* Now check if Panel/Sink supports it */
+	if (!dev_priv->psr.sink_support) {
+		DRM_DEBUG_KMS("PSR not supported by this panel\n");
+		return false;
+	}
+
 	return true;
 }
 
+
 static void intel_psr_activate(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -365,15 +382,8 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
 
-	if (!HAS_PSR(dev)) {
-		DRM_DEBUG_KMS("PSR not supported on this platform\n");
-		return;
-	}
-
-	if (!is_edp_psr(intel_dp)) {
-		DRM_DEBUG_KMS("PSR not supported by this panel\n");
+	if (!crtc->config->psr_ready)
 		return;
-	}
 
 	mutex_lock(&dev_priv->psr.lock);
 	if (dev_priv->psr.enabled) {
@@ -381,9 +391,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		goto unlock;
 	}
 
-	if (!intel_psr_match_conditions(intel_dp))
-		goto unlock;
-
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
 	if (HAS_DDI(dev)) {
-- 
2.1.0

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