Re: [Intel-gfx] [PATCH 21/23] drm/i915/dmc: MTL DMC debugfs entries
> -Original Message-
> From: Intel-gfx On Behalf Of Matt
> Roper
> Sent: Tuesday, August 2, 2022 11:23 AM
> To: Sripada, Radhakrishna
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 21/23] drm/i915/dmc: MTL DMC debugfs
> entries
>
> On Wed, Jul 27, 2022 at 06:34:18PM -0700, Radhakrishna Sripada wrote:
> > From: Anusha Srivatsa
> >
> > MTL needs both Pipe A and Pipe B DMC to be loaded along with Main DMC.
> > Patch also adds
>
> That's true, but it's unrelated to this patch. intel_dmc_load_program()
> always loads all of the pipe firmwares (including pipe C and pipe D) assuming
> it found them in the firmware file.
>
> > DMC debug register for MTL.
> >
> > BSpec: 49788
> > Cc: Matt Roper
> > Signed-off-by: Anusha Srivatsa
> > ---
> > drivers/gpu/drm/i915/display/intel_dmc.c | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> > b/drivers/gpu/drm/i915/display/intel_dmc.c
> > index 9c4f442fa407..2fabb2760474 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > @@ -1005,7 +1005,7 @@ static int intel_dmc_debugfs_status_show(struct
> seq_file *m, void *unused)
> > seq_printf(m, "Pipe A fw loaded: %s\n",
> >str_yes_no(dmc->dmc_info[DMC_FW_PIPEA].payload));
> > seq_printf(m, "Pipe B fw support: %s\n",
> > - str_yes_no(IS_ALDERLAKE_P(i915)));
> > + str_yes_no(DISPLAY_VER(i915) >= 13));
>
> What is this debugfs trying to tell us? Pipe DMC fw for all four pipes has
> been supported since TGL. So the output here is misleading (and incomplete
> since it doesn't include C/D).
>
> The thing that changed in DG2 was that we were required to upload the pipe
> A firmware along with the main firmware (other pipes optional).
> The thing that further changed in ADL-P was that we were required to upload
> *both* pipe A and pipe B along with the main firmware (other two pipes still
> optional).
>
> Even if the output here was trying to indicate which pipe firmware(s) need to
> be uploaded at the same time as the main firmware (rather than being
> uploaded later), the change here wouldn't be correct since as noted above,
> DG2 (which has display version 13) only required pipe A and not B.
>
> I think we probably need to decide what the purpose of this debugfs is
> supposed to be and then rework it accordingly.
>
IMO the debugfs should give a gist of the different firmwares that was loaded
and which ones the platform actually needs. At this point the driver loads all
available firmwares even if the platform doesn't need them. Debugfs should
clearly state wat Is needed and if that is loaded or not.
Something like:
Pipe A loaded: yes/no
Pipe B loaded: yes/no
And so on.
As well as:
Pipe A needed: yes/no
Pipe B needed: yes/no
Does this sound like the right way forward?
Anusha
> Matt
>
> > seq_printf(m, "Pipe B fw loaded: %s\n",
> >str_yes_no(dmc->dmc_info[DMC_FW_PIPEB].payload));
> >
> > @@ -1029,9 +1029,9 @@ static int intel_dmc_debugfs_status_show(struct
> seq_file *m, void *unused)
> > * reg for DC3CO debugging and validation,
> > * but TGL DMC f/w is using DMC_DEBUG3 reg for DC3CO
> counter.
> > */
> > - seq_printf(m, "DC3CO count: %d\n",
> > - intel_de_read(i915, IS_DGFX(i915) ?
> > -DG1_DMC_DEBUG3 :
> TGL_DMC_DEBUG3));
> > + seq_printf(m, "DC3CO count: %d\n", intel_de_read(i915,
> > + (IS_DGFX(i915) || DISPLAY_VER(i915) >= 14) ?
> > + DG1_DMC_DEBUG3 : TGL_DMC_DEBUG3));
> > } else {
> > dc5_reg = IS_BROXTON(i915) ? BXT_DMC_DC3_DC5_COUNT :
> > SKL_DMC_DC3_DC5_COUNT;
> > --
> > 2.25.1
> >
>
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
Re: [Intel-gfx] [PATCH 21/23] drm/i915/dmc: MTL DMC debugfs entries
On Wed, Jul 27, 2022 at 06:34:18PM -0700, Radhakrishna Sripada wrote:
> From: Anusha Srivatsa
>
> MTL needs both Pipe A and Pipe B DMC to be loaded
> along with Main DMC. Patch also adds
That's true, but it's unrelated to this patch. intel_dmc_load_program()
always loads all of the pipe firmwares (including pipe C and pipe D)
assuming it found them in the firmware file.
> DMC debug register for MTL.
>
> BSpec: 49788
> Cc: Matt Roper
> Signed-off-by: Anusha Srivatsa
> ---
> drivers/gpu/drm/i915/display/intel_dmc.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> b/drivers/gpu/drm/i915/display/intel_dmc.c
> index 9c4f442fa407..2fabb2760474 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> @@ -1005,7 +1005,7 @@ static int intel_dmc_debugfs_status_show(struct
> seq_file *m, void *unused)
> seq_printf(m, "Pipe A fw loaded: %s\n",
> str_yes_no(dmc->dmc_info[DMC_FW_PIPEA].payload));
> seq_printf(m, "Pipe B fw support: %s\n",
> -str_yes_no(IS_ALDERLAKE_P(i915)));
> +str_yes_no(DISPLAY_VER(i915) >= 13));
What is this debugfs trying to tell us? Pipe DMC fw for all four pipes
has been supported since TGL. So the output here is misleading (and
incomplete since it doesn't include C/D).
The thing that changed in DG2 was that we were required to upload the
pipe A firmware along with the main firmware (other pipes optional).
The thing that further changed in ADL-P was that we were required to
upload *both* pipe A and pipe B along with the main firmware (other two
pipes still optional).
Even if the output here was trying to indicate which pipe firmware(s)
need to be uploaded at the same time as the main firmware (rather than
being uploaded later), the change here wouldn't be correct since as
noted above, DG2 (which has display version 13) only required pipe A and
not B.
I think we probably need to decide what the purpose of this debugfs is
supposed to be and then rework it accordingly.
Matt
> seq_printf(m, "Pipe B fw loaded: %s\n",
> str_yes_no(dmc->dmc_info[DMC_FW_PIPEB].payload));
>
> @@ -1029,9 +1029,9 @@ static int intel_dmc_debugfs_status_show(struct
> seq_file *m, void *unused)
>* reg for DC3CO debugging and validation,
>* but TGL DMC f/w is using DMC_DEBUG3 reg for DC3CO counter.
>*/
> - seq_printf(m, "DC3CO count: %d\n",
> -intel_de_read(i915, IS_DGFX(i915) ?
> - DG1_DMC_DEBUG3 : TGL_DMC_DEBUG3));
> + seq_printf(m, "DC3CO count: %d\n", intel_de_read(i915,
> +(IS_DGFX(i915) || DISPLAY_VER(i915) >= 14) ?
> + DG1_DMC_DEBUG3 : TGL_DMC_DEBUG3));
> } else {
> dc5_reg = IS_BROXTON(i915) ? BXT_DMC_DC3_DC5_COUNT :
> SKL_DMC_DC3_DC5_COUNT;
> --
> 2.25.1
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
[Intel-gfx] [PATCH 21/23] drm/i915/dmc: MTL DMC debugfs entries
From: Anusha Srivatsa
MTL needs both Pipe A and Pipe B DMC to be loaded
along with Main DMC. Patch also adds
DMC debug register for MTL.
BSpec: 49788
Cc: Matt Roper
Signed-off-by: Anusha Srivatsa
---
drivers/gpu/drm/i915/display/intel_dmc.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
b/drivers/gpu/drm/i915/display/intel_dmc.c
index 9c4f442fa407..2fabb2760474 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -1005,7 +1005,7 @@ static int intel_dmc_debugfs_status_show(struct seq_file
*m, void *unused)
seq_printf(m, "Pipe A fw loaded: %s\n",
str_yes_no(dmc->dmc_info[DMC_FW_PIPEA].payload));
seq_printf(m, "Pipe B fw support: %s\n",
- str_yes_no(IS_ALDERLAKE_P(i915)));
+ str_yes_no(DISPLAY_VER(i915) >= 13));
seq_printf(m, "Pipe B fw loaded: %s\n",
str_yes_no(dmc->dmc_info[DMC_FW_PIPEB].payload));
@@ -1029,9 +1029,9 @@ static int intel_dmc_debugfs_status_show(struct seq_file
*m, void *unused)
* reg for DC3CO debugging and validation,
* but TGL DMC f/w is using DMC_DEBUG3 reg for DC3CO counter.
*/
- seq_printf(m, "DC3CO count: %d\n",
- intel_de_read(i915, IS_DGFX(i915) ?
-DG1_DMC_DEBUG3 : TGL_DMC_DEBUG3));
+ seq_printf(m, "DC3CO count: %d\n", intel_de_read(i915,
+ (IS_DGFX(i915) || DISPLAY_VER(i915) >= 14) ?
+ DG1_DMC_DEBUG3 : TGL_DMC_DEBUG3));
} else {
dc5_reg = IS_BROXTON(i915) ? BXT_DMC_DC3_DC5_COUNT :
SKL_DMC_DC3_DC5_COUNT;
--
2.25.1
