[PATCH] drm/i915: fix up the edp power well check

[PATCH] drm/i915: fix up the edp power well check

[Daniel Vetter]

Now that we track the cpu transcoder we need accurately in the pipe
config we can finally fix up the transcoder check. With the current
code eDP on port D will be broken since we'd errornously cut the
power.

For reference see

commit 2124b72e6283c4e84a55e71077fee91793f4c801
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
Date:   Fri Mar 22 14:07:23 2013 -0300

    drm/i915: don't disable the power well yet

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3db4ed2..1ee9e92 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5809,12 +5809,8 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
 		/* Even the eDP panel fitter is outside the always-on well. */
 		if (crtc->config.pch_pfit.size && crtc->base.enabled)
 			enable = true;
-	}
 
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
-			    base.head) {
-		if (encoder->type != INTEL_OUTPUT_EDP &&
-		    encoder->connectors_active)
+		if (crtc->config.cpu_transcoder != TRANSCODER_EDP)
 			enable = true;
 	}
 
-- 
1.7.10.4

Re: [PATCH] drm/i915: fix up the edp power well check

[Daniel Vetter]

On Wed, May 29, 2013 at 07:56:11PM +0200, Daniel Vetter wrote:
> Now that we track the cpu transcoder we need accurately in the pipe
> config we can finally fix up the transcoder check. With the current
> code eDP on port D will be broken since we'd errornously cut the
> power.
> 
> For reference see
> 
> commit 2124b72e6283c4e84a55e71077fee91793f4c801
> Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Date:   Fri Mar 22 14:07:23 2013 -0300
> 
>     drm/i915: don't disable the power well yet
> 
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Adding imre since he smashed his r-b onto the regressing commit.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3db4ed2..1ee9e92 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5809,12 +5809,8 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
>  		/* Even the eDP panel fitter is outside the always-on well. */
>  		if (crtc->config.pch_pfit.size && crtc->base.enabled)
>  			enable = true;
> -	}
>  
> -	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> -			    base.head) {
> -		if (encoder->type != INTEL_OUTPUT_EDP &&
> -		    encoder->connectors_active)
> +		if (crtc->config.cpu_transcoder != TRANSCODER_EDP)
>  			enable = true;
>  	}
>  
> -- 
> 1.7.10.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: fix up the edp power well check

[Daniel Vetter]

On Wed, May 29, 2013 at 09:45:20PM +0200, Daniel Vetter wrote:
> On Wed, May 29, 2013 at 07:56:11PM +0200, Daniel Vetter wrote:
> > Now that we track the cpu transcoder we need accurately in the pipe
> > config we can finally fix up the transcoder check. With the current
> > code eDP on port D will be broken since we'd errornously cut the
> > power.
> > 
> > For reference see
> > 
> > commit 2124b72e6283c4e84a55e71077fee91793f4c801
> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Date:   Fri Mar 22 14:07:23 2013 -0300
> > 
> >     drm/i915: don't disable the power well yet
> > 
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Adding imre since he smashed his r-b onto the regressing commit.

Meh, replied to the wrong patch, never mind.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: fix up the edp power well check

[Takashi Iwai]

At Wed, 29 May 2013 19:56:11 +0200,
Daniel Vetter wrote:
> 
> Now that we track the cpu transcoder we need accurately in the pipe
> config we can finally fix up the transcoder check. With the current
> code eDP on port D will be broken since we'd errornously cut the
> power.
> 
> For reference see
> 
> commit 2124b72e6283c4e84a55e71077fee91793f4c801
> Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Date:   Fri Mar 22 14:07:23 2013 -0300
> 
>     drm/i915: don't disable the power well yet
> 
> Cc: Takashi Iwai <tiwai@suse.de>

Sorry, I have no hardware that showed the problem around here for
testing now, unfortunately.


Takashi

> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3db4ed2..1ee9e92 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5809,12 +5809,8 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
>  		/* Even the eDP panel fitter is outside the always-on well. */
>  		if (crtc->config.pch_pfit.size && crtc->base.enabled)
>  			enable = true;
> -	}
>  
> -	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> -			    base.head) {
> -		if (encoder->type != INTEL_OUTPUT_EDP &&
> -		    encoder->connectors_active)
> +		if (crtc->config.cpu_transcoder != TRANSCODER_EDP)
>  			enable = true;
>  	}
>  
> -- 
> 1.7.10.4
> 

Re: [PATCH] drm/i915: fix up the edp power well check

[Paulo Zanoni]

2013/5/29 Daniel Vetter <daniel.vetter@ffwll.ch>:
> Now that we track the cpu transcoder we need accurately in the pipe
> config we can finally fix up the transcoder check. With the current
> code eDP on port D will be broken since we'd errornously cut the
> power.
>
> For reference see
>
> commit 2124b72e6283c4e84a55e71077fee91793f4c801
> Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Date:   Fri Mar 22 14:07:23 2013 -0300
>
>     drm/i915: don't disable the power well yet
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3db4ed2..1ee9e92 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5809,12 +5809,8 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
>                 /* Even the eDP panel fitter is outside the always-on well. */
>                 if (crtc->config.pch_pfit.size && crtc->base.enabled)
>                         enable = true;
> -       }
>
> -       list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> -                           base.head) {
> -               if (encoder->type != INTEL_OUTPUT_EDP &&
> -                   encoder->connectors_active)
> +               if (crtc->config.cpu_transcoder != TRANSCODER_EDP)

Don't you also need to check for crtc->base.enabled here? If you do
this, please notice that all 3 checks inside the loop will have a
check for crtc->base.enabled inside, so you might want to move that to
a "if (!crtc->base.enabled) continue;" or something similar.

Please also update/remove the comment that has an "XXX" tag.


>                         enable = true;
>         }
>
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH] drm/i915: fix up the edp power well check

[Daniel Vetter]

On Fri, May 31, 2013 at 03:53:05PM +0200, Takashi Iwai wrote:
> At Wed, 29 May 2013 19:56:11 +0200,
> Daniel Vetter wrote:
> > 
> > Now that we track the cpu transcoder we need accurately in the pipe
> > config we can finally fix up the transcoder check. With the current
> > code eDP on port D will be broken since we'd errornously cut the
> > power.
> > 
> > For reference see
> > 
> > commit 2124b72e6283c4e84a55e71077fee91793f4c801
> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Date:   Fri Mar 22 14:07:23 2013 -0300
> > 
> >     drm/i915: don't disable the power well yet
> > 
> > Cc: Takashi Iwai <tiwai@suse.de>
> 
> Sorry, I have no hardware that showed the problem around here for
> testing now, unfortunately.

Np, that was just an fyi cc ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/i915: fix up the edp power well check

[Daniel Vetter]

Now that we track the cpu transcoder we need accurately in the pipe
config we can finally fix up the transcoder check. With the current
code eDP on port D will be broken since we'd errornously cut the
power.

For reference see

commit 2124b72e6283c4e84a55e71077fee91793f4c801
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
Date:   Fri Mar 22 14:07:23 2013 -0300

    drm/i915: don't disable the power well yet

v2:
- Kill the now outdated comment (Paulo)
- Add the missing crtc->base.enabled check and consolidate it (Paulo)
- Smash all checks together, looks neater that way.

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |   17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3db4ed2..7cc953d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5801,20 +5801,11 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
 	struct intel_encoder *encoder;
 
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
-		if (crtc->pipe != PIPE_A && crtc->base.enabled)
-			enable = true;
-		/* XXX: Should check for edp transcoder here, but thanks to init
-		 * sequence that's not yet available. Just in case desktop eDP
-		 * on PORT D is possible on haswell, too. */
-		/* Even the eDP panel fitter is outside the always-on well. */
-		if (crtc->config.pch_pfit.size && crtc->base.enabled)
-			enable = true;
-	}
+		if (!crtc->base.enabled)
+			continue;
 
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
-			    base.head) {
-		if (encoder->type != INTEL_OUTPUT_EDP &&
-		    encoder->connectors_active)
+		if (crtc->pipe != PIPE_A || crtc->config.pch_pfit.size ||
+		    crtc->config.cpu_transcoder != TRANSCODER_EDP)
 			enable = true;
 	}
 
-- 
1.7.10.4

[PATCH] drm/i915: fix up the edp power well check

[Daniel Vetter]

Now that we track the cpu transcoder we need accurately in the pipe
config we can finally fix up the transcoder check. With the current
code eDP on port D will be broken since we'd errornously cut the
power.

For reference see

commit 2124b72e6283c4e84a55e71077fee91793f4c801
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
Date:   Fri Mar 22 14:07:23 2013 -0300

    drm/i915: don't disable the power well yet

v2:
- Kill the now outdated comment (Paulo)
- Add the missing crtc->base.enabled check and consolidate it (Paulo)
- Smash all checks together, looks neater that way.

v3: Kill the unused encoder variable.

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |   18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3db4ed2..c4adbfe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5798,23 +5798,13 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
 {
 	bool enable = false;
 	struct intel_crtc *crtc;
-	struct intel_encoder *encoder;
 
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
-		if (crtc->pipe != PIPE_A && crtc->base.enabled)
-			enable = true;
-		/* XXX: Should check for edp transcoder here, but thanks to init
-		 * sequence that's not yet available. Just in case desktop eDP
-		 * on PORT D is possible on haswell, too. */
-		/* Even the eDP panel fitter is outside the always-on well. */
-		if (crtc->config.pch_pfit.size && crtc->base.enabled)
-			enable = true;
-	}
+		if (!crtc->base.enabled)
+			continue;
 
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
-			    base.head) {
-		if (encoder->type != INTEL_OUTPUT_EDP &&
-		    encoder->connectors_active)
+		if (crtc->pipe != PIPE_A || crtc->config.pch_pfit.size ||
+		    crtc->config.cpu_transcoder != TRANSCODER_EDP)
 			enable = true;
 	}
 
-- 
1.7.10.4

Re: [PATCH] drm/i915: fix up the edp power well check

[Paulo Zanoni]

2013/5/31 Daniel Vetter <daniel.vetter@ffwll.ch>:
> Now that we track the cpu transcoder we need accurately in the pipe
> config we can finally fix up the transcoder check. With the current
> code eDP on port D will be broken since we'd errornously cut the
> power.
>
> For reference see
>
> commit 2124b72e6283c4e84a55e71077fee91793f4c801
> Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Date:   Fri Mar 22 14:07:23 2013 -0300
>
>     drm/i915: don't disable the power well yet
>
> v2:
> - Kill the now outdated comment (Paulo)
> - Add the missing crtc->base.enabled check and consolidate it (Paulo)
> - Smash all checks together, looks neater that way.
>
> v3: Kill the unused encoder variable.
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_display.c |   18 ++++--------------
>  1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3db4ed2..c4adbfe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5798,23 +5798,13 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
>  {
>         bool enable = false;
>         struct intel_crtc *crtc;
> -       struct intel_encoder *encoder;
>
>         list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
> -               if (crtc->pipe != PIPE_A && crtc->base.enabled)
> -                       enable = true;
> -               /* XXX: Should check for edp transcoder here, but thanks to init
> -                * sequence that's not yet available. Just in case desktop eDP
> -                * on PORT D is possible on haswell, too. */
> -               /* Even the eDP panel fitter is outside the always-on well. */
> -               if (crtc->config.pch_pfit.size && crtc->base.enabled)
> -                       enable = true;
> -       }
> +               if (!crtc->base.enabled)
> +                       continue;
>
> -       list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> -                           base.head) {
> -               if (encoder->type != INTEL_OUTPUT_EDP &&
> -                   encoder->connectors_active)
> +               if (crtc->pipe != PIPE_A || crtc->config.pch_pfit.size ||
> +                   crtc->config.cpu_transcoder != TRANSCODER_EDP)
>                         enable = true;
>         }
>
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH] drm/i915: fix up the edp power well check

[Daniel Vetter]

On Fri, May 31, 2013 at 02:24:11PM -0300, Paulo Zanoni wrote:
> 2013/5/31 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > Now that we track the cpu transcoder we need accurately in the pipe
> > config we can finally fix up the transcoder check. With the current
> > code eDP on port D will be broken since we'd errornously cut the
> > power.
> >
> > For reference see
> >
> > commit 2124b72e6283c4e84a55e71077fee91793f4c801
> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Date:   Fri Mar 22 14:07:23 2013 -0300
> >
> >     drm/i915: don't disable the power well yet
> >
> > v2:
> > - Kill the now outdated comment (Paulo)
> > - Add the missing crtc->base.enabled check and consolidate it (Paulo)
> > - Smash all checks together, looks neater that way.
> >
> > v3: Kill the unused encoder variable.
> >
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Queued for -next, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch