* [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 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-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 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox