All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too
@ 2014-11-17 21:08 Jesse Barnes
  2014-11-17 21:08 ` [PATCH 2/2] drm/i915/ddi: add break in DDI mode select switch Jesse Barnes
  2014-11-18  8:14 ` [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too Daniel Vetter
  0 siblings, 2 replies; 13+ messages in thread
From: Jesse Barnes @ 2014-11-17 21:08 UTC (permalink / raw)
  To: intel-gfx

Just like we do in the HDMI code, set the infoframe flag if we detect an
HDMI sink.

Reported-by: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_ddi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 86745da..78576e0 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2062,6 +2062,7 @@ 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;
+		pipe_config->has_infoframe = true;
 	case TRANS_DDI_MODE_SELECT_DVI:
 	case TRANS_DDI_MODE_SELECT_FDI:
 		break;
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] drm/i915/ddi: add break in DDI mode select switch
  2014-11-17 21:08 [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too Jesse Barnes
@ 2014-11-17 21:08 ` Jesse Barnes
  2014-11-18  3:27   ` [PATCH 2/2] drm/i915/ddi: add break in DDI mode select shuang.he
  2014-11-18  8:14   ` [PATCH 2/2] drm/i915/ddi: add break in DDI mode select switch Daniel Vetter
  2014-11-18  8:14 ` [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too Daniel Vetter
  1 sibling, 2 replies; 13+ messages in thread
From: Jesse Barnes @ 2014-11-17 21:08 UTC (permalink / raw)
  To: intel-gfx

The lack of a break here wasn't for falling through to some other
important code, so made me do a double take.  Add a break just to make
things a little less confusing.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_ddi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 78576e0..66c373b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2063,6 +2063,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 	case TRANS_DDI_MODE_SELECT_HDMI:
 		pipe_config->has_hdmi_sink = true;
 		pipe_config->has_infoframe = true;
+		break;
 	case TRANS_DDI_MODE_SELECT_DVI:
 	case TRANS_DDI_MODE_SELECT_FDI:
 		break;
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] drm/i915/ddi: add break in DDI mode select
  2014-11-17 21:08 ` [PATCH 2/2] drm/i915/ddi: add break in DDI mode select switch Jesse Barnes
@ 2014-11-18  3:27   ` shuang.he
  2014-11-18  8:14   ` [PATCH 2/2] drm/i915/ddi: add break in DDI mode select switch Daniel Vetter
  1 sibling, 0 replies; 13+ messages in thread
From: shuang.he @ 2014-11-18  3:27 UTC (permalink / raw)
  To: shuang.he, intel-gfx, jbarnes

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
BYT: pass/total=290/291->291/291
PNV: pass/total=356/356->356/356
ILK: pass/total=371/372->367/372
IVB: pass/total=544/546->546/546
SNB: pass/total=424/425->425/425
HSW: pass/total=579/579->579/579
BDW: pass/total=432/435->432/435
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
BYT: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M36)PASS(3, M31) -> TIMEOUT(3, M31)PASS(1, M31)
ILK: Intel_gpu_tools, igt_kms_flip_bcs-flip-vs-modeset-interruptible, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_flip_bcs-wf_vblank-vs-dpms, PASS(4, M37M26) -> NSPT(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-modeset-vs-hang-interruptible, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_flip_plain-flip-fb-recreate-interruptible, PASS(4, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_flip_rcs-flip-vs-dpms, DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M37)PASS(3, M26) -> FAIL(1, M26)DMESG_FAIL(1, M26)TIMEOUT(1, M26)PASS(1, M26)
IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-random, DMESG_WARN(1, M21)PASS(3, M21) -> PASS(4, M21)
IVB: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M21)PASS(3, M21) -> TIMEOUT(3, M21)PASS(1, M21)
SNB: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M35)PASS(3, M35) -> TIMEOUT(3, M35)PASS(1, M35)
BDW: Intel_gpu_tools, igt_gem_reset_stats_close-pending-blt, PASS(4, M28M30) -> FAIL(1, M30)PASS(3, M30)
BDW: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M28)PASS(3, M30) -> TIMEOUT(3, M30)PASS(1, M30)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too
  2014-11-17 21:08 [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too Jesse Barnes
  2014-11-17 21:08 ` [PATCH 2/2] drm/i915/ddi: add break in DDI mode select switch Jesse Barnes
@ 2014-11-18  8:14 ` Daniel Vetter
  2014-11-18 16:16   ` Jesse Barnes
  2014-11-18 17:45   ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2 Jesse Barnes
  1 sibling, 2 replies; 13+ messages in thread
From: Daniel Vetter @ 2014-11-18  8:14 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Mon, Nov 17, 2014 at 01:08:46PM -0800, Jesse Barnes wrote:
> Just like we do in the HDMI code, set the infoframe flag if we detect an
> HDMI sink.
> 
> Reported-by: Paulo Zanoni <przanoni@gmail.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 86745da..78576e0 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2062,6 +2062,7 @@ 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;
> +		pipe_config->has_infoframe = true;

Infoframes aren't controlled by this bit here but set in
HSW_TVIDEO_DIP_CTL. Since the point of this is to detect mismatches
between the bios and what we'd like to have for fastboot I think we need
to check that register. Instead of blindly deriving state to appease the
cross checker.
-Daniel

>  	case TRANS_DDI_MODE_SELECT_DVI:
>  	case TRANS_DDI_MODE_SELECT_FDI:
>  		break;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] drm/i915/ddi: add break in DDI mode select switch
  2014-11-17 21:08 ` [PATCH 2/2] drm/i915/ddi: add break in DDI mode select switch Jesse Barnes
  2014-11-18  3:27   ` [PATCH 2/2] drm/i915/ddi: add break in DDI mode select shuang.he
@ 2014-11-18  8:14   ` Daniel Vetter
  1 sibling, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2014-11-18  8:14 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Mon, Nov 17, 2014 at 01:08:47PM -0800, Jesse Barnes wrote:
> The lack of a break here wasn't for falling through to some other
> important code, so made me do a double take.  Add a break just to make
> things a little less confusing.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Confusing indeed. Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too
  2014-11-18  8:14 ` [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too Daniel Vetter
@ 2014-11-18 16:16   ` Jesse Barnes
  2014-11-18 17:45   ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2 Jesse Barnes
  1 sibling, 0 replies; 13+ messages in thread
From: Jesse Barnes @ 2014-11-18 16:16 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Tue, 18 Nov 2014 09:14:05 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Nov 17, 2014 at 01:08:46PM -0800, Jesse Barnes wrote:
> > Just like we do in the HDMI code, set the infoframe flag if we detect an
> > HDMI sink.
> > 
> > Reported-by: Paulo Zanoni <przanoni@gmail.com>
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 86745da..78576e0 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2062,6 +2062,7 @@ 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;
> > +		pipe_config->has_infoframe = true;
> 
> Infoframes aren't controlled by this bit here but set in
> HSW_TVIDEO_DIP_CTL. Since the point of this is to detect mismatches
> between the bios and what we'd like to have for fastboot I think we need
> to check that register. Instead of blindly deriving state to appease the
> cross checker.

Oh you're right; I was thinking of compute_config.  I'll send a follow
up.

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2
  2014-11-18  8:14 ` [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too Daniel Vetter
  2014-11-18 16:16   ` Jesse Barnes
@ 2014-11-18 17:45   ` Jesse Barnes
  2014-11-19  0:00     ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too shuang.he
                       ` (2 more replies)
  1 sibling, 3 replies; 13+ messages in thread
From: Jesse Barnes @ 2014-11-18 17:45 UTC (permalink / raw)
  To: intel-gfx

Just like we do in the HDMI code, set the infoframe flag if we detect
that infoframes are enabled.

v2: check for actual infoframe status as in hdmi code (Daniel)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_ddi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 07c5625..24110c9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2075,6 +2075,14 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 		break;
 	}
 
+	if (encoder->type == INTEL_OUTPUT_HDMI) {
+		struct intel_hdmi *intel_hdmi =
+			enc_to_intel_hdmi(&encoder->base);
+
+		if (intel_hdmi->infoframe_enabled(&encoder->base))
+			pipe_config->has_infoframe = true;
+	}
+
 	if (intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_AUDIO)) {
 		temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
 		if (temp & AUDIO_OUTPUT_ENABLE(intel_crtc->pipe))
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too
  2014-11-18 17:45   ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2 Jesse Barnes
@ 2014-11-19  0:00     ` shuang.he
  2014-11-19 13:53       ` Daniel Vetter
  2014-11-19 13:55     ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2 Daniel Vetter
  2014-11-20 15:49     ` Daniel Vetter
  2 siblings, 1 reply; 13+ messages in thread
From: shuang.he @ 2014-11-19  0:00 UTC (permalink / raw)
  To: shuang.he, intel-gfx, jbarnes

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
BYT: pass/total=290/290->290/290
PNV: pass/total=362/362->362/362
ILK: pass/total=381/381->381/381
IVB: pass/total=522/559->522/559
SNB: pass/total=444/444->444/444
HSW: pass/total=595/595->595/595
BDW: pass/total=436/436->435/436
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
BDW: Intel_gpu_tools, igt_gem_reset_stats_close-pending-ctx-render, PASS(4, M28) -> FAIL(1, M28)PASS(3, M28)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too
  2014-11-19  0:00     ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too shuang.he
@ 2014-11-19 13:53       ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2014-11-19 13:53 UTC (permalink / raw)
  To: shuang.he; +Cc: intel-gfx

On Tue, Nov 18, 2014 at 04:00:58PM -0800, shuang.he@intel.com wrote:
> Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
> -------------------------------------Summary-------------------------------------
> Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
> BYT: pass/total=290/290->290/290
> PNV: pass/total=362/362->362/362
> ILK: pass/total=381/381->381/381
> IVB: pass/total=522/559->522/559
> SNB: pass/total=444/444->444/444
> HSW: pass/total=595/595->595/595
> BDW: pass/total=436/436->435/436
> -------------------------------------Detailed-------------------------------------
> test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
> BDW: Intel_gpu_tools, igt_gem_reset_stats_close-pending-ctx-render, PASS(4, M28) -> FAIL(1, M28)PASS(3, M28)

This patch /should/ fix a pretty decent amount of WARN backtrace in
modeset tests on hsw/bdw (as long as there's a hdmi screen around).
Instead it causes a completely unrelated test to fail.

Can you please dig into what's going on here?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2
  2014-11-18 17:45   ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2 Jesse Barnes
  2014-11-19  0:00     ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too shuang.he
@ 2014-11-19 13:55     ` Daniel Vetter
  2014-11-20 15:49     ` Daniel Vetter
  2 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2014-11-19 13:55 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Tue, Nov 18, 2014 at 09:45:52AM -0800, Jesse Barnes wrote:
> Just like we do in the HDMI code, set the infoframe flag if we detect
> that infoframes are enabled.
> 
> v2: check for actual infoframe status as in hdmi code (Daniel)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Queued for -next, thanks for the patch.

Now I need to cry over why QA hasn't found this one :(
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2
  2014-11-18 17:45   ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2 Jesse Barnes
  2014-11-19  0:00     ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too shuang.he
  2014-11-19 13:55     ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2 Daniel Vetter
@ 2014-11-20 15:49     ` Daniel Vetter
  2014-11-20 19:54       ` Jesse Barnes
  2 siblings, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2014-11-20 15:49 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Tue, Nov 18, 2014 at 09:45:52AM -0800, Jesse Barnes wrote:
> Just like we do in the HDMI code, set the infoframe flag if we detect
> that infoframes are enabled.
> 
> v2: check for actual infoframe status as in hdmi code (Daniel)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 07c5625..24110c9 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2075,6 +2075,14 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  		break;
>  	}
>  
> +	if (encoder->type == INTEL_OUTPUT_HDMI) {

Hm I dind't look too closely apparently at this. You again rely upon sw
state here, just encoder->type this time around. Which means you can't
upcast the intel_hdmi struct, and you also can't really rely upon the
encoder->crtc link (that's all just about to get reconstructed). Imo the
code should have stayed in the TRANS_DDI_MODE_SELECT_HDMI case.

The later depency upon encoder->crtc is an issue for everything !g4x, but
on hsw there's the additional issue that you have to look at the cpu
transcoder and I guess that part blows up.

g4x infoframe readout is probably broken too because it doesn't check that
the port selected is the one actually queried for.

Overall I think we need to:
- Inline the g4x readout into the hdmi get_config function and check the
  port.
- Inline the ibx/cpt readout code into the relevant get_pipe_config
  functions (well pch config) since that state is per-pipe. We should
  probably double-check the port, too.
- Same inline for vlv and hsw, with the addition that we need to make sure
  on hsw to not try to read this for the edp transcoder.

Or maybe I'm totally missing why the state gets out of sync on Paulo's
machine.
-Daniel

> +		struct intel_hdmi *intel_hdmi =
> +			enc_to_intel_hdmi(&encoder->base);
> +
> +		if (intel_hdmi->infoframe_enabled(&encoder->base))
> +			pipe_config->has_infoframe = true;
> +	}
> +
>  	if (intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_AUDIO)) {
>  		temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>  		if (temp & AUDIO_OUTPUT_ENABLE(intel_crtc->pipe))
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2
  2014-11-20 15:49     ` Daniel Vetter
@ 2014-11-20 19:54       ` Jesse Barnes
  2014-11-20 21:40         ` Daniel Vetter
  0 siblings, 1 reply; 13+ messages in thread
From: Jesse Barnes @ 2014-11-20 19:54 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Thu, 20 Nov 2014 16:49:42 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Nov 18, 2014 at 09:45:52AM -0800, Jesse Barnes wrote:
> > Just like we do in the HDMI code, set the infoframe flag if we detect
> > that infoframes are enabled.
> > 
> > v2: check for actual infoframe status as in hdmi code (Daniel)
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 07c5625..24110c9 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2075,6 +2075,14 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  		break;
> >  	}
> >  
> > +	if (encoder->type == INTEL_OUTPUT_HDMI) {
> 
> Hm I dind't look too closely apparently at this. You again rely upon sw
> state here, just encoder->type this time around. Which means you can't
> upcast the intel_hdmi struct, and you also can't really rely upon the
> encoder->crtc link (that's all just about to get reconstructed). Imo the
> code should have stayed in the TRANS_DDI_MODE_SELECT_HDMI case.

Hm, we look at the encoder->type later and check for eDP, so I figured
it must be valid at this point.  It's easy enough to move though if
not...

> The later depency upon encoder->crtc is an issue for everything !g4x, but
> on hsw there's the additional issue that you have to look at the cpu
> transcoder and I guess that part blows up.

I'll check out Paulo's trace; haven't looked yet.

> g4x infoframe readout is probably broken too because it doesn't check that
> the port selected is the one actually queried for.

Yeah that looks like a separate bug from the infoframe_enabled
additions.

> 
> Overall I think we need to:
> - Inline the g4x readout into the hdmi get_config function and check the
>   port.
> - Inline the ibx/cpt readout code into the relevant get_pipe_config
>   functions (well pch config) since that state is per-pipe. We should
>   probably double-check the port, too.
> - Same inline for vlv and hsw, with the addition that we need to make sure
>   on hsw to not try to read this for the edp transcoder.

I'll check, we may not need to inline since we should be able to get
the port info easily enough.

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2
  2014-11-20 19:54       ` Jesse Barnes
@ 2014-11-20 21:40         ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2014-11-20 21:40 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

Let's be good citizen and summarize the important points from the irc
chat between Jesse&me.

On Thu, Nov 20, 2014 at 8:54 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> Hm I dind't look too closely apparently at this. You again rely upon sw
>> state here, just encoder->type this time around. Which means you can't
>> upcast the intel_hdmi struct, and you also can't really rely upon the
>> encoder->crtc link (that's all just about to get reconstructed). Imo the
>> code should have stayed in the TRANS_DDI_MODE_SELECT_HDMI case.
>
> Hm, we look at the encoder->type later and check for eDP, so I figured
> it must be valid at this point.  It's easy enough to move though if
> not...

eDP is special since it doesn't change its personality ever. That's
why we get away with it. And there's also not really any indication
whether a given port would be eDP in the hw since at least on big core
the panel power sequencer is global (and not per-pipe like on vlv/bsw)
so we just can't tell. Well we could poke at the dp aux stuff.

>> The later depency upon encoder->crtc is an issue for everything !g4x, but
>> on hsw there's the additional issue that you have to look at the cpu
>> transcoder and I guess that part blows up.
>
> I'll check out Paulo's trace; haven't looked yet.

Summary from our irc analysis is in the patch I've just submitted.
Hopefully that fixes Paulo's backtrace.

>> g4x infoframe readout is probably broken too because it doesn't check that
>> the port selected is the one actually queried for.
>
> Yeah that looks like a separate bug from the infoframe_enabled
> additions.
>
>>
>> Overall I think we need to:
>> - Inline the g4x readout into the hdmi get_config function and check the
>>   port.
>> - Inline the ibx/cpt readout code into the relevant get_pipe_config
>>   functions (well pch config) since that state is per-pipe. We should
>>   probably double-check the port, too.
>> - Same inline for vlv and hsw, with the addition that we need to make sure
>>   on hsw to not try to read this for the edp transcoder.
>
> I'll check, we may not need to inline since we should be able to get
> the port info easily enough.

Per our discussion encoder->crtc gets reconstructed properly so we
should be able to rely on that relationship. Might be good paranoid
practice to double-check the port mapping for ibx/vlv, but imo not
really required.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-11-20 21:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-17 21:08 [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too Jesse Barnes
2014-11-17 21:08 ` [PATCH 2/2] drm/i915/ddi: add break in DDI mode select switch Jesse Barnes
2014-11-18  3:27   ` [PATCH 2/2] drm/i915/ddi: add break in DDI mode select shuang.he
2014-11-18  8:14   ` [PATCH 2/2] drm/i915/ddi: add break in DDI mode select switch Daniel Vetter
2014-11-18  8:14 ` [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too Daniel Vetter
2014-11-18 16:16   ` Jesse Barnes
2014-11-18 17:45   ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2 Jesse Barnes
2014-11-19  0:00     ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too shuang.he
2014-11-19 13:53       ` Daniel Vetter
2014-11-19 13:55     ` [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2 Daniel Vetter
2014-11-20 15:49     ` Daniel Vetter
2014-11-20 19:54       ` Jesse Barnes
2014-11-20 21:40         ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.