Re: [Intel-gfx] [PATCH v2] drm/i915/debugfs : PM_REQ and PM_RES registers
On Mon, 04 Jan 2021, Saichandana S wrote: > From: Saichandana > > PM_REQ register provides the value of the last PM request from PCU to > Display Engine.PM_RES register provides the value of the last PM > response from Display Engine to PCU.This debugfs will be used by > DC9 IGT test to know about "DC9 Ready" status. > > B.Spec : 49501, 49502 > > Signed-off-by: Saichandana > --- > .../drm/i915/display/intel_display_debugfs.c | 30 +++ > drivers/gpu/drm/i915/i915_reg.h | 8 + > 2 files changed, 38 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index cd7e5519ee7d..551fb1a90bb3 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -559,6 +559,36 @@ static int i915_dmc_info(struct seq_file *m, void > *unused) > return 0; > } > > +static int i915_pm_req_res_info(struct seq_file *m, void *unused) > +{ > + struct drm_i915_private *dev_priv = node_to_i915(m->private); All new code should call the local variable "i915". > + struct intel_csr *csr = _priv->csr; > + const char *status; > + > + if (!HAS_CSR(dev_priv)) > + return -ENODEV; > + if (!csr->dmc_payload) > + return 0; > + seq_printf(m, "PM debug request 0 (0x45284): 0x%08x\n", > +intel_de_read(dev_priv, PM_REQ_DBG_0)); > + seq_printf(m, "PM debug request 1 (0x45288): 0x%08x\n", > +intel_de_read(dev_priv, PM_REQ_DBG_1)); > + seq_printf(m, "PM debug response 0 (0x4528C): 0x%08x\n", > +intel_de_read(dev_priv, PM_RSP_DBG_0)); > + seq_printf(m, "PM debug response 1 (0x45290): 0x%08x\n", > +intel_de_read(dev_priv, PM_RSP_DBG_1)); Like I said before [1], do *not* dump the registers. We have userspace tools for dumping register contents as-is when you need that for debug purposes. And even the userspace tool can do some register content parsing. See tools/intel_reg in igt. The only reason to have a debugfs file is to provide some other, added value that a register dump can't provide. [1] http://lore.kernel.org/r/87mtyl8vpu@intel.com > + status = (intel_de_read(dev_priv, PM_RSP_DBG_1) & MASK_DC9_BIT) ? "yes" > : "no"; See yesno() in i915_utils.h. You probably don't want the local variable for the string. > + > + seq_printf(m, "Time to Next Fill = 0x%0x\n", > +(intel_de_read(dev_priv, PM_RSP_DBG_0) & ~MASK_RSP_0)); > + seq_printf(m, "Time to Next VBI = 0x%0x\n", > +((intel_de_read(dev_priv, PM_RSP_DBG_0) & MASK_RSP_0)) >> > 16); > + seq_printf(m, "Selective Exit Latency = 0x%0x\n", > +(intel_de_read(dev_priv, PM_RSP_DBG_1) & MASK_RSP_1)); There's a bunch of unnecessary parenthesis around the 3rd parameter to seq_printf. > + seq_printf(m, "DC9 Ready = %s\n", status); > + return 0; > +} > + > static void intel_seq_print_mode(struct seq_file *m, int tabs, >const struct drm_display_mode *mode) > { > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 0023c023f472..3e9ed555f928 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -371,6 +371,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > #define VLV_G3DCTL _MMIO(0x9024) > #define VLV_GSCKGCTL _MMIO(0x9028) > > +#define PM_REQ_DBG_0 _MMIO(0x45284) > +#define PM_REQ_DBG_1 _MMIO(0x45288) > +#define PM_RSP_DBG_0 _MMIO(0x4528C) > +#define PM_RSP_DBG_1 _MMIO(0x45290) > +#define MASK_RSP_0 (0x << 16) > +#define MASK_RSP_1 (7 << 0) > +#define MASK_DC9_BIT (1 << 17) This is a random location in i915_reg.h, out of place. Please also read the big comment near the top of this file. BR, Jani. > + > #define GEN6_MBCTL _MMIO(0x0907c) > #define GEN6_MBCTL_ENABLE_BOOT_FETCH (1 << 4) > #define GEN6_MBCTL_CTX_FETCH_NEEDED(1 << 3) -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/debugfs : PM_REQ and PM_RES registers
> -Original Message- > From: S, Saichandana > Sent: Monday, January 4, 2021 4:01 PM > To: intel-gfx@lists.freedesktop.org > Cc: S, Saichandana ; Nikula, Jani > ; Gupta, Anshuman > Subject: [PATCH v2] drm/i915/debugfs : PM_REQ and PM_RES registers > > From: Saichandana > > PM_REQ register provides the value of the last PM request from PCU to > Display Engine.PM_RES register provides the value of the last PM response > from Display Engine to PCU.This debugfs will be used by > DC9 IGT test to know about "DC9 Ready" status. > > B.Spec : 49501, 49502 Please mention here the review comment u had fixed. So it will be easy for review. V2: Added a functional print to debugs. [Keep name of Reviewer] > > Signed-off-by: Saichandana > --- > .../drm/i915/display/intel_display_debugfs.c | 30 > +++ > drivers/gpu/drm/i915/i915_reg.h | 8 + > 2 files changed, 38 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index cd7e5519ee7d..551fb1a90bb3 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -559,6 +559,36 @@ static int i915_dmc_info(struct seq_file *m, void > *unused) > return 0; > } > > +static int i915_pm_req_res_info(struct seq_file *m, void *unused) { > + struct drm_i915_private *dev_priv = node_to_i915(m->private); > + struct intel_csr *csr = _priv->csr; > + const char *status; > + > + if (!HAS_CSR(dev_priv)) > + return -ENODEV; > + if (!csr->dmc_payload) > + return 0; > + seq_printf(m, "PM debug request 0 (0x45284): 0x%08x\n", > +intel_de_read(dev_priv, PM_REQ_DBG_0)); > + seq_printf(m, "PM debug request 1 (0x45288): 0x%08x\n", > +intel_de_read(dev_priv, PM_REQ_DBG_1)); > + seq_printf(m, "PM debug response 0 (0x4528C): 0x%08x\n", > +intel_de_read(dev_priv, PM_RSP_DBG_0)); > + seq_printf(m, "PM debug response 1 (0x45290): 0x%08x\n", > +intel_de_read(dev_priv, PM_RSP_DBG_1)); > + status = (intel_de_read(dev_priv, PM_RSP_DBG_1) & > MASK_DC9_BIT) ? > +"yes" : "no"; You don't need to read the value of register again , please store in a variable And use that value. Thanks, Anshuman Gupta. > + > + seq_printf(m, "Time to Next Fill = 0x%0x\n", > +(intel_de_read(dev_priv, PM_RSP_DBG_0) & > ~MASK_RSP_0)); > + seq_printf(m, "Time to Next VBI = 0x%0x\n", > +((intel_de_read(dev_priv, PM_RSP_DBG_0) & > MASK_RSP_0)) >> 16); > + seq_printf(m, "Selective Exit Latency = 0x%0x\n", > +(intel_de_read(dev_priv, PM_RSP_DBG_1) & > MASK_RSP_1)); > + seq_printf(m, "DC9 Ready = %s\n", status); > + return 0; > +} > + > static void intel_seq_print_mode(struct seq_file *m, int tabs, >const struct drm_display_mode *mode) { > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h index 0023c023f472..3e9ed555f928 > 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -371,6 +371,14 @@ static inline bool > i915_mmio_reg_valid(i915_reg_t reg) > #define VLV_G3DCTL _MMIO(0x9024) > #define VLV_GSCKGCTL _MMIO(0x9028) > > +#define PM_REQ_DBG_0 _MMIO(0x45284) > +#define PM_REQ_DBG_1 _MMIO(0x45288) > +#define PM_RSP_DBG_0 _MMIO(0x4528C) > +#define PM_RSP_DBG_1 _MMIO(0x45290) > +#define MASK_RSP_0 (0x << 16) > +#define MASK_RSP_1 (7 << 0) > +#define MASK_DC9_BIT (1 << 17) > + > #define GEN6_MBCTL _MMIO(0x0907c) > #define GEN6_MBCTL_ENABLE_BOOT_FETCH (1 << 4) > #define GEN6_MBCTL_CTX_FETCH_NEEDED(1 << 3) > -- > 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/debugfs : PM_REQ and PM_RES registers
From: Saichandana PM_REQ register provides the value of the last PM request from PCU to Display Engine.PM_RES register provides the value of the last PM response from Display Engine to PCU.This debugfs will be used by DC9 IGT test to know about "DC9 Ready" status. B.Spec : 49501, 49502 Signed-off-by: Saichandana --- .../drm/i915/display/intel_display_debugfs.c | 30 +++ drivers/gpu/drm/i915/i915_reg.h | 8 + 2 files changed, 38 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c index cd7e5519ee7d..551fb1a90bb3 100644 --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c @@ -559,6 +559,36 @@ static int i915_dmc_info(struct seq_file *m, void *unused) return 0; } +static int i915_pm_req_res_info(struct seq_file *m, void *unused) +{ + struct drm_i915_private *dev_priv = node_to_i915(m->private); + struct intel_csr *csr = _priv->csr; + const char *status; + + if (!HAS_CSR(dev_priv)) + return -ENODEV; + if (!csr->dmc_payload) + return 0; + seq_printf(m, "PM debug request 0 (0x45284): 0x%08x\n", + intel_de_read(dev_priv, PM_REQ_DBG_0)); + seq_printf(m, "PM debug request 1 (0x45288): 0x%08x\n", + intel_de_read(dev_priv, PM_REQ_DBG_1)); + seq_printf(m, "PM debug response 0 (0x4528C): 0x%08x\n", + intel_de_read(dev_priv, PM_RSP_DBG_0)); + seq_printf(m, "PM debug response 1 (0x45290): 0x%08x\n", + intel_de_read(dev_priv, PM_RSP_DBG_1)); + status = (intel_de_read(dev_priv, PM_RSP_DBG_1) & MASK_DC9_BIT) ? "yes" : "no"; + + seq_printf(m, "Time to Next Fill = 0x%0x\n", + (intel_de_read(dev_priv, PM_RSP_DBG_0) & ~MASK_RSP_0)); + seq_printf(m, "Time to Next VBI = 0x%0x\n", + ((intel_de_read(dev_priv, PM_RSP_DBG_0) & MASK_RSP_0)) >> 16); + seq_printf(m, "Selective Exit Latency = 0x%0x\n", + (intel_de_read(dev_priv, PM_RSP_DBG_1) & MASK_RSP_1)); + seq_printf(m, "DC9 Ready = %s\n", status); + return 0; +} + static void intel_seq_print_mode(struct seq_file *m, int tabs, const struct drm_display_mode *mode) { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 0023c023f472..3e9ed555f928 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -371,6 +371,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define VLV_G3DCTL _MMIO(0x9024) #define VLV_GSCKGCTL _MMIO(0x9028) +#define PM_REQ_DBG_0 _MMIO(0x45284) +#define PM_REQ_DBG_1 _MMIO(0x45288) +#define PM_RSP_DBG_0 _MMIO(0x4528C) +#define PM_RSP_DBG_1 _MMIO(0x45290) +#define MASK_RSP_0 (0x << 16) +#define MASK_RSP_1 (7 << 0) +#define MASK_DC9_BIT (1 << 17) + #define GEN6_MBCTL _MMIO(0x0907c) #define GEN6_MBCTL_ENABLE_BOOT_FETCH (1 << 4) #define GEN6_MBCTL_CTX_FETCH_NEEDED (1 << 3) -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx