Re: [igt-dev] [PATCH i-g-t 4/6] tools/intel_watermark: Show whether each plane is enabled

Souza, Jose
2019-01-28T22:53:35Z

[-- Attachment #1.1: Type: text/plain, Size: 3523 bytes --]

On Mon, 2019-01-28 at 22:01 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> To make it easier to spot errors with watermarks vs. plane
> being enabled/disabled indicate which planes are actually
> enabled and which are not.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> tools/intel_watermark.c | 40 ++++++++++++++++++++++++++++++++++++++-
> -
> 1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/tools/intel_watermark.c b/tools/intel_watermark.c
> index 97b769250927..8405d6489628 100644
> --- a/tools/intel_watermark.c
> +++ b/tools/intel_watermark.c
> @@ -180,6 +180,20 @@ static const char *skl_wm_linetime_reg_name(int
> pipe)
> return reg_name;
> }
>
> +static const char *skl_plane_ctl_reg_name(int pipe, int plane)
> +{
> + static char reg_name[32];
> +
> + if (plane == 0)
> + snprintf(reg_name, sizeof(reg_name), "CUR_CTL_%c",
> + pipe_name(pipe));
> + else
> + snprintf(reg_name, sizeof(reg_name),
> "PLANE_CTL_%1d_%c",
> + plane, pipe_name(pipe));
> +
> + return reg_name;
> +}
> +
> static const char *skl_wm_reg_name(int pipe, int plane, int level)
> {
> static char reg_name[32];
> @@ -231,6 +245,7 @@ static void skl_wm_dump(void)
> uint32_t wm[num_levels][num_pipes][max_planes];
> uint32_t wm_trans[num_pipes][max_planes];
> uint32_t buf_cfg[num_pipes][max_planes];
> + uint32_t plane_ctl[num_pipes][max_planes];
> uint32_t wm_linetime[num_pipes];
>
> intel_register_access_init(intel_get_pci_device(), 0, -1);
> @@ -243,6 +258,7 @@ static void skl_wm_dump(void)
> for (plane = 0; plane < num_planes; plane++) {
> addr = base_addr + pipe * 0x1000 + plane *
> 0x100;
>
> + plane_ctl[pipe][plane] = read_reg(addr + 0x80);
> wm_trans[pipe][plane] = read_reg(addr +
> 0x00168);
> buf_cfg[pipe][plane] = read_reg(addr +
> 0x0017C);
> for (level = 0; level < num_levels; level++) {
> @@ -259,6 +275,19 @@ static void skl_wm_dump(void)
> }
> printf("\n\n");
>
> + for (plane = 0; plane < max_planes; plane++) {
> + for (pipe = 0; pipe < num_pipes; pipe++) {
> + if (plane >= skl_num_planes(devid, pipe))
> + break;
> +
> + printf("%18s 0x%08x\t" ,
> + skl_plane_ctl_reg_name(pipe, plane),
> + plane_ctl[pipe][plane]);
> + }
> + printf("\n");
> + }
> + printf("\n");
> +
> for (plane = 0; plane < max_planes; plane++) {
> for (level = 0; level < num_levels; level++) {
> for (pipe = 0; pipe < num_pipes; pipe++) {
> @@ -312,8 +341,15 @@ static void skl_wm_dump(void)
> printf("LINETIME: %d (%.3f usec)\n", linetime,
> linetime* 0.125f);
>
> printf("LEVEL");
> - for (plane = 0; plane < num_planes; plane++)
> - printf("%10s", skl_plane_name(pipe, plane));
> + for (plane = 0; plane < num_planes; plane++) {
> + if (plane == 0)
> + enable =
> REG_DECODE1(plane_ctl[pipe][plane], 0, 3) ||
> + REG_DECODE1(plane_ctl[pipe][pla
> ne], 5, 1);

I guess we only enabled ARGB and AND/XOR cursors?! But why not keep it
simple and check for REG_DECODE1(plane_ctl[pipe][plane], 0, 5)?

Other than that LGTM:

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> + else
> + enable =
> REG_DECODE1(plane_ctl[pipe][plane], 31, 1);
> + printf("%9s%c", skl_plane_name(pipe, plane),
> + endis_ast(enable));
> + }
> printf("\n");
>
> for (level = 0; level < num_levels; level++) {

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]