diff mbox

[1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT

Message ID 1430379455-21244-1-git-send-email-vandana.kannan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

vandana.kannan@intel.com April 30, 2015, 7:37 a.m. UTC 
Changes based on future platform readiness patches related to
HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
---
 drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
 drivers/gpu/drm/i915/intel_dp.c     | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Jani Nikula April 30, 2015, 8:50 a.m. UTC | #1 
On Thu, 30 Apr 2015, Vandana Kannan <vandana.kannan@intel.com> wrote:
> Changes based on future platform readiness patches related to
> HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
>  drivers/gpu/drm/i915/intel_dp.c     | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index cf67f82..e91d637 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -44,7 +44,7 @@ static void i915_save_display(struct drm_device *dev)
>  		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
>  
>  	/* Panel power sequencer */
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
>  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
>  		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
> @@ -79,7 +79,7 @@ static void i915_restore_display(struct drm_device *dev)
>  		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
>  
>  	/* Panel power sequencer */
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
>  		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
>  		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);

I don't think you should touch i915_suspend.c at all. We're trying to
get rid of this blind register save/restore, and make everything work in
the encoder/connector code. Do note that we already skip this for
vlv/chv power sequencer registers.

> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 937ba31..68e10c1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -559,7 +559,7 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
>  {
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  
> -	if (HAS_PCH_SPLIT(dev))
> +	if (!HAS_GMCH_DISPLAY(dev))
>  		return PCH_PP_CONTROL;
>  	else
>  		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
> @@ -569,7 +569,7 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>  {
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  
> -	if (HAS_PCH_SPLIT(dev))
> +	if (!HAS_GMCH_DISPLAY(dev))
>  		return PCH_PP_STATUS;
>  	else
>  		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> @@ -4963,7 +4963,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>  	if (final->t11_t12 != 0)
>  		return;
>  
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		pp_ctrl_reg = PCH_PP_CONTROL;
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
> @@ -5063,7 +5063,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
>  		pp_div_reg = PCH_PP_DIVISOR;

For the rest, I'd like you to do what I suggested in my reply to patch
3, i.e. add a new if (IS_BROXTON(dev)) at the top of each of these, with
BXT_PP_WHATEVER(0) as the register. Later on, we can add code to choose
the power sequencer, a bit similar to what vlv/chv do now (except there
it's based on pipe).

BR,
Jani.


> -- 
> 2.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter May 6, 2015, 10:08 a.m. UTC | #2 
On Thu, Apr 30, 2015 at 01:07:33PM +0530, Vandana Kannan wrote:
> Changes based on future platform readiness patches related to
> HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>

A least for edp we really need to get rid of the save/restore code in
i915_suspend.c and instead have it all tied together with the other pp
code in intel_dp.c. This series seems like a good opportunity to do just
that.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
>  drivers/gpu/drm/i915/intel_dp.c     | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index cf67f82..e91d637 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -44,7 +44,7 @@ static void i915_save_display(struct drm_device *dev)
>  		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
>  
>  	/* Panel power sequencer */
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
>  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
>  		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
> @@ -79,7 +79,7 @@ static void i915_restore_display(struct drm_device *dev)
>  		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
>  
>  	/* Panel power sequencer */
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
>  		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
>  		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 937ba31..68e10c1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -559,7 +559,7 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
>  {
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  
> -	if (HAS_PCH_SPLIT(dev))
> +	if (!HAS_GMCH_DISPLAY(dev))
>  		return PCH_PP_CONTROL;
>  	else
>  		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
> @@ -569,7 +569,7 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>  {
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  
> -	if (HAS_PCH_SPLIT(dev))
> +	if (!HAS_GMCH_DISPLAY(dev))
>  		return PCH_PP_STATUS;
>  	else
>  		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> @@ -4963,7 +4963,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>  	if (final->t11_t12 != 0)
>  		return;
>  
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		pp_ctrl_reg = PCH_PP_CONTROL;
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
> @@ -5063,7 +5063,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
>  		pp_div_reg = PCH_PP_DIVISOR;
> -- 
> 2.0.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula May 6, 2015, 10:12 a.m. UTC | #3 
On Wed, 06 May 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Apr 30, 2015 at 01:07:33PM +0530, Vandana Kannan wrote:
>> Changes based on future platform readiness patches related to
>> HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT
>> 
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
>
> A least for edp we really need to get rid of the save/restore code in
> i915_suspend.c and instead have it all tied together with the other pp
> code in intel_dp.c. This series seems like a good opportunity to do just
> that.

The next version of the patch already drops the blind save/restore in
i915_suspend.c and looks much better.

BR,
Jani.

> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
>>  drivers/gpu/drm/i915/intel_dp.c     | 8 ++++----
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
>> index cf67f82..e91d637 100644
>> --- a/drivers/gpu/drm/i915/i915_suspend.c
>> +++ b/drivers/gpu/drm/i915/i915_suspend.c
>> @@ -44,7 +44,7 @@ static void i915_save_display(struct drm_device *dev)
>>  		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
>>  
>>  	/* Panel power sequencer */
>> -	if (HAS_PCH_SPLIT(dev)) {
>> +	if (!HAS_GMCH_DISPLAY(dev)) {
>>  		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
>>  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
>>  		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
>> @@ -79,7 +79,7 @@ static void i915_restore_display(struct drm_device *dev)
>>  		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
>>  
>>  	/* Panel power sequencer */
>> -	if (HAS_PCH_SPLIT(dev)) {
>> +	if (!HAS_GMCH_DISPLAY(dev)) {
>>  		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
>>  		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
>>  		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 937ba31..68e10c1 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -559,7 +559,7 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
>>  {
>>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>  
>> -	if (HAS_PCH_SPLIT(dev))
>> +	if (!HAS_GMCH_DISPLAY(dev))
>>  		return PCH_PP_CONTROL;
>>  	else
>>  		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
>> @@ -569,7 +569,7 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>>  {
>>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>  
>> -	if (HAS_PCH_SPLIT(dev))
>> +	if (!HAS_GMCH_DISPLAY(dev))
>>  		return PCH_PP_STATUS;
>>  	else
>>  		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
>> @@ -4963,7 +4963,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>>  	if (final->t11_t12 != 0)
>>  		return;
>>  
>> -	if (HAS_PCH_SPLIT(dev)) {
>> +	if (!HAS_GMCH_DISPLAY(dev)) {
>>  		pp_ctrl_reg = PCH_PP_CONTROL;
>>  		pp_on_reg = PCH_PP_ON_DELAYS;
>>  		pp_off_reg = PCH_PP_OFF_DELAYS;
>> @@ -5063,7 +5063,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>>  
>>  	lockdep_assert_held(&dev_priv->pps_mutex);
>>  
>> -	if (HAS_PCH_SPLIT(dev)) {
>> +	if (!HAS_GMCH_DISPLAY(dev)) {
>>  		pp_on_reg = PCH_PP_ON_DELAYS;
>>  		pp_off_reg = PCH_PP_OFF_DELAYS;
>>  		pp_div_reg = PCH_PP_DIVISOR;
>> -- 
>> 2.0.1
>> 
>> _______________________________________________
>> 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
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index cf67f82..e91d637 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -44,7 +44,7 @@  static void i915_save_display(struct drm_device *dev)
 		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
 
 	/* Panel power sequencer */
-	if (HAS_PCH_SPLIT(dev)) {
+	if (!HAS_GMCH_DISPLAY(dev)) {
 		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
 		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
 		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
@@ -79,7 +79,7 @@  static void i915_restore_display(struct drm_device *dev)
 		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
 
 	/* Panel power sequencer */
-	if (HAS_PCH_SPLIT(dev)) {
+	if (!HAS_GMCH_DISPLAY(dev)) {
 		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
 		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
 		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 937ba31..68e10c1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -559,7 +559,7 @@  static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-	if (HAS_PCH_SPLIT(dev))
+	if (!HAS_GMCH_DISPLAY(dev))
 		return PCH_PP_CONTROL;
 	else
 		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
@@ -569,7 +569,7 @@  static u32 _pp_stat_reg(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-	if (HAS_PCH_SPLIT(dev))
+	if (!HAS_GMCH_DISPLAY(dev))
 		return PCH_PP_STATUS;
 	else
 		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
@@ -4963,7 +4963,7 @@  intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 	if (final->t11_t12 != 0)
 		return;
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (!HAS_GMCH_DISPLAY(dev)) {
 		pp_ctrl_reg = PCH_PP_CONTROL;
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
@@ -5063,7 +5063,7 @@  intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (!HAS_GMCH_DISPLAY(dev)) {
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
 		pp_div_reg = PCH_PP_DIVISOR;