[Intel-gfx] [PATCH 01/13] drm/i915/dsb: Stop with the RMW

[Manna, Animesh]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Friday, December 16, 2022 6:08 AM
> To: intel-gfx at lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 01/13] drm/i915/dsb: Stop with the RMW
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> We don't want to keep random bits set in DSB_CTRL. Stop the harmful RMW.
> 
> Also flip the reverse & around to appease my ocd.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

LGTM.
Reviewed-by: Animesh Manna <animesh.manna at intel.com>


> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 22 +++++++---------------
>  drivers/gpu/drm/i915/i915_reg.h          |  2 +-
>  2 files changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 3d63c1bf1e4f..ebebaf802dee 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -73,42 +73,34 @@ struct intel_dsb {
>  static bool is_dsb_busy(struct drm_i915_private *i915, enum pipe pipe,
>  			enum dsb_id id)
>  {
> -	return DSB_STATUS & intel_de_read(i915, DSB_CTRL(pipe, id));
> +	return intel_de_read(i915, DSB_CTRL(pipe, id)) & DSB_STATUS_BUSY;
>  }
> 
>  static bool intel_dsb_enable_engine(struct drm_i915_private *i915,
>  				    enum pipe pipe, enum dsb_id id)  {
> -	u32 dsb_ctrl;
> -
> -	dsb_ctrl = intel_de_read(i915, DSB_CTRL(pipe, id));
> -	if (DSB_STATUS & dsb_ctrl) {
> +	if (is_dsb_busy(i915, pipe, id)) {
>  		drm_dbg_kms(&i915->drm, "DSB engine is busy.\n");
>  		return false;
>  	}
> 
> -	dsb_ctrl |= DSB_ENABLE;
> -	intel_de_write(i915, DSB_CTRL(pipe, id), dsb_ctrl);
> -
> +	intel_de_write(i915, DSB_CTRL(pipe, id), DSB_ENABLE);
>  	intel_de_posting_read(i915, DSB_CTRL(pipe, id));
> +
>  	return true;
>  }
> 
>  static bool intel_dsb_disable_engine(struct drm_i915_private *i915,
>  				     enum pipe pipe, enum dsb_id id)  {
> -	u32 dsb_ctrl;
> -
> -	dsb_ctrl = intel_de_read(i915, DSB_CTRL(pipe, id));
> -	if (DSB_STATUS & dsb_ctrl) {
> +	if (is_dsb_busy(i915, pipe, id)) {
>  		drm_dbg_kms(&i915->drm, "DSB engine is busy.\n");
>  		return false;
>  	}
> 
> -	dsb_ctrl &= ~DSB_ENABLE;
> -	intel_de_write(i915, DSB_CTRL(pipe, id), dsb_ctrl);
> -
> +	intel_de_write(i915, DSB_CTRL(pipe, id), 0);
>  	intel_de_posting_read(i915, DSB_CTRL(pipe, id));
> +
>  	return true;
>  }
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index cef9418beec0..ed989e749635
> 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8104,7 +8104,7 @@ enum skl_power_gate {
>  #define DSB_TAIL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) +
> 0x4)
>  #define DSB_CTRL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) +
> 0x8)
>  #define   DSB_ENABLE			(1 << 31)
> -#define   DSB_STATUS			(1 << 0)
> +#define   DSB_STATUS_BUSY		(1 << 0)
> 
>  #define CLKREQ_POLICY			_MMIO(0x101038)
>  #define  CLKREQ_POLICY_MEM_UP_OVRD	REG_BIT(1)
> --
> 2.37.4