[1/2] drm/i915: Dump some DPLL fields in pipe config debug
diff mbox

Message ID 1431532268-4363-1-git-send-email-tvrtko.ursulin@linux.intel.com
State New
Headers show

Commit Message

Tvrtko Ursulin May 13, 2015, 3:51 p.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Tried to get the platform split right, please shout if I failed.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

Comments

Lespiau, Damien May 13, 2015, 4:41 p.m. UTC | #1
On Wed, May 13, 2015 at 04:51:07PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Tried to get the platform split right, please shout if I failed.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

So, previously, we used to dump the whole list of states, even when the
platform didn't care about all of them. I do like a platform-aware dump
of the state better and this looks right to me (surely we can break the
strings at a ','!).

You probably also want to add pipe_config->ddi_pll_sel on DDI platforms
as it encodes the actual PLL selection (at least) and maybe look at the
other DPLL-related states in there (I can give a r-b tag without those
:p)

A small typo below.
Daniel Vetter May 18, 2015, 7:57 a.m. UTC | #2
On Wed, May 13, 2015 at 05:41:40PM +0100, Damien Lespiau wrote:
> On Wed, May 13, 2015 at 04:51:07PM +0100, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> > Tried to get the platform split right, please shout if I failed.
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> So, previously, we used to dump the whole list of states, even when the
> platform didn't care about all of them. I do like a platform-aware dump
> of the state better and this looks right to me (surely we can break the
> strings at a ','!).
> 
> You probably also want to add pipe_config->ddi_pll_sel on DDI platforms
> as it encodes the actual PLL selection (at least) and maybe look at the
> other DPLL-related states in there (I can give a r-b tag without those
> :p)
> 
> A small typo below.

Not convinced, for the same reason that we don't have platform checks
(except where absolutely necessary) in the pipe_config_compare code: No
one will notice bug here until it's too late. Otoh the pipe dump code
always seems to be lacking.
-Daniel

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 52f9cbc..b3ed8a0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11418,6 +11418,32 @@  static void intel_dump_pipe_config(struct intel_crtc *crtc,
 	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
 	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
 
+	if (IS_BROXTON(dev)) {
+		DRM_DEBUG_KMS("dpll_hw_state: ebb0: 0x%x, pll0: 0x%x, pll1: 0x%x, pll2: 0x%x, pll3: 0x%x, pll6: 0x%x, pll8: 0x%x, pcsdw12: 0x%x\n",
+		      pipe_config->dpll_hw_state.ebb0,
+		      pipe_config->dpll_hw_state.pll0,
+		      pipe_config->dpll_hw_state.pll1,
+		      pipe_config->dpll_hw_state.pll2,
+		      pipe_config->dpll_hw_state.pll3,
+		      pipe_config->dpll_hw_state.pll6,
+		      pipe_config->dpll_hw_state.pll8,
+		      pipe_config->dpll_hw_state.pcsdw12);
+	} else if (IS_SKYLAKE(dev)) {
+		DRM_DEBUG_KMS("dpll_hw_state: ctrl1: 0x%x, cfgcr1: 0x%x, cfgcr2: 0x%x\n",
+		      pipe_config->dpll_hw_state.ctrl1,
+		      pipe_config->dpll_hw_state.cfgcr1,
+		      pipe_config->dpll_hw_state.cfgcr2);
+	} else if (HAS_DDI(dev)) {
+		DRM_DEBUG_KMS("dpll_hw_state: wrpll: 0x%x\n",
+		      pipe_config->dpll_hw_state.wrpll);
+	} else {
+		DRM_DEBUG_KMS("dpll_hw_state: dpll: 0x%x, dpll_md: 0x%x, fp0: 0x%x, dp1: 0x%x\n",
+		      pipe_config->dpll_hw_state.dpll,
+		      pipe_config->dpll_hw_state.dpll_md,
+		      pipe_config->dpll_hw_state.fp0,
+		      pipe_config->dpll_hw_state.fp1);
+	}
+
 	DRM_DEBUG_KMS("planes on this crtc\n");
 	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
 		intel_plane = to_intel_plane(plane);