diff mbox series

[6/6] drm/i915/display/adl_p: Implement PSR changes

Message ID 20210616203158.118111-6-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/6] drm/i915/display/psr: Handle SU Y granularity | expand

Commit Message

Souza, Jose June 16, 2021, 8:31 p.m. UTC 
Implements changes around PSR for alderlake-P:

- EDP_SU_TRACK_ENABLE was removed and bit 30 now has other function
- Some bits of PSR2_MAN_TRK_CTL moved and SF_PARTIAL_FRAME_UPDATE was
  removed setting SU_REGION_START/END_ADDR will do this job
- SU_REGION_START/END_ADDR have now line granularity but will need to
  be aligned with DSC when the PSRS + DSC support lands

BSpec: 50422
BSpec: 50424
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 43 ++++++++++++++++++------
 drivers/gpu/drm/i915/i915_reg.h          | 26 ++++++++------
 2 files changed, 48 insertions(+), 21 deletions(-)

Comments

Gwan-gyeong Mun June 23, 2021, 7:06 p.m. UTC | #1 
On 6/16/21 11:31 PM, José Roberto de Souza wrote:
> Implements changes around PSR for alderlake-P:
> 
> - EDP_SU_TRACK_ENABLE was removed and bit 30 now has other function
> - Some bits of PSR2_MAN_TRK_CTL moved and SF_PARTIAL_FRAME_UPDATE was
>    removed setting SU_REGION_START/END_ADDR will do this job
> - SU_REGION_START/END_ADDR have now line granularity but will need to
>    be aligned with DSC when the PSRS + DSC support lands
> 
> BSpec: 50422
> BSpec: 50424
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_psr.c | 43 ++++++++++++++++++------
>   drivers/gpu/drm/i915/i915_reg.h          | 26 ++++++++------
>   2 files changed, 48 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 9643624fe160d..46bb19c4b63a4 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -534,11 +534,13 @@ static u32 intel_psr2_get_tp_time(struct intel_dp *intel_dp)
>   static void hsw_activate_psr2(struct intel_dp *intel_dp)
>   {
>   	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	u32 val;
> +	u32 val = EDP_PSR2_ENABLE;
> +
> +	val |= psr_compute_idle_frames(intel_dp) << EDP_PSR2_IDLE_FRAME_SHIFT;
>   
> -	val = psr_compute_idle_frames(intel_dp) << EDP_PSR2_IDLE_FRAME_SHIFT;
> +	if (!IS_ALDERLAKE_P(dev_priv))
> +		val |= EDP_SU_TRACK_ENABLE;
>   
> -	val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
>   	if (DISPLAY_VER(dev_priv) >= 10 && DISPLAY_VER(dev_priv) <= 12)
>   		val |= EDP_Y_COORDINATE_ENABLE;
>   
> @@ -793,6 +795,7 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
>   static bool psr2_granularity_check(struct intel_dp *intel_dp,
>   				   struct intel_crtc_state *crtc_state)
>   {
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>   	const int crtc_hdisplay = crtc_state->hw.adjusted_mode.crtc_hdisplay;
>   	const int crtc_vdisplay = crtc_state->hw.adjusted_mode.crtc_vdisplay;
>   	u16 y_granularity = 0;
> @@ -809,10 +812,13 @@ static bool psr2_granularity_check(struct intel_dp *intel_dp,
>   		return intel_dp->psr.su_y_granularity == 4;
>   
>   	/*
> -	 * For SW tracking we can adjust the y to match sink requirement if
> -	 * multiple of 4
> +	 * adl_p has 1 line granularity for other platforms with SW tracking we
> +	 * can adjust the y coordinate to match sink requirement if multiple of
> +	 * 4
>   	 */
> -	if (intel_dp->psr.su_y_granularity <= 2)
> +	if (IS_ALDERLAKE_P(dev_priv))
> +		y_granularity = intel_dp->psr.su_y_granularity;
> +	else if (intel_dp->psr.su_y_granularity <= 2)
>   		y_granularity = 4;
>   	else if ((intel_dp->psr.su_y_granularity % 4) == 0)
>   		y_granularity = intel_dp->psr.su_y_granularity;
> @@ -1525,21 +1531,32 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
>   static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
>   				  struct drm_rect *clip, bool full_update)
>   {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>   	u32 val = PSR2_MAN_TRK_CTL_ENABLE;
The logic is not wrong, but the meaning of the register bit has changed.
The 31st bit in ADL-P means "SF partial frame enable".
It is recommended to add a macro such as 
ADLP_PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_ENABLE to the code to clarify the 
role of the changed register.
>   
>   	if (full_update) {
> -		val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> +		if (IS_ALDERLAKE_P(dev_priv))
> +			val |= ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> +		else
> +			val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> +
>   		goto exit;
>   	}
>   
>   	if (clip->y1 == -1)
>   		goto exit;
>   
> -	drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip->y2 % 4);
> +	if (IS_ALDERLAKE_P(dev_priv)) {
> +		val |= ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1);
> +		val |= ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2);
> +	} else {
> +		drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip->y2 % 4);
>   
> -	val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> -	val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> -	val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1);
> +		val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> +		val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> +		val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1);
> +	}
>   exit:
>   	crtc_state->psr2_man_track_ctl = val;
>   }
> @@ -1563,11 +1580,15 @@ static void clip_area_update(struct drm_rect *overlap_damage_area,
>   static void intel_psr2_sel_fetch_pipe_alignment(const struct intel_crtc_state *crtc_state,
>   						struct drm_rect *pipe_clip)
>   {
> +	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
>   	const u16 y_alignment = crtc_state->su_y_granularity;
>   
>   	pipe_clip->y1 -= pipe_clip->y1 % y_alignment;
>   	if (pipe_clip->y2 % y_alignment)
>   		pipe_clip->y2 = ((pipe_clip->y2 / y_alignment) + 1) * y_alignment;
> +
> +	if (IS_ALDERLAKE_P(dev_priv) && crtc_state->dsc.compression_enable)
> +		drm_warn(&dev_priv->drm, "Missing PSR2 sel fetch alignment with DSC\n");
>   }
>   
>   int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e0bd60fe7a190..74dc5ebce60e7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4586,7 +4586,7 @@ enum {
>   #define _PSR2_CTL_EDP				0x6f900
>   #define EDP_PSR2_CTL(tran)			_MMIO_TRANS2(tran, _PSR2_CTL_A)
>   #define   EDP_PSR2_ENABLE			(1 << 31)
> -#define   EDP_SU_TRACK_ENABLE			(1 << 30)
> +#define   EDP_SU_TRACK_ENABLE			(1 << 30) /* up to adl-p */
>   #define   TGL_EDP_PSR2_BLOCK_COUNT_NUM_2	(0 << 28)
>   #define   TGL_EDP_PSR2_BLOCK_COUNT_NUM_3	(1 << 28)
>   #define   EDP_Y_COORDINATE_ENABLE		REG_BIT(25) /* display 10, 11 and 12 */
> @@ -4655,17 +4655,23 @@ enum {
>   #define PSR2_SU_STATUS_MASK(frame)	(0x3ff << PSR2_SU_STATUS_SHIFT(frame))
>   #define PSR2_SU_STATUS_FRAMES		8
>   
> -#define _PSR2_MAN_TRK_CTL_A				0x60910
> -#define _PSR2_MAN_TRK_CTL_EDP				0x6f910
> -#define PSR2_MAN_TRK_CTL(tran)				_MMIO_TRANS2(tran, _PSR2_MAN_TRK_CTL_A)
> -#define  PSR2_MAN_TRK_CTL_ENABLE			REG_BIT(31)
> -#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK	REG_GENMASK(30, 21)
> -#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)	REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
> +#define _PSR2_MAN_TRK_CTL_A					0x60910
> +#define _PSR2_MAN_TRK_CTL_EDP					0x6f910
> +#define PSR2_MAN_TRK_CTL(tran)					_MMIO_TRANS2(tran, _PSR2_MAN_TRK_CTL_A)
> +#define  PSR2_MAN_TRK_CTL_ENABLE				REG_BIT(31)
> +#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK		REG_GENMASK(30, 21)
> +#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)		REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
>   #define  PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK		REG_GENMASK(20, 11)
>   #define  PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(val)		REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK, val)
> -#define  PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME		REG_BIT(3)
> -#define  PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME	REG_BIT(2)
> -#define  PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE	REG_BIT(1)
> +#define  PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME			REG_BIT(3)
> +#define  PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME		REG_BIT(2)
> +#define  PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE		REG_BIT(1)
> +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK	REG_GENMASK(28, 16)
> +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)	REG_FIELD_PREP(ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
> +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK		REG_GENMASK(12, 0)
> +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(val)		REG_FIELD_PREP(ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK, val)
> +#define  ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME		REG_BIT(14)
> +#define  ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME		REG_BIT(13)
>   
>   /* Icelake DSC Rate Control Range Parameter Registers */
>   #define DSCA_RC_RANGE_PARAMETERS_0		_MMIO(0x6B240)
>
Souza, Jose June 24, 2021, 4:19 p.m. UTC | #2 
On Wed, 2021-06-23 at 22:06 +0300, Gwan-gyeong Mun wrote:
> On 6/16/21 11:31 PM, José Roberto de Souza wrote:
> > Implements changes around PSR for alderlake-P:
> > 
> > - EDP_SU_TRACK_ENABLE was removed and bit 30 now has other function
> > - Some bits of PSR2_MAN_TRK_CTL moved and SF_PARTIAL_FRAME_UPDATE was
> >    removed setting SU_REGION_START/END_ADDR will do this job
> > - SU_REGION_START/END_ADDR have now line granularity but will need to
> >    be aligned with DSC when the PSRS + DSC support lands
> > 
> > BSpec: 50422
> > BSpec: 50424
> > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > ---
> >   drivers/gpu/drm/i915/display/intel_psr.c | 43 ++++++++++++++++++------
> >   drivers/gpu/drm/i915/i915_reg.h          | 26 ++++++++------
> >   2 files changed, 48 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 9643624fe160d..46bb19c4b63a4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -534,11 +534,13 @@ static u32 intel_psr2_get_tp_time(struct intel_dp *intel_dp)
> >   static void hsw_activate_psr2(struct intel_dp *intel_dp)
> >   {
> >   	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > -	u32 val;
> > +	u32 val = EDP_PSR2_ENABLE;
> > +
> > +	val |= psr_compute_idle_frames(intel_dp) << EDP_PSR2_IDLE_FRAME_SHIFT;
> >   
> > -	val = psr_compute_idle_frames(intel_dp) << EDP_PSR2_IDLE_FRAME_SHIFT;
> > +	if (!IS_ALDERLAKE_P(dev_priv))
> > +		val |= EDP_SU_TRACK_ENABLE;
> >   
> > -	val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> >   	if (DISPLAY_VER(dev_priv) >= 10 && DISPLAY_VER(dev_priv) <= 12)
> >   		val |= EDP_Y_COORDINATE_ENABLE;
> >   
> > @@ -793,6 +795,7 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> >   static bool psr2_granularity_check(struct intel_dp *intel_dp,
> >   				   struct intel_crtc_state *crtc_state)
> >   {
> > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >   	const int crtc_hdisplay = crtc_state->hw.adjusted_mode.crtc_hdisplay;
> >   	const int crtc_vdisplay = crtc_state->hw.adjusted_mode.crtc_vdisplay;
> >   	u16 y_granularity = 0;
> > @@ -809,10 +812,13 @@ static bool psr2_granularity_check(struct intel_dp *intel_dp,
> >   		return intel_dp->psr.su_y_granularity == 4;
> >   
> >   	/*
> > -	 * For SW tracking we can adjust the y to match sink requirement if
> > -	 * multiple of 4
> > +	 * adl_p has 1 line granularity for other platforms with SW tracking we
> > +	 * can adjust the y coordinate to match sink requirement if multiple of
> > +	 * 4
> >   	 */
> > -	if (intel_dp->psr.su_y_granularity <= 2)
> > +	if (IS_ALDERLAKE_P(dev_priv))
> > +		y_granularity = intel_dp->psr.su_y_granularity;
> > +	else if (intel_dp->psr.su_y_granularity <= 2)
> >   		y_granularity = 4;
> >   	else if ((intel_dp->psr.su_y_granularity % 4) == 0)
> >   		y_granularity = intel_dp->psr.su_y_granularity;
> > @@ -1525,21 +1531,32 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
> >   static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
> >   				  struct drm_rect *clip, bool full_update)
> >   {
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >   	u32 val = PSR2_MAN_TRK_CTL_ENABLE;
> The logic is not wrong, but the meaning of the register bit has changed.
> The 31st bit in ADL-P means "SF partial frame enable".
> It is recommended to add a macro such as 
> ADLP_PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_ENABLE to the code to clarify the 
> role of the changed register.

In my opinion the meaning is the same, enable manual/software tracking.
It was just a register rename done as part of changes of the other bits. 

But if you really think is necessary I can do that, please let me know.

> >   
> >   	if (full_update) {
> > -		val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > +		if (IS_ALDERLAKE_P(dev_priv))
> > +			val |= ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > +		else
> > +			val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > +
> >   		goto exit;
> >   	}
> >   
> >   	if (clip->y1 == -1)
> >   		goto exit;
> >   
> > -	drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip->y2 % 4);
> > +	if (IS_ALDERLAKE_P(dev_priv)) {
> > +		val |= ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1);
> > +		val |= ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2);
> > +	} else {
> > +		drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip->y2 % 4);
> >   
> > -	val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> > -	val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> > -	val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1);
> > +		val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> > +		val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> > +		val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1);
> > +	}
> >   exit:
> >   	crtc_state->psr2_man_track_ctl = val;
> >   }
> > @@ -1563,11 +1580,15 @@ static void clip_area_update(struct drm_rect *overlap_damage_area,
> >   static void intel_psr2_sel_fetch_pipe_alignment(const struct intel_crtc_state *crtc_state,
> >   						struct drm_rect *pipe_clip)
> >   {
> > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> >   	const u16 y_alignment = crtc_state->su_y_granularity;
> >   
> >   	pipe_clip->y1 -= pipe_clip->y1 % y_alignment;
> >   	if (pipe_clip->y2 % y_alignment)
> >   		pipe_clip->y2 = ((pipe_clip->y2 / y_alignment) + 1) * y_alignment;
> > +
> > +	if (IS_ALDERLAKE_P(dev_priv) && crtc_state->dsc.compression_enable)
> > +		drm_warn(&dev_priv->drm, "Missing PSR2 sel fetch alignment with DSC\n");
> >   }
> >   
> >   int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index e0bd60fe7a190..74dc5ebce60e7 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4586,7 +4586,7 @@ enum {
> >   #define _PSR2_CTL_EDP				0x6f900
> >   #define EDP_PSR2_CTL(tran)			_MMIO_TRANS2(tran, _PSR2_CTL_A)
> >   #define   EDP_PSR2_ENABLE			(1 << 31)
> > -#define   EDP_SU_TRACK_ENABLE			(1 << 30)
> > +#define   EDP_SU_TRACK_ENABLE			(1 << 30) /* up to adl-p */
> >   #define   TGL_EDP_PSR2_BLOCK_COUNT_NUM_2	(0 << 28)
> >   #define   TGL_EDP_PSR2_BLOCK_COUNT_NUM_3	(1 << 28)
> >   #define   EDP_Y_COORDINATE_ENABLE		REG_BIT(25) /* display 10, 11 and 12 */
> > @@ -4655,17 +4655,23 @@ enum {
> >   #define PSR2_SU_STATUS_MASK(frame)	(0x3ff << PSR2_SU_STATUS_SHIFT(frame))
> >   #define PSR2_SU_STATUS_FRAMES		8
> >   
> > -#define _PSR2_MAN_TRK_CTL_A				0x60910
> > -#define _PSR2_MAN_TRK_CTL_EDP				0x6f910
> > -#define PSR2_MAN_TRK_CTL(tran)				_MMIO_TRANS2(tran, _PSR2_MAN_TRK_CTL_A)
> > -#define  PSR2_MAN_TRK_CTL_ENABLE			REG_BIT(31)
> > -#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK	REG_GENMASK(30, 21)
> > -#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)	REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
> > +#define _PSR2_MAN_TRK_CTL_A					0x60910
> > +#define _PSR2_MAN_TRK_CTL_EDP					0x6f910
> > +#define PSR2_MAN_TRK_CTL(tran)					_MMIO_TRANS2(tran, _PSR2_MAN_TRK_CTL_A)
> > +#define  PSR2_MAN_TRK_CTL_ENABLE				REG_BIT(31)
> > +#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK		REG_GENMASK(30, 21)
> > +#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)		REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
> >   #define  PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK		REG_GENMASK(20, 11)
> >   #define  PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(val)		REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK, val)
> > -#define  PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME		REG_BIT(3)
> > -#define  PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME	REG_BIT(2)
> > -#define  PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE	REG_BIT(1)
> > +#define  PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME			REG_BIT(3)
> > +#define  PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME		REG_BIT(2)
> > +#define  PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE		REG_BIT(1)
> > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK	REG_GENMASK(28, 16)
> > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)	REG_FIELD_PREP(ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
> > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK		REG_GENMASK(12, 0)
> > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(val)		REG_FIELD_PREP(ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK, val)
> > +#define  ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME		REG_BIT(14)
> > +#define  ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME		REG_BIT(13)
> >   
> >   /* Icelake DSC Rate Control Range Parameter Registers */
> >   #define DSCA_RC_RANGE_PARAMETERS_0		_MMIO(0x6B240)
> >
Souza, Jose June 24, 2021, 4:19 p.m. UTC | #3 
On Thu, 2021-06-24 at 09:22 -0700, José Roberto de Souza wrote:
> On Wed, 2021-06-23 at 22:06 +0300, Gwan-gyeong Mun wrote:
> > On 6/16/21 11:31 PM, José Roberto de Souza wrote:
> > > Implements changes around PSR for alderlake-P:
> > > 
> > > - EDP_SU_TRACK_ENABLE was removed and bit 30 now has other function
> > > - Some bits of PSR2_MAN_TRK_CTL moved and SF_PARTIAL_FRAME_UPDATE was
> > >    removed setting SU_REGION_START/END_ADDR will do this job
> > > - SU_REGION_START/END_ADDR have now line granularity but will need to
> > >    be aligned with DSC when the PSRS + DSC support lands
> > > 
> > > BSpec: 50422
> > > BSpec: 50424
> > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_psr.c | 43 ++++++++++++++++++------
> > >   drivers/gpu/drm/i915/i915_reg.h          | 26 ++++++++------
> > >   2 files changed, 48 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 9643624fe160d..46bb19c4b63a4 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -534,11 +534,13 @@ static u32 intel_psr2_get_tp_time(struct intel_dp *intel_dp)
> > >   static void hsw_activate_psr2(struct intel_dp *intel_dp)
> > >   {
> > >   	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > -	u32 val;
> > > +	u32 val = EDP_PSR2_ENABLE;
> > > +
> > > +	val |= psr_compute_idle_frames(intel_dp) << EDP_PSR2_IDLE_FRAME_SHIFT;
> > >   
> > > -	val = psr_compute_idle_frames(intel_dp) << EDP_PSR2_IDLE_FRAME_SHIFT;
> > > +	if (!IS_ALDERLAKE_P(dev_priv))
> > > +		val |= EDP_SU_TRACK_ENABLE;
> > >   
> > > -	val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> > >   	if (DISPLAY_VER(dev_priv) >= 10 && DISPLAY_VER(dev_priv) <= 12)
> > >   		val |= EDP_Y_COORDINATE_ENABLE;
> > >   
> > > @@ -793,6 +795,7 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> > >   static bool psr2_granularity_check(struct intel_dp *intel_dp,
> > >   				   struct intel_crtc_state *crtc_state)
> > >   {
> > > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > >   	const int crtc_hdisplay = crtc_state->hw.adjusted_mode.crtc_hdisplay;
> > >   	const int crtc_vdisplay = crtc_state->hw.adjusted_mode.crtc_vdisplay;
> > >   	u16 y_granularity = 0;
> > > @@ -809,10 +812,13 @@ static bool psr2_granularity_check(struct intel_dp *intel_dp,
> > >   		return intel_dp->psr.su_y_granularity == 4;
> > >   
> > >   	/*
> > > -	 * For SW tracking we can adjust the y to match sink requirement if
> > > -	 * multiple of 4
> > > +	 * adl_p has 1 line granularity for other platforms with SW tracking we
> > > +	 * can adjust the y coordinate to match sink requirement if multiple of
> > > +	 * 4
> > >   	 */
> > > -	if (intel_dp->psr.su_y_granularity <= 2)
> > > +	if (IS_ALDERLAKE_P(dev_priv))
> > > +		y_granularity = intel_dp->psr.su_y_granularity;
> > > +	else if (intel_dp->psr.su_y_granularity <= 2)
> > >   		y_granularity = 4;
> > >   	else if ((intel_dp->psr.su_y_granularity % 4) == 0)
> > >   		y_granularity = intel_dp->psr.su_y_granularity;
> > > @@ -1525,21 +1531,32 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
> > >   static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
> > >   				  struct drm_rect *clip, bool full_update)
> > >   {
> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > >   	u32 val = PSR2_MAN_TRK_CTL_ENABLE;
> > The logic is not wrong, but the meaning of the register bit has changed.
> > The 31st bit in ADL-P means "SF partial frame enable".
> > It is recommended to add a macro such as 
> > ADLP_PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_ENABLE to the code to clarify the 
> > role of the changed register.
> 
> In my opinion the meaning is the same, enable manual/software tracking.
> It was just a register rename done as part of changes of the other bits. 
> 
> But if you really think is necessary I can do that, please let me know.

And with or without that can I add your RVB?

> 
> > >   
> > >   	if (full_update) {
> > > -		val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > > +		if (IS_ALDERLAKE_P(dev_priv))
> > > +			val |= ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > > +		else
> > > +			val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > > +
> > >   		goto exit;
> > >   	}
> > >   
> > >   	if (clip->y1 == -1)
> > >   		goto exit;
> > >   
> > > -	drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip->y2 % 4);
> > > +	if (IS_ALDERLAKE_P(dev_priv)) {
> > > +		val |= ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1);
> > > +		val |= ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2);
> > > +	} else {
> > > +		drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip->y2 % 4);
> > >   
> > > -	val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> > > -	val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> > > -	val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1);
> > > +		val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> > > +		val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> > > +		val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1);
> > > +	}
> > >   exit:
> > >   	crtc_state->psr2_man_track_ctl = val;
> > >   }
> > > @@ -1563,11 +1580,15 @@ static void clip_area_update(struct drm_rect *overlap_damage_area,
> > >   static void intel_psr2_sel_fetch_pipe_alignment(const struct intel_crtc_state *crtc_state,
> > >   						struct drm_rect *pipe_clip)
> > >   {
> > > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> > >   	const u16 y_alignment = crtc_state->su_y_granularity;
> > >   
> > >   	pipe_clip->y1 -= pipe_clip->y1 % y_alignment;
> > >   	if (pipe_clip->y2 % y_alignment)
> > >   		pipe_clip->y2 = ((pipe_clip->y2 / y_alignment) + 1) * y_alignment;
> > > +
> > > +	if (IS_ALDERLAKE_P(dev_priv) && crtc_state->dsc.compression_enable)
> > > +		drm_warn(&dev_priv->drm, "Missing PSR2 sel fetch alignment with DSC\n");
> > >   }
> > >   
> > >   int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index e0bd60fe7a190..74dc5ebce60e7 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4586,7 +4586,7 @@ enum {
> > >   #define _PSR2_CTL_EDP				0x6f900
> > >   #define EDP_PSR2_CTL(tran)			_MMIO_TRANS2(tran, _PSR2_CTL_A)
> > >   #define   EDP_PSR2_ENABLE			(1 << 31)
> > > -#define   EDP_SU_TRACK_ENABLE			(1 << 30)
> > > +#define   EDP_SU_TRACK_ENABLE			(1 << 30) /* up to adl-p */
> > >   #define   TGL_EDP_PSR2_BLOCK_COUNT_NUM_2	(0 << 28)
> > >   #define   TGL_EDP_PSR2_BLOCK_COUNT_NUM_3	(1 << 28)
> > >   #define   EDP_Y_COORDINATE_ENABLE		REG_BIT(25) /* display 10, 11 and 12 */
> > > @@ -4655,17 +4655,23 @@ enum {
> > >   #define PSR2_SU_STATUS_MASK(frame)	(0x3ff << PSR2_SU_STATUS_SHIFT(frame))
> > >   #define PSR2_SU_STATUS_FRAMES		8
> > >   
> > > -#define _PSR2_MAN_TRK_CTL_A				0x60910
> > > -#define _PSR2_MAN_TRK_CTL_EDP				0x6f910
> > > -#define PSR2_MAN_TRK_CTL(tran)				_MMIO_TRANS2(tran, _PSR2_MAN_TRK_CTL_A)
> > > -#define  PSR2_MAN_TRK_CTL_ENABLE			REG_BIT(31)
> > > -#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK	REG_GENMASK(30, 21)
> > > -#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)	REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
> > > +#define _PSR2_MAN_TRK_CTL_A					0x60910
> > > +#define _PSR2_MAN_TRK_CTL_EDP					0x6f910
> > > +#define PSR2_MAN_TRK_CTL(tran)					_MMIO_TRANS2(tran, _PSR2_MAN_TRK_CTL_A)
> > > +#define  PSR2_MAN_TRK_CTL_ENABLE				REG_BIT(31)
> > > +#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK		REG_GENMASK(30, 21)
> > > +#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)		REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
> > >   #define  PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK		REG_GENMASK(20, 11)
> > >   #define  PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(val)		REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK, val)
> > > -#define  PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME		REG_BIT(3)
> > > -#define  PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME	REG_BIT(2)
> > > -#define  PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE	REG_BIT(1)
> > > +#define  PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME			REG_BIT(3)
> > > +#define  PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME		REG_BIT(2)
> > > +#define  PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE		REG_BIT(1)
> > > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK	REG_GENMASK(28, 16)
> > > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)	REG_FIELD_PREP(ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
> > > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK		REG_GENMASK(12, 0)
> > > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(val)		REG_FIELD_PREP(ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK, val)
> > > +#define  ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME		REG_BIT(14)
> > > +#define  ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME		REG_BIT(13)
> > >   
> > >   /* Icelake DSC Rate Control Range Parameter Registers */
> > >   #define DSCA_RC_RANGE_PARAMETERS_0		_MMIO(0x6B240)
> > > 
>
Souza, Jose June 24, 2021, 11:27 p.m. UTC | #4 
On Thu, 2021-06-24 at 16:19 +0000, Souza, Jose wrote:
> On Thu, 2021-06-24 at 09:22 -0700, José Roberto de Souza wrote:
> > On Wed, 2021-06-23 at 22:06 +0300, Gwan-gyeong Mun wrote:
> > > On 6/16/21 11:31 PM, José Roberto de Souza wrote:
> > > > Implements changes around PSR for alderlake-P:
> > > > 
> > > > - EDP_SU_TRACK_ENABLE was removed and bit 30 now has other function
> > > > - Some bits of PSR2_MAN_TRK_CTL moved and SF_PARTIAL_FRAME_UPDATE was
> > > >    removed setting SU_REGION_START/END_ADDR will do this job
> > > > - SU_REGION_START/END_ADDR have now line granularity but will need to
> > > >    be aligned with DSC when the PSRS + DSC support lands
> > > > 
> > > > BSpec: 50422
> > > > BSpec: 50424
> > > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/i915/display/intel_psr.c | 43 ++++++++++++++++++------
> > > >   drivers/gpu/drm/i915/i915_reg.h          | 26 ++++++++------
> > > >   2 files changed, 48 insertions(+), 21 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 9643624fe160d..46bb19c4b63a4 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -534,11 +534,13 @@ static u32 intel_psr2_get_tp_time(struct intel_dp *intel_dp)
> > > >   static void hsw_activate_psr2(struct intel_dp *intel_dp)
> > > >   {
> > > >   	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > > -	u32 val;
> > > > +	u32 val = EDP_PSR2_ENABLE;
> > > > +
> > > > +	val |= psr_compute_idle_frames(intel_dp) << EDP_PSR2_IDLE_FRAME_SHIFT;
> > > >   
> > > > -	val = psr_compute_idle_frames(intel_dp) << EDP_PSR2_IDLE_FRAME_SHIFT;
> > > > +	if (!IS_ALDERLAKE_P(dev_priv))
> > > > +		val |= EDP_SU_TRACK_ENABLE;
> > > >   
> > > > -	val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> > > >   	if (DISPLAY_VER(dev_priv) >= 10 && DISPLAY_VER(dev_priv) <= 12)
> > > >   		val |= EDP_Y_COORDINATE_ENABLE;
> > > >   
> > > > @@ -793,6 +795,7 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> > > >   static bool psr2_granularity_check(struct intel_dp *intel_dp,
> > > >   				   struct intel_crtc_state *crtc_state)
> > > >   {
> > > > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > >   	const int crtc_hdisplay = crtc_state->hw.adjusted_mode.crtc_hdisplay;
> > > >   	const int crtc_vdisplay = crtc_state->hw.adjusted_mode.crtc_vdisplay;
> > > >   	u16 y_granularity = 0;
> > > > @@ -809,10 +812,13 @@ static bool psr2_granularity_check(struct intel_dp *intel_dp,
> > > >   		return intel_dp->psr.su_y_granularity == 4;
> > > >   
> > > >   	/*
> > > > -	 * For SW tracking we can adjust the y to match sink requirement if
> > > > -	 * multiple of 4
> > > > +	 * adl_p has 1 line granularity for other platforms with SW tracking we
> > > > +	 * can adjust the y coordinate to match sink requirement if multiple of
> > > > +	 * 4
> > > >   	 */
> > > > -	if (intel_dp->psr.su_y_granularity <= 2)
> > > > +	if (IS_ALDERLAKE_P(dev_priv))
> > > > +		y_granularity = intel_dp->psr.su_y_granularity;
> > > > +	else if (intel_dp->psr.su_y_granularity <= 2)
> > > >   		y_granularity = 4;
> > > >   	else if ((intel_dp->psr.su_y_granularity % 4) == 0)
> > > >   		y_granularity = intel_dp->psr.su_y_granularity;
> > > > @@ -1525,21 +1531,32 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
> > > >   static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
> > > >   				  struct drm_rect *clip, bool full_update)
> > > >   {
> > > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > >   	u32 val = PSR2_MAN_TRK_CTL_ENABLE;
> > > The logic is not wrong, but the meaning of the register bit has changed.
> > > The 31st bit in ADL-P means "SF partial frame enable".
> > > It is recommended to add a macro such as 
> > > ADLP_PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_ENABLE to the code to clarify the 
> > > role of the changed register.
> > 
> > In my opinion the meaning is the same, enable manual/software tracking.
> > It was just a register rename done as part of changes of the other bits. 
> > 
> > But if you really think is necessary I can do that, please let me know.
> 
> And with or without that can I add your RVB?

I have pushed all the reviewed patches.

Another note about the requested change, we use PSR2_MAN_TRK_CTL_ENABLE in 3 different places.
Add a IS_ALDERLAKE_P() check in each place to use ADLP_PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_ENABLE instead of PSR2_MAN_TRK_CTL_ENABLE and both having the
same value looks terrible.

> 
> > 
> > > >   
> > > >   	if (full_update) {
> > > > -		val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > > > +		if (IS_ALDERLAKE_P(dev_priv))
> > > > +			val |= ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > > > +		else
> > > > +			val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > > > +
> > > >   		goto exit;
> > > >   	}
> > > >   
> > > >   	if (clip->y1 == -1)
> > > >   		goto exit;
> > > >   
> > > > -	drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip->y2 % 4);
> > > > +	if (IS_ALDERLAKE_P(dev_priv)) {
> > > > +		val |= ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1);
> > > > +		val |= ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2);
> > > > +	} else {
> > > > +		drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip->y2 % 4);
> > > >   
> > > > -	val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> > > > -	val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> > > > -	val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1);
> > > > +		val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> > > > +		val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> > > > +		val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1);
> > > > +	}
> > > >   exit:
> > > >   	crtc_state->psr2_man_track_ctl = val;
> > > >   }
> > > > @@ -1563,11 +1580,15 @@ static void clip_area_update(struct drm_rect *overlap_damage_area,
> > > >   static void intel_psr2_sel_fetch_pipe_alignment(const struct intel_crtc_state *crtc_state,
> > > >   						struct drm_rect *pipe_clip)
> > > >   {
> > > > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> > > >   	const u16 y_alignment = crtc_state->su_y_granularity;
> > > >   
> > > >   	pipe_clip->y1 -= pipe_clip->y1 % y_alignment;
> > > >   	if (pipe_clip->y2 % y_alignment)
> > > >   		pipe_clip->y2 = ((pipe_clip->y2 / y_alignment) + 1) * y_alignment;
> > > > +
> > > > +	if (IS_ALDERLAKE_P(dev_priv) && crtc_state->dsc.compression_enable)
> > > > +		drm_warn(&dev_priv->drm, "Missing PSR2 sel fetch alignment with DSC\n");
> > > >   }
> > > >   
> > > >   int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > > index e0bd60fe7a190..74dc5ebce60e7 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -4586,7 +4586,7 @@ enum {
> > > >   #define _PSR2_CTL_EDP				0x6f900
> > > >   #define EDP_PSR2_CTL(tran)			_MMIO_TRANS2(tran, _PSR2_CTL_A)
> > > >   #define   EDP_PSR2_ENABLE			(1 << 31)
> > > > -#define   EDP_SU_TRACK_ENABLE			(1 << 30)
> > > > +#define   EDP_SU_TRACK_ENABLE			(1 << 30) /* up to adl-p */
> > > >   #define   TGL_EDP_PSR2_BLOCK_COUNT_NUM_2	(0 << 28)
> > > >   #define   TGL_EDP_PSR2_BLOCK_COUNT_NUM_3	(1 << 28)
> > > >   #define   EDP_Y_COORDINATE_ENABLE		REG_BIT(25) /* display 10, 11 and 12 */
> > > > @@ -4655,17 +4655,23 @@ enum {
> > > >   #define PSR2_SU_STATUS_MASK(frame)	(0x3ff << PSR2_SU_STATUS_SHIFT(frame))
> > > >   #define PSR2_SU_STATUS_FRAMES		8
> > > >   
> > > > -#define _PSR2_MAN_TRK_CTL_A				0x60910
> > > > -#define _PSR2_MAN_TRK_CTL_EDP				0x6f910
> > > > -#define PSR2_MAN_TRK_CTL(tran)				_MMIO_TRANS2(tran, _PSR2_MAN_TRK_CTL_A)
> > > > -#define  PSR2_MAN_TRK_CTL_ENABLE			REG_BIT(31)
> > > > -#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK	REG_GENMASK(30, 21)
> > > > -#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)	REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
> > > > +#define _PSR2_MAN_TRK_CTL_A					0x60910
> > > > +#define _PSR2_MAN_TRK_CTL_EDP					0x6f910
> > > > +#define PSR2_MAN_TRK_CTL(tran)					_MMIO_TRANS2(tran, _PSR2_MAN_TRK_CTL_A)
> > > > +#define  PSR2_MAN_TRK_CTL_ENABLE				REG_BIT(31)
> > > > +#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK		REG_GENMASK(30, 21)
> > > > +#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)		REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
> > > >   #define  PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK		REG_GENMASK(20, 11)
> > > >   #define  PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(val)		REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK, val)
> > > > -#define  PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME		REG_BIT(3)
> > > > -#define  PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME	REG_BIT(2)
> > > > -#define  PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE	REG_BIT(1)
> > > > +#define  PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME			REG_BIT(3)
> > > > +#define  PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME		REG_BIT(2)
> > > > +#define  PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE		REG_BIT(1)
> > > > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK	REG_GENMASK(28, 16)
> > > > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)	REG_FIELD_PREP(ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
> > > > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK		REG_GENMASK(12, 0)
> > > > +#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(val)		REG_FIELD_PREP(ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK, val)
> > > > +#define  ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME		REG_BIT(14)
> > > > +#define  ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME		REG_BIT(13)
> > > >   
> > > >   /* Icelake DSC Rate Control Range Parameter Registers */
> > > >   #define DSCA_RC_RANGE_PARAMETERS_0		_MMIO(0x6B240)
> > > > 
> > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 9643624fe160d..46bb19c4b63a4 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -534,11 +534,13 @@  static u32 intel_psr2_get_tp_time(struct intel_dp *intel_dp)
 static void hsw_activate_psr2(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	u32 val;
+	u32 val = EDP_PSR2_ENABLE;
+
+	val |= psr_compute_idle_frames(intel_dp) << EDP_PSR2_IDLE_FRAME_SHIFT;
 
-	val = psr_compute_idle_frames(intel_dp) << EDP_PSR2_IDLE_FRAME_SHIFT;
+	if (!IS_ALDERLAKE_P(dev_priv))
+		val |= EDP_SU_TRACK_ENABLE;
 
-	val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
 	if (DISPLAY_VER(dev_priv) >= 10 && DISPLAY_VER(dev_priv) <= 12)
 		val |= EDP_Y_COORDINATE_ENABLE;
 
@@ -793,6 +795,7 @@  static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
 static bool psr2_granularity_check(struct intel_dp *intel_dp,
 				   struct intel_crtc_state *crtc_state)
 {
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 	const int crtc_hdisplay = crtc_state->hw.adjusted_mode.crtc_hdisplay;
 	const int crtc_vdisplay = crtc_state->hw.adjusted_mode.crtc_vdisplay;
 	u16 y_granularity = 0;
@@ -809,10 +812,13 @@  static bool psr2_granularity_check(struct intel_dp *intel_dp,
 		return intel_dp->psr.su_y_granularity == 4;
 
 	/*
-	 * For SW tracking we can adjust the y to match sink requirement if
-	 * multiple of 4
+	 * adl_p has 1 line granularity for other platforms with SW tracking we
+	 * can adjust the y coordinate to match sink requirement if multiple of
+	 * 4
 	 */
-	if (intel_dp->psr.su_y_granularity <= 2)
+	if (IS_ALDERLAKE_P(dev_priv))
+		y_granularity = intel_dp->psr.su_y_granularity;
+	else if (intel_dp->psr.su_y_granularity <= 2)
 		y_granularity = 4;
 	else if ((intel_dp->psr.su_y_granularity % 4) == 0)
 		y_granularity = intel_dp->psr.su_y_granularity;
@@ -1525,21 +1531,32 @@  void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
 static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
 				  struct drm_rect *clip, bool full_update)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	u32 val = PSR2_MAN_TRK_CTL_ENABLE;
 
 	if (full_update) {
-		val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
+		if (IS_ALDERLAKE_P(dev_priv))
+			val |= ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
+		else
+			val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
+
 		goto exit;
 	}
 
 	if (clip->y1 == -1)
 		goto exit;
 
-	drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip->y2 % 4);
+	if (IS_ALDERLAKE_P(dev_priv)) {
+		val |= ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1);
+		val |= ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2);
+	} else {
+		drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip->y2 % 4);
 
-	val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
-	val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
-	val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1);
+		val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
+		val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
+		val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1);
+	}
 exit:
 	crtc_state->psr2_man_track_ctl = val;
 }
@@ -1563,11 +1580,15 @@  static void clip_area_update(struct drm_rect *overlap_damage_area,
 static void intel_psr2_sel_fetch_pipe_alignment(const struct intel_crtc_state *crtc_state,
 						struct drm_rect *pipe_clip)
 {
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
 	const u16 y_alignment = crtc_state->su_y_granularity;
 
 	pipe_clip->y1 -= pipe_clip->y1 % y_alignment;
 	if (pipe_clip->y2 % y_alignment)
 		pipe_clip->y2 = ((pipe_clip->y2 / y_alignment) + 1) * y_alignment;
+
+	if (IS_ALDERLAKE_P(dev_priv) && crtc_state->dsc.compression_enable)
+		drm_warn(&dev_priv->drm, "Missing PSR2 sel fetch alignment with DSC\n");
 }
 
 int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e0bd60fe7a190..74dc5ebce60e7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4586,7 +4586,7 @@  enum {
 #define _PSR2_CTL_EDP				0x6f900
 #define EDP_PSR2_CTL(tran)			_MMIO_TRANS2(tran, _PSR2_CTL_A)
 #define   EDP_PSR2_ENABLE			(1 << 31)
-#define   EDP_SU_TRACK_ENABLE			(1 << 30)
+#define   EDP_SU_TRACK_ENABLE			(1 << 30) /* up to adl-p */
 #define   TGL_EDP_PSR2_BLOCK_COUNT_NUM_2	(0 << 28)
 #define   TGL_EDP_PSR2_BLOCK_COUNT_NUM_3	(1 << 28)
 #define   EDP_Y_COORDINATE_ENABLE		REG_BIT(25) /* display 10, 11 and 12 */
@@ -4655,17 +4655,23 @@  enum {
 #define PSR2_SU_STATUS_MASK(frame)	(0x3ff << PSR2_SU_STATUS_SHIFT(frame))
 #define PSR2_SU_STATUS_FRAMES		8
 
-#define _PSR2_MAN_TRK_CTL_A				0x60910
-#define _PSR2_MAN_TRK_CTL_EDP				0x6f910
-#define PSR2_MAN_TRK_CTL(tran)				_MMIO_TRANS2(tran, _PSR2_MAN_TRK_CTL_A)
-#define  PSR2_MAN_TRK_CTL_ENABLE			REG_BIT(31)
-#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK	REG_GENMASK(30, 21)
-#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)	REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
+#define _PSR2_MAN_TRK_CTL_A					0x60910
+#define _PSR2_MAN_TRK_CTL_EDP					0x6f910
+#define PSR2_MAN_TRK_CTL(tran)					_MMIO_TRANS2(tran, _PSR2_MAN_TRK_CTL_A)
+#define  PSR2_MAN_TRK_CTL_ENABLE				REG_BIT(31)
+#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK		REG_GENMASK(30, 21)
+#define  PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)		REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
 #define  PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK		REG_GENMASK(20, 11)
 #define  PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(val)		REG_FIELD_PREP(PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK, val)
-#define  PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME		REG_BIT(3)
-#define  PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME	REG_BIT(2)
-#define  PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE	REG_BIT(1)
+#define  PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME			REG_BIT(3)
+#define  PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME		REG_BIT(2)
+#define  PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE		REG_BIT(1)
+#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK	REG_GENMASK(28, 16)
+#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val)	REG_FIELD_PREP(ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val)
+#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK		REG_GENMASK(12, 0)
+#define  ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(val)		REG_FIELD_PREP(ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK, val)
+#define  ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME		REG_BIT(14)
+#define  ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME		REG_BIT(13)
 
 /* Icelake DSC Rate Control Range Parameter Registers */
 #define DSCA_RC_RANGE_PARAMETERS_0		_MMIO(0x6B240)