Re: [Intel-gfx] [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
On Fri, Oct 05, 2018 at 11:04:35PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 05, 2018 at 02:08:46PM -0400, Jyoti Yadav wrote:
> > DC5 and DC6 counter register tells about residency of DC5 and DC6.
> > Added the same in debugfs file.
> >
> > v2 : Remove csr_version check.
> > Added generic check regarding DC counters for Gen9 onwards. (Rodrigo)
> > v3 : Simplified gen checks. (Chris)
> > v4 : Simplified "if" ladder for multiple gens.
> > v5 : Removed unnecessary comment.
> >
> > Signed-off-by: Jyoti Yadav
> > Reviewed-by: Rodrigo Vivi
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 9 -
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index a5265c2..738f8c7 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2897,15 +2897,14 @@ static int i915_dmc_info(struct seq_file *m, void
> > *unused)
> > seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
> >CSR_VERSION_MINOR(csr->version));
> >
> > - if (IS_KABYLAKE(dev_priv) ||
> > - (IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) {
> > + if (IS_BROXTON(dev_priv)) {
> > + seq_printf(m, "DC3 -> DC5 count: %d\n",
> > + I915_READ(BXT_CSR_DC3_DC5_COUNT));
> > + } else if (IS_GEN(dev_priv, 9, 11)) {
>
> What about CFL/GLK/CNL? They didn't take either branch previously.
In the past we didn't have the confirmation from DMC teams
the registers were there. But we got confirmation that they
are there and other drivers use those for validation.
So this adds for all platforms. Maybe commit message could address
that better? But the patch result is right imh so I believe we
could just go ahead.
Also I understand that Jyoti is working on IGT test cases for
this in a generic way too...
>
> > seq_printf(m, "DC3 -> DC5 count: %d\n",
> >I915_READ(SKL_CSR_DC3_DC5_COUNT));
> > seq_printf(m, "DC5 -> DC6 count: %d\n",
> >I915_READ(SKL_CSR_DC5_DC6_COUNT));
> > - } else if (IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1, 4)) {
> > - seq_printf(m, "DC3 -> DC5 count: %d\n",
> > - I915_READ(BXT_CSR_DC3_DC5_COUNT));
> > }
> >
> > out:
> > --
> > 1.9.1
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
On Fri, Oct 05, 2018 at 02:08:46PM -0400, Jyoti Yadav wrote:
> DC5 and DC6 counter register tells about residency of DC5 and DC6.
> Added the same in debugfs file.
>
> v2 : Remove csr_version check.
> Added generic check regarding DC counters for Gen9 onwards. (Rodrigo)
> v3 : Simplified gen checks. (Chris)
> v4 : Simplified "if" ladder for multiple gens.
> v5 : Removed unnecessary comment.
>
> Signed-off-by: Jyoti Yadav
> Reviewed-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index a5265c2..738f8c7 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2897,15 +2897,14 @@ static int i915_dmc_info(struct seq_file *m, void
> *unused)
> seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
> CSR_VERSION_MINOR(csr->version));
>
> - if (IS_KABYLAKE(dev_priv) ||
> - (IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) {
> + if (IS_BROXTON(dev_priv)) {
> + seq_printf(m, "DC3 -> DC5 count: %d\n",
> +I915_READ(BXT_CSR_DC3_DC5_COUNT));
> + } else if (IS_GEN(dev_priv, 9, 11)) {
What about CFL/GLK/CNL? They didn't take either branch previously.
> seq_printf(m, "DC3 -> DC5 count: %d\n",
> I915_READ(SKL_CSR_DC3_DC5_COUNT));
> seq_printf(m, "DC5 -> DC6 count: %d\n",
> I915_READ(SKL_CSR_DC5_DC6_COUNT));
> - } else if (IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1, 4)) {
> - seq_printf(m, "DC3 -> DC5 count: %d\n",
> -I915_READ(BXT_CSR_DC3_DC5_COUNT));
> }
>
> out:
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
On Fri, Oct 05, 2018 at 12:02:26AM -0400, Jyoti Yadav wrote:
> DC5 and DC6 counter register tells about residency of DC5 and DC6.
> These registers are same for SKL and ICL.
>
> v2 : Remove csr_version check.
> Added generic check regarding DC counters for Gen9 onwards. (Rodrigo)
> v3 : Simplified gen checks. (Chris)
> v4 : Simplified "if" ladder for multiple gens.
>
> Signed-off-by: Jyoti Yadav
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 9 -
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index a5265c2..738f8c7 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2897,15 +2897,14 @@ static int i915_dmc_info(struct seq_file *m, void
> *unused)
> seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
> CSR_VERSION_MINOR(csr->version));
>
> - if (IS_KABYLAKE(dev_priv) ||
> - (IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) {
> + if (IS_BROXTON(dev_priv)) {
> + seq_printf(m, "DC3 -> DC5 count: %d\n",
> +I915_READ(BXT_CSR_DC3_DC5_COUNT));
> + } else if (IS_GEN(dev_priv, 9, 11)) {
> seq_printf(m, "DC3 -> DC5 count: %d\n",
> I915_READ(SKL_CSR_DC3_DC5_COUNT));
> seq_printf(m, "DC5 -> DC6 count: %d\n",
> I915_READ(SKL_CSR_DC5_DC6_COUNT));
> - } else if (IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1, 4)) {
> - seq_printf(m, "DC3 -> DC5 count: %d\n",
> -I915_READ(BXT_CSR_DC3_DC5_COUNT));
> }
>
> out:
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8534f88..573d5f3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6985,6 +6985,7 @@ enum {
> /* MMIO address range for CSR program (0x8 - 0x82FFF) */
> #define CSR_MMIO_START_RANGE 0x8
> #define CSR_MMIO_END_RANGE 0x8
> +/* DC3_DC5 count and DC5_DC6 count registers are same for SKL and ICL */
This comment is incorrect... it is same for skl, kbl, cfl, cnl, whl, aml,
and icl.
But it is also useless because the code is clear already.
And not needed, because "SKL_" prefix shows already
started on SKL, but can be used for SKL+ or not...
Also it has a risk of getting outdated and forgotten.
So, with this removed:
Reviewed-by: Rodrigo Vivi
Sorry for not spotting this earlier.
> #define SKL_CSR_DC3_DC5_COUNT_MMIO(0x80030)
> #define SKL_CSR_DC5_DC6_COUNT_MMIO(0x8002C)
> #define BXT_CSR_DC3_DC5_COUNT_MMIO(0x80038)
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
On Wed, Oct 03, 2018 at 07:51:24AM -0700, Rodrigo Vivi wrote:
> On Wed, Oct 03, 2018 at 11:27:42AM +0530, Yadav, Jyoti R wrote:
> >
> >
> > On 10/3/2018 10:36 AM, Vivi, Rodrigo wrote:
> > >
> > > > On Oct 2, 2018, at 9:20 PM, Yadav, Jyoti R
> > > > wrote:
> > > >
> > > > DC5 and DC6 counter register tells about residency of DC5 and DC6.
> > > > These registers are same for SKL and ICL.
> > > >
> > > > v2 : Remove csr_version check.
> > > > Added generic check regarding DC counters for Gen9 onwards.
> > > > (Rodrigo)
> > > > v3 : Simplified gen checks. (Chris)
> > > >
> > > > Signed-off-by: Jyoti Yadav
> > > > ---
> > > > drivers/gpu/drm/i915/i915_debugfs.c | 5 ++---
> > > > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > index a5265c2..af13077 100644
> > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > @@ -2897,13 +2897,12 @@ static int i915_dmc_info(struct seq_file *m,
> > > > void *unused)
> > > > seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
> > > >CSR_VERSION_MINOR(csr->version));
> > > >
> > > > -if (IS_KABYLAKE(dev_priv) ||
> > > > -(IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) {
> > > > +if ((!IS_BROXTON(dev_priv)) && IS_GEN(dev_priv, 9, 11)) {
> > > > seq_printf(m, "DC3 -> DC5 count: %d\n",
> > > >I915_READ(SKL_CSR_DC3_DC5_COUNT));
> > > > seq_printf(m, "DC5 -> DC6 count: %d\n",
> > > >I915_READ(SKL_CSR_DC5_DC6_COUNT));
> > > > -} else if (IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1,
> > > > 4)) {
> > > > +} else if (IS_BROXTON(dev_priv)) {
> > > > seq_printf(m, "DC3 -> DC5 count: %d\n",
> > > >I915_READ(BXT_CSR_DC3_DC5_COUNT));
> > > > }
> > > Please do the other way around...
> > > If is broxton {
> > > } else if gen(9,11) {
> > > }
> > >
> > > So no need for repetition with nots...
> > I was thinking of doing the same earlier, but then thought we should
> > maintain Platform hierarchy. Same trend is followed in other files as well.
>
> Hmm... general guidance is to leave newer platforms at the top indeed.
> But I think we cannot enforce that as a hard rule when code cleanliness is
> compromised.
> Besides that we already have other exceptions around and this case is a range
> against only platform who is exception, not actually a platform order. BXT is
> inside the range(9, 11) and is the exception... SKL is older than BXT and in
> your proposed block it comes before bxt what breaks that rule anyways right?!
I think the "new first, old last" is a good general guideline. But I
agree with Rodrigo that sometimes that would make things too messy.
In which case I think we can extend the guideline to "helpful
exceptions first, then follow the new->old order". Eg. VLV/CHV
have a habit of needing such exceptions.
I think generally if you have to stop and really think what the
if-statement is saying you should do something to simplify it.
Sometimes just reordering the if ladder a bit is enough, other
times it can be helpful to just extract the entire condition
into a small helper function that has a descriptive name.
Eg.
if (IS_A || (gen >= x && !IS_B))
vs.
if (has_something())
The first one almost needs pen and paper to decypher. With the
second one you can just keep on reading without having to
waste further brain cells.
>
> > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > index 8534f88..573d5f3 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -6985,6 +6985,7 @@ enum {
> > > > /* MMIO address range for CSR program (0x8 - 0x82FFF) */
> > > > #define CSR_MMIO_START_RANGE0x8
> > > > #define CSR_MMIO_END_RANGE0x8
> > > > +/* DC3_DC5 count and DC5_DC6 count registers are same for SKL and ICL
> > > > */
> > > > #define SKL_CSR_DC3_DC5_COUNT_MMIO(0x80030)
> > > > #define SKL_CSR_DC5_DC6_COUNT_MMIO(0x8002C)
> > > > #define BXT_CSR_DC3_DC5_COUNT_MMIO(0x80038)
> > > > --
> > > > 1.9.1
> > > >
> >
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
On Wed, Oct 03, 2018 at 11:27:42AM +0530, Yadav, Jyoti R wrote:
>
>
> On 10/3/2018 10:36 AM, Vivi, Rodrigo wrote:
> >
> > > On Oct 2, 2018, at 9:20 PM, Yadav, Jyoti R
> > > wrote:
> > >
> > > DC5 and DC6 counter register tells about residency of DC5 and DC6.
> > > These registers are same for SKL and ICL.
> > >
> > > v2 : Remove csr_version check.
> > > Added generic check regarding DC counters for Gen9 onwards.
> > > (Rodrigo)
> > > v3 : Simplified gen checks. (Chris)
> > >
> > > Signed-off-by: Jyoti Yadav
> > > ---
> > > drivers/gpu/drm/i915/i915_debugfs.c | 5 ++---
> > > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index a5265c2..af13077 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -2897,13 +2897,12 @@ static int i915_dmc_info(struct seq_file *m, void
> > > *unused)
> > > seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
> > >CSR_VERSION_MINOR(csr->version));
> > >
> > > -if (IS_KABYLAKE(dev_priv) ||
> > > -(IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) {
> > > +if ((!IS_BROXTON(dev_priv)) && IS_GEN(dev_priv, 9, 11)) {
> > > seq_printf(m, "DC3 -> DC5 count: %d\n",
> > >I915_READ(SKL_CSR_DC3_DC5_COUNT));
> > > seq_printf(m, "DC5 -> DC6 count: %d\n",
> > >I915_READ(SKL_CSR_DC5_DC6_COUNT));
> > > -} else if (IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1,
> > > 4)) {
> > > +} else if (IS_BROXTON(dev_priv)) {
> > > seq_printf(m, "DC3 -> DC5 count: %d\n",
> > >I915_READ(BXT_CSR_DC3_DC5_COUNT));
> > > }
> > Please do the other way around...
> > If is broxton {
> > } else if gen(9,11) {
> > }
> >
> > So no need for repetition with nots...
> I was thinking of doing the same earlier, but then thought we should
> maintain Platform hierarchy. Same trend is followed in other files as well.
Hmm... general guidance is to leave newer platforms at the top indeed.
But I think we cannot enforce that as a hard rule when code cleanliness is
compromised.
Besides that we already have other exceptions around and this case is a range
against only platform who is exception, not actually a platform order. BXT is
inside the range(9, 11) and is the exception... SKL is older than BXT and in
your proposed block it comes before bxt what breaks that rule anyways right?!
> >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 8534f88..573d5f3 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6985,6 +6985,7 @@ enum {
> > > /* MMIO address range for CSR program (0x8 - 0x82FFF) */
> > > #define CSR_MMIO_START_RANGE0x8
> > > #define CSR_MMIO_END_RANGE0x8
> > > +/* DC3_DC5 count and DC5_DC6 count registers are same for SKL and ICL */
> > > #define SKL_CSR_DC3_DC5_COUNT_MMIO(0x80030)
> > > #define SKL_CSR_DC5_DC6_COUNT_MMIO(0x8002C)
> > > #define BXT_CSR_DC3_DC5_COUNT_MMIO(0x80038)
> > > --
> > > 1.9.1
> > >
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
On 10/3/2018 10:36 AM, Vivi, Rodrigo wrote:
On Oct 2, 2018, at 9:20 PM, Yadav, Jyoti R wrote:
DC5 and DC6 counter register tells about residency of DC5 and DC6.
These registers are same for SKL and ICL.
v2 : Remove csr_version check.
Added generic check regarding DC counters for Gen9 onwards. (Rodrigo)
v3 : Simplified gen checks. (Chris)
Signed-off-by: Jyoti Yadav
---
drivers/gpu/drm/i915/i915_debugfs.c | 5 ++---
drivers/gpu/drm/i915/i915_reg.h | 1 +
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index a5265c2..af13077 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2897,13 +2897,12 @@ static int i915_dmc_info(struct seq_file *m, void
*unused)
seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
CSR_VERSION_MINOR(csr->version));
-if (IS_KABYLAKE(dev_priv) ||
-(IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) {
+if ((!IS_BROXTON(dev_priv)) && IS_GEN(dev_priv, 9, 11)) {
seq_printf(m, "DC3 -> DC5 count: %d\n",
I915_READ(SKL_CSR_DC3_DC5_COUNT));
seq_printf(m, "DC5 -> DC6 count: %d\n",
I915_READ(SKL_CSR_DC5_DC6_COUNT));
-} else if (IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1, 4)) {
+} else if (IS_BROXTON(dev_priv)) {
seq_printf(m, "DC3 -> DC5 count: %d\n",
I915_READ(BXT_CSR_DC3_DC5_COUNT));
}
Please do the other way around...
If is broxton {
} else if gen(9,11) {
}
So no need for repetition with nots...
I was thinking of doing the same earlier, but then thought we should
maintain Platform hierarchy. Same trend is followed in other files as well.
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8534f88..573d5f3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6985,6 +6985,7 @@ enum {
/* MMIO address range for CSR program (0x8 - 0x82FFF) */
#define CSR_MMIO_START_RANGE0x8
#define CSR_MMIO_END_RANGE0x8
+/* DC3_DC5 count and DC5_DC6 count registers are same for SKL and ICL */
#define SKL_CSR_DC3_DC5_COUNT_MMIO(0x80030)
#define SKL_CSR_DC5_DC6_COUNT_MMIO(0x8002C)
#define BXT_CSR_DC3_DC5_COUNT_MMIO(0x80038)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
> On Oct 2, 2018, at 9:20 PM, Yadav, Jyoti R wrote:
>
> DC5 and DC6 counter register tells about residency of DC5 and DC6.
> These registers are same for SKL and ICL.
>
> v2 : Remove csr_version check.
> Added generic check regarding DC counters for Gen9 onwards. (Rodrigo)
> v3 : Simplified gen checks. (Chris)
>
> Signed-off-by: Jyoti Yadav
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 5 ++---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index a5265c2..af13077 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2897,13 +2897,12 @@ static int i915_dmc_info(struct seq_file *m, void
> *unused)
>seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
> CSR_VERSION_MINOR(csr->version));
>
> -if (IS_KABYLAKE(dev_priv) ||
> -(IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) {
> +if ((!IS_BROXTON(dev_priv)) && IS_GEN(dev_priv, 9, 11)) {
>seq_printf(m, "DC3 -> DC5 count: %d\n",
> I915_READ(SKL_CSR_DC3_DC5_COUNT));
>seq_printf(m, "DC5 -> DC6 count: %d\n",
> I915_READ(SKL_CSR_DC5_DC6_COUNT));
> -} else if (IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1, 4)) {
> +} else if (IS_BROXTON(dev_priv)) {
>seq_printf(m, "DC3 -> DC5 count: %d\n",
> I915_READ(BXT_CSR_DC3_DC5_COUNT));
>}
Please do the other way around...
If is broxton {
} else if gen(9,11) {
}
So no need for repetition with nots...
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8534f88..573d5f3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6985,6 +6985,7 @@ enum {
> /* MMIO address range for CSR program (0x8 - 0x82FFF) */
> #define CSR_MMIO_START_RANGE0x8
> #define CSR_MMIO_END_RANGE0x8
> +/* DC3_DC5 count and DC5_DC6 count registers are same for SKL and ICL */
> #define SKL_CSR_DC3_DC5_COUNT_MMIO(0x80030)
> #define SKL_CSR_DC5_DC6_COUNT_MMIO(0x8002C)
> #define BXT_CSR_DC3_DC5_COUNT_MMIO(0x80038)
> --
> 1.9.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
Quoting Jyoti Yadav (2018-10-02 05:42:27)
> DC5 and DC6 counter register tells about residency of DC5 and DC6.
> These registers are same for SKL and ICL.
>
> v2 : Remove csr_version check.
> Added generic check regarding DC counters for Gen9 onwards. (Rodrigo)
>
> Signed-off-by: Jyoti Yadav
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 7 ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index a5265c2..bcc1e86 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2880,11 +2880,13 @@ static int i915_dmc_info(struct seq_file *m, void
> *unused)
> {
> struct drm_i915_private *dev_priv = node_to_i915(m->private);
> struct intel_csr *csr;
> + int gen;
>
> if (!HAS_CSR(dev_priv))
> return -ENODEV;
>
> csr = &dev_priv->csr;
> + gen = INTEL_GEN(dev_priv);
>
> intel_runtime_pm_get(dev_priv);
>
> @@ -2897,13 +2899,12 @@ static int i915_dmc_info(struct seq_file *m, void
> *unused)
> seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
>CSR_VERSION_MINOR(csr->version));
>
> - if (IS_KABYLAKE(dev_priv) ||
> - (IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) {
> + if ((!IS_BROXTON(dev_priv)) && gen >= 9 && gen <= 11) {
IS_GEN(dev_priv, 9, 11)
Though fixing the if-ladder would be a lot simpler.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
Hi Jyoti,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.19-rc4 next-20180913]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Jyoti-Yadav/drm-i915-csr-Added-DC5-and-DC6-counter-register-for-ICL-in-debugfs-entry/20180918-012350
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x077-201837 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:10:0,
from include/linux/list.h:9,
from include/linux/wait.h:7,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from include/linux/debugfs.h:15,
from drivers/gpu//drm/i915/i915_debugfs.c:29:
drivers/gpu//drm/i915/i915_debugfs.c: In function 'i915_dmc_info':
drivers/gpu//drm/i915/i915_debugfs.c:2884:28: error: called object is not a
function or function pointer
(IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))
~~^
include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/gpu//drm/i915/i915_debugfs.c:2883:2: note: in expansion of macro 'if'
if (IS_KABYLAKE(dev_priv) ||
^~
drivers/gpu//drm/i915/i915_debugfs.c:2884:28: error: called object is not a
function or function pointer
(IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))
~~^
include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/gpu//drm/i915/i915_debugfs.c:2883:2: note: in expansion of macro 'if'
if (IS_KABYLAKE(dev_priv) ||
^~
drivers/gpu//drm/i915/i915_debugfs.c:2884:28: error: called object is not a
function or function pointer
(IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))
~~^
include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
__r = !!(cond); \
^~~~
>> drivers/gpu//drm/i915/i915_debugfs.c:2883:2: note: in expansion of macro 'if'
if (IS_KABYLAKE(dev_priv) ||
^~
vim +/if +2883 drivers/gpu//drm/i915/i915_debugfs.c
1da51581 Imre Deak2013-11-25 2861
b7cec66d Damien Lespiau 2015-10-27 2862 static int i915_dmc_info(struct
seq_file *m, void *unused)
b7cec66d Damien Lespiau 2015-10-27 2863 {
36cdd013 David Weinehall 2016-08-22 2864 struct drm_i915_private
*dev_priv = node_to_i915(m->private);
b7cec66d Damien Lespiau 2015-10-27 2865 struct intel_csr *csr;
b7cec66d Damien Lespiau 2015-10-27 2866
ab309a6a Michal Wajdeczko 2017-12-15 2867 if (!HAS_CSR(dev_priv))
ab309a6a Michal Wajdeczko 2017-12-15 2868 return -ENODEV;
b7cec66d Damien Lespiau 2015-10-27 2869
b7cec66d Damien Lespiau 2015-10-27 2870 csr = &dev_priv->csr;
b7cec66d Damien Lespiau 2015-10-27 2871
6fb403de Mika Kuoppala2015-10-30 2872 intel_runtime_pm_get(dev_priv);
6fb403de Mika Kuoppala2015-10-30 2873
b7cec66d Damien Lespiau 2015-10-27 2874 seq_printf(m, "fw loaded:
%s\n", yesno(csr->dmc_payload != NULL));
b7cec66d Damien Lespiau 2015-10-27 2875 seq_printf(m, "path: %s\n",
csr->fw_path);
b7cec66d Damien Lespiau 2015-10-27 2876
b7cec66d Damien Lespiau 2015-10-27 2877 if (!csr->dmc_payload)
6fb403de Mika Kuoppala2015-10-30 2878 goto out;
b7cec66d Damien Lespiau 2015-10-27 2879
b7cec66d Damien Lespiau 2015-10-27 2880 seq_printf(m, "version:
%d.%d\n", CSR_VERSION_MAJOR(csr->version),
b7cec66d Damien Lespiau 2015-10-27 2881
CSR_VERSION_MINOR(csr->version));
b7cec66d Damien Lespiau 2015-10-27 2882
48de568c Mika Kuoppala2017-05-09 @2883 if (IS_KABYLAKE(dev_priv) ||
71660c92 Jyoti Yadav 2018-09-17 2884 (IS_SKYLAKE(dev_priv) &&
csr->version >= CSR_VERSION(1, 6))
71660c92 Jyoti Yadav 2018-09-17 2885 (IS_ICELAKE(dev_priv)
&& csr->version >= CSR_VERSION(1, 7))) {
8337206d Damien Lespiau 2015-10-30 2886 seq_printf(m, "DC3 ->
DC5 count: %d\n",
8337206d Damien Lespiau 2015-10-30 2887
I915_READ(SKL_CSR_DC3_DC5_COUNT));
8337206d Damien Lespiau 2015-10-30 2888 seq_printf(m, "DC5 ->
DC6 count: %d\n",
8337206d Damien Lespiau 2015-10-30
Re: [Intel-gfx] [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
Hi Jyoti,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.19-rc4 next-20180913]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Jyoti-Yadav/drm-i915-csr-Added-DC5-and-DC6-counter-register-for-ICL-in-debugfs-entry/20180918-012350
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x005-201837 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/gpu//drm/i915/i915_debugfs.c: In function 'i915_dmc_info':
>> drivers/gpu//drm/i915/i915_debugfs.c:2884:28: error: called object is not a
>> function or function pointer
(IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))
~~^
vim +2884 drivers/gpu//drm/i915/i915_debugfs.c
2861
2862 static int i915_dmc_info(struct seq_file *m, void *unused)
2863 {
2864 struct drm_i915_private *dev_priv = node_to_i915(m->private);
2865 struct intel_csr *csr;
2866
2867 if (!HAS_CSR(dev_priv))
2868 return -ENODEV;
2869
2870 csr = &dev_priv->csr;
2871
2872 intel_runtime_pm_get(dev_priv);
2873
2874 seq_printf(m, "fw loaded: %s\n", yesno(csr->dmc_payload !=
NULL));
2875 seq_printf(m, "path: %s\n", csr->fw_path);
2876
2877 if (!csr->dmc_payload)
2878 goto out;
2879
2880 seq_printf(m, "version: %d.%d\n",
CSR_VERSION_MAJOR(csr->version),
2881 CSR_VERSION_MINOR(csr->version));
2882
2883 if (IS_KABYLAKE(dev_priv) ||
> 2884 (IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))
2885 (IS_ICELAKE(dev_priv) && csr->version >= CSR_VERSION(1,
7))) {
2886 seq_printf(m, "DC3 -> DC5 count: %d\n",
2887 I915_READ(SKL_CSR_DC3_DC5_COUNT));
2888 seq_printf(m, "DC5 -> DC6 count: %d\n",
2889 I915_READ(SKL_CSR_DC5_DC6_COUNT));
2890 } else if (IS_BROXTON(dev_priv) && csr->version >=
CSR_VERSION(1, 4)) {
2891 seq_printf(m, "DC3 -> DC5 count: %d\n",
2892 I915_READ(BXT_CSR_DC3_DC5_COUNT));
2893 }
2894
2895 out:
2896 seq_printf(m, "program base: 0x%08x\n",
I915_READ(CSR_PROGRAM(0)));
2897 seq_printf(m, "ssp base: 0x%08x\n", I915_READ(CSR_SSP_BASE));
2898 seq_printf(m, "htp: 0x%08x\n", I915_READ(CSR_HTP_SKL));
2899
2900 intel_runtime_pm_put(dev_priv);
2901
2902 return 0;
2903 }
2904
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] [intel-gfx] drm/i915/csr Added DC5 and DC6 counter register for ICL in debugfs entry.
On Mon, Sep 17, 2018 at 12:49:35PM -0400, Jyoti Yadav wrote:
> DC5 and DC6 counter register tells about residency of DC5 and DC6.
> These registers are same for SKL and ICL.
>
> Signed-off-by: Jyoti Yadav
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 3 ++-
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index a5265c2..328e39c 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2898,7 +2898,8 @@ static int i915_dmc_info(struct seq_file *m, void
> *unused)
> CSR_VERSION_MINOR(csr->version));
>
> if (IS_KABYLAKE(dev_priv) ||
> - (IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) {
> + (IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))
> + (IS_ICELAKE(dev_priv) && csr->version >= CSR_VERSION(1, 7))) {
Why on ICELAKE we need to be greater than version 1.7?
Specially because our only version starts on 1.7 I don't believe we should
be checking version here.
Also SKL check is useless nowadays and should be removed. It came from
the times we didn't have tied version of kernel and firmware...
Also I'm missing CFL, BXT, GLK and CNL on this range here...
Probably good to replace everything
if gen >= 9 && gen <= 11
?
> seq_printf(m, "DC3 -> DC5 count: %d\n",
> I915_READ(SKL_CSR_DC3_DC5_COUNT));
> seq_printf(m, "DC5 -> DC6 count: %d\n",
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8534f88..573d5f3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6985,6 +6985,7 @@ enum {
> /* MMIO address range for CSR program (0x8 - 0x82FFF) */
> #define CSR_MMIO_START_RANGE 0x8
> #define CSR_MMIO_END_RANGE 0x8
> +/* DC3_DC5 count and DC5_DC6 count registers are same for SKL and ICL */
> #define SKL_CSR_DC3_DC5_COUNT_MMIO(0x80030)
> #define SKL_CSR_DC5_DC6_COUNT_MMIO(0x8002C)
> #define BXT_CSR_DC3_DC5_COUNT_MMIO(0x80038)
> --
> 1.9.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
