[v6,06/10] drm/i915/dsb: functions to enable/disable DSB engine.

Submitted by Animesh Manna on Sept. 11, 2019, 7:11 p.m.

Details

Message ID 20190911191133.23383-7-animesh.manna@intel.com
State New
Headers show
Series "DSB enablement." ( rev: 6 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Animesh Manna Sept. 11, 2019, 7:11 p.m.
DSB will be used for performance improvement for some special scenario.
DSB engine will be enabled based on need and after completion of its work
will be disabled. Api added for enable/disable operation by using DSB_CTRL
register.

v1: Initial version.
v2: POSTING_READ added after writing control register. (Shashank)
v3: cosmetic changes done. (Shashank)

Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 40 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h          |  1 +
 2 files changed, 41 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index b1da2b06263a..2b0ffc0afb74 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -26,6 +26,46 @@  static inline bool is_dsb_busy(struct intel_dsb *dsb)
 	return DSB_STATUS & I915_READ(DSB_CTRL(pipe, dsb->id));
 }
 
+static inline bool intel_dsb_enable_engine(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	u32 dsb_ctrl;
+
+	dsb_ctrl = I915_READ(DSB_CTRL(pipe, dsb->id));
+	if (DSB_STATUS & dsb_ctrl) {
+		DRM_DEBUG_KMS("DSB engine is busy.\n");
+		return false;
+	}
+
+	dsb_ctrl |= DSB_ENABLE;
+	I915_WRITE(DSB_CTRL(pipe, dsb->id), dsb_ctrl);
+
+	POSTING_READ(DSB_CTRL(pipe, dsb->id));
+	return true;
+}
+
+static inline bool intel_dsb_disable_engine(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	u32 dsb_ctrl;
+
+	dsb_ctrl = I915_READ(DSB_CTRL(pipe, dsb->id));
+	if (DSB_STATUS & dsb_ctrl) {
+		DRM_DEBUG_KMS("DSB engine is busy.\n");
+		return false;
+	}
+
+	dsb_ctrl &= ~DSB_ENABLE;
+	I915_WRITE(DSB_CTRL(pipe, dsb->id), dsb_ctrl);
+
+	POSTING_READ(DSB_CTRL(pipe, dsb->id));
+	return true;
+}
+
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc)
 {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9188a0b53538..2dbaa49f5c74 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11688,6 +11688,7 @@  enum skl_power_gate {
 #define DSBSL_INSTANCE(pipe, id)	(_DSBSL_INSTANCE_BASE + \
 					 (pipe) * 0x1000 + (id) * 100)
 #define DSB_CTRL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x8)
+#define   DSB_ENABLE			(1 << 31)
 #define   DSB_STATUS			(1 << 0)
 
 #endif /* _I915_REG_H_ */

Comments

On 9/12/2019 12:41 AM, Animesh Manna wrote:
> DSB will be used for performance improvement for some special scenario.
> DSB engine will be enabled based on need and after completion of its work
> will be disabled. Api added for enable/disable operation by using DSB_CTRL
> register.
>
> v1: Initial version.
> v2: POSTING_READ added after writing control register. (Shashank)
> v3: cosmetic changes done. (Shashank)
>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dsb.c | 40 ++++++++++++++++++++++++
>   drivers/gpu/drm/i915/i915_reg.h          |  1 +
>   2 files changed, 41 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index b1da2b06263a..2b0ffc0afb74 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -26,6 +26,46 @@ static inline bool is_dsb_busy(struct intel_dsb *dsb)
>   	return DSB_STATUS & I915_READ(DSB_CTRL(pipe, dsb->id));
>   }
>   
> +static inline bool intel_dsb_enable_engine(struct intel_dsb *dsb)
> +{
> +	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	enum pipe pipe = crtc->pipe;
> +	u32 dsb_ctrl;
> +
> +	dsb_ctrl = I915_READ(DSB_CTRL(pipe, dsb->id));
> +	if (DSB_STATUS & dsb_ctrl) {
> +		DRM_DEBUG_KMS("DSB engine is busy.\n");
> +		return false;
> +	}
> +
> +	dsb_ctrl |= DSB_ENABLE;
> +	I915_WRITE(DSB_CTRL(pipe, dsb->id), dsb_ctrl);
> +
> +	POSTING_READ(DSB_CTRL(pipe, dsb->id));
> +	return true;
> +}
> +
> +static inline bool intel_dsb_disable_engine(struct intel_dsb *dsb)
> +{
> +	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	enum pipe pipe = crtc->pipe;
> +	u32 dsb_ctrl;
> +
> +	dsb_ctrl = I915_READ(DSB_CTRL(pipe, dsb->id));
> +	if (DSB_STATUS & dsb_ctrl) {
> +		DRM_DEBUG_KMS("DSB engine is busy.\n");
> +		return false;
> +	}
> +
> +	dsb_ctrl &= ~DSB_ENABLE;

Do we really need to care about reading the reg val first and then 
disabling it ? I can understand that for enable().

How about this:

if (!dsb_is_busy()) {

     I915_WRITE(DSB_CTRL(pipe, dsb->id), 0);

    POSTING_READ();

    return true;

}

DRM_DEBUG_KMS("DSB engine is busy.\n");
return false;

But this is optional suggestion, you can take a call on this.

> +	I915_WRITE(DSB_CTRL(pipe, dsb->id), dsb_ctrl);
> +
> +	POSTING_READ(DSB_CTRL(pipe, dsb->id));
> +	return true;
> +}
> +
>   struct intel_dsb *
>   intel_dsb_get(struct intel_crtc *crtc)
>   {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9188a0b53538..2dbaa49f5c74 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -11688,6 +11688,7 @@ enum skl_power_gate {
>   #define DSBSL_INSTANCE(pipe, id)	(_DSBSL_INSTANCE_BASE + \
>   					 (pipe) * 0x1000 + (id) * 100)
>   #define DSB_CTRL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x8)
> +#define   DSB_ENABLE			(1 << 31)
>   #define   DSB_STATUS			(1 << 0)
>   

With or without suggested change above: Feel free to use

Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>

- Shashank

>   #endif /* _I915_REG_H_ */