Re: [Intel-gfx] [PATCH 6/6] drm/i915/psr: Define more PSR mask bits

[Imre Deak <imre.deak@intel.com>]

On Mon, Mar 20, 2023 at 11:54:38AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Define more of the PSR mask bits. Even if we don't set them
> from the driver they can be very useful during PSR debugging.
> Having to trawl through bspec every time to find them is
> not fun.
> 
> The particularly interesting bits are:
> 
> - PIPE_MISC[21]/PIPE_MISC_PSR_MASK_PIPE_REG_WRITE
> 
>   Normally most, if not all, pipe/plane registers (even the
>   noarm ones) trigger a PSR exit. This stops that, leaving
>   only PLANE_SURF (or so it seems) to do the triggering
> 
> - CHICKEN_TRANS[30]/NO_VBLANK_MASK_IN_DEEP_PSR
> 
>   Stops PSR exit from generating an extra vblank before the
>   first frame is transmitted.
> 
>   Looks like with DC states enabled the extra vblank happens
>   after link traning, with DC states disabled it seems to happen
>   immediately. No idea as of now why there is a difference.
> 
>   Unfortunately CHICKEN_TRANS itself seems to be double buffered
>   and thus won't latch until the first vblank. So with DC states
>   enabled the register effctively uses the reset value during DC5
>   exit+PSR exit sequence, and thus the bit does nothing until
>   latched by the vblank that it was trying to prevent from being
>   generated in the first place. So we should probably call this one
>   a chicken/egg bit instead.
> 
>   TODO: should confirm what this does when using PSR standby
>         mode instead of deep/link-off mode...
> 
> Cc: Manasi Navare <navaremanasi@google.com>
> Cc: Drew Davenport <ddavenport@chromium.org>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d22ffd7a32dc..383c532320f9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2205,6 +2205,7 @@
>  #define   EDP_PSR_DEBUG_MASK_LPSP              (1 << 27)
>  #define   EDP_PSR_DEBUG_MASK_MEMUP             (1 << 26)
>  #define   EDP_PSR_DEBUG_MASK_HPD               (1 << 25)
> +#define   EDP_PSR_DEBUG_MASK_FBC_MODIFY        (1 << 24)
>  #define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16) /* Reserved in ICL+ */
>  #define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
>  
> @@ -3500,8 +3501,13 @@
>  #define   PIPE_MISC_YUV420_ENABLE		REG_BIT(27) /* glk+ */
>  #define   PIPE_MISC_YUV420_MODE_FULL_BLEND	REG_BIT(26) /* glk+ */
>  #define   PIPE_MISC_HDR_MODE_PRECISION		REG_BIT(23) /* icl+ */

'Allow Double Buffer Update Disable' BIT(24) on tgl+ looks also related
to these flags.

The patchset looks ok to me:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> +#define   PIPE_MISC_PSR_MASK_PRIMARY_FLIP	REG_BIT(23) /* bdw only */
> +#define   PIPE_MISC_PSR_MASK_SPRITE_ENABLE	REG_BIT(22) /* bdw only */
> +#define   PIPE_MISC_PSR_MASK_PIPE_REG_WRITE	REG_BIT(21) /* skl+ */
> +#define   PIPE_MISC_PSR_MASK_CURSOR_MOVE	REG_BIT(21) /* bdw only */
> +#define   PIPE_MISC_PSR_MASK_VBLANK_VSYNC_INT	REG_BIT(20)
>  #define   PIPE_MISC_OUTPUT_COLORSPACE_YUV	REG_BIT(11)
> -#define   PIPE_MISC_PIXEL_ROUNDING_TRUNC		REG_BIT(8) /* tgl+ */
> +#define   PIPE_MISC_PIXEL_ROUNDING_TRUNC	REG_BIT(8) /* tgl+ */
>  /*
>   * For Display < 13, Bits 5-7 of PIPE MISC represent DITHER BPC with
>   * valid values of: 6, 8, 10 BPC.
> @@ -5550,7 +5556,7 @@
>  #define MTL_CHICKEN_TRANS(trans)	_MMIO_TRANS((trans), \
>  						    _MTL_CHICKEN_TRANS_A, \
>  						    _MTL_CHICKEN_TRANS_B)
> -
> +#define  NO_VBLANK_MASK_IN_DEEP_PSR	REG_BIT(30) /* skl+ */
>  #define  HSW_FRAME_START_DELAY_MASK	REG_GENMASK(28, 27)
>  #define  HSW_FRAME_START_DELAY(x)	REG_FIELD_PREP(HSW_FRAME_START_DELAY_MASK, x)
>  #define  VSC_DATA_SEL_SOFTWARE_CONTROL	REG_BIT(25) /* GLK */
> -- 
> 2.39.2
>