[Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Rodrigo Vivi]

uncore->lock only protects the forcewake domain itself,
not the register accesses.

uncore's _fw alternatives are for cases where the domains
are not needed because we are sure that they are already
awake.

So the move towards the uncore's _fw alternatives seems
right, however using the uncore-lock to protect the dsparb
registers seems an abuse of the uncore-lock.

Let's restore the previous individual lock and try to get
rid of the direct uncore accesses from the display code.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
 drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
 drivers/gpu/drm/i915/i915_driver.c                |  1 +
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
index caef72d38798..8fe0b5c63d3a 100644
--- a/drivers/gpu/drm/i915/display/i9xx_wm.c
+++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
@@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
 
-	/*
-	 * uncore.lock serves a double purpose here. It allows us to
-	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
-	 * it protects the DSPARB registers from getting clobbered by
-	 * parallel updates from multiple pipes.
-	 *
-	 * intel_pipe_update_start() has already disabled interrupts
-	 * for us, so a plain spin_lock() is sufficient here.
-	 */
-	spin_lock(&uncore->lock);
+	spin_lock(&dev_priv->display.wm.dsparb_lock);
 
 	switch (crtc->pipe) {
 	case PIPE_A:
@@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 	intel_uncore_posting_read_fw(uncore, DSPARB);
 
-	spin_unlock(&uncore->lock);
+	spin_unlock(&dev_priv->display.wm.dsparb_lock);
 }
 
 #undef VLV_FIFO
diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
index fdab7bb93a7d..68c6bfb91dbe 100644
--- a/drivers/gpu/drm/i915/display/intel_display_core.h
+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
@@ -253,6 +253,9 @@ struct intel_wm {
 	 */
 	struct mutex wm_mutex;
 
+	/* protects DSPARB registers on pre-g4x/vlv/chv */
+	spinlock_t dsparb_lock;
+
 	bool ipc_enabled;
 };
 
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index a53fd339e2cc..c78e36444a12 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	mutex_init(&dev_priv->display.pps.mutex);
 	mutex_init(&dev_priv->display.hdcp.comp_mutex);
 	spin_lock_init(&dev_priv->display.dkl.phy_lock);
+	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
 
 	i915_memcpy_init_early(dev_priv);
 	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
-- 
2.39.2

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Ville Syrjälä]

On Wed, Mar 08, 2023 at 11:58:58AM -0500, Rodrigo Vivi wrote:
> uncore->lock only protects the forcewake domain itself,
> not the register accesses.
> 
> uncore's _fw alternatives are for cases where the domains
> are not needed because we are sure that they are already
> awake.
> 
> So the move towards the uncore's _fw alternatives seems
> right, however using the uncore-lock to protect the dsparb
> registers seems an abuse of the uncore-lock.
> 
> Let's restore the previous individual lock and try to get
> rid of the direct uncore accesses from the display code.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
>  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
>  drivers/gpu/drm/i915/i915_driver.c                |  1 +
>  3 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> index caef72d38798..8fe0b5c63d3a 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  
>  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
>  
> -	/*
> -	 * uncore.lock serves a double purpose here. It allows us to
> -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> -	 * it protects the DSPARB registers from getting clobbered by
> -	 * parallel updates from multiple pipes.
> -	 *
> -	 * intel_pipe_update_start() has already disabled interrupts
> -	 * for us, so a plain spin_lock() is sufficient here.
> -	 */

I was wondering if we need to preserve the comment about irqs,
but since this is the only place using this lock, and it's never
called from an irq handler a non-irq disabling spinlock will suffice
anyway.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> -	spin_lock(&uncore->lock);
> +	spin_lock(&dev_priv->display.wm.dsparb_lock);
>  
>  	switch (crtc->pipe) {
>  	case PIPE_A:
> @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  
>  	intel_uncore_posting_read_fw(uncore, DSPARB);
>  
> -	spin_unlock(&uncore->lock);
> +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
>  }
>  
>  #undef VLV_FIFO
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> index fdab7bb93a7d..68c6bfb91dbe 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -253,6 +253,9 @@ struct intel_wm {
>  	 */
>  	struct mutex wm_mutex;
>  
> +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> +	spinlock_t dsparb_lock;
> +
>  	bool ipc_enabled;
>  };
>  
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index a53fd339e2cc..c78e36444a12 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>  	mutex_init(&dev_priv->display.pps.mutex);
>  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
>  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
>  
>  	i915_memcpy_init_early(dev_priv);
>  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
> -- 
> 2.39.2

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Rodrigo Vivi]

On Thu, Mar 09, 2023 at 12:03:19AM +0200, Ville Syrjälä wrote:
> On Wed, Mar 08, 2023 at 11:58:58AM -0500, Rodrigo Vivi wrote:
> > uncore->lock only protects the forcewake domain itself,
> > not the register accesses.
> > 
> > uncore's _fw alternatives are for cases where the domains
> > are not needed because we are sure that they are already
> > awake.
> > 
> > So the move towards the uncore's _fw alternatives seems
> > right, however using the uncore-lock to protect the dsparb
> > registers seems an abuse of the uncore-lock.
> > 
> > Let's restore the previous individual lock and try to get
> > rid of the direct uncore accesses from the display code.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
> >  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
> >  drivers/gpu/drm/i915/i915_driver.c                |  1 +
> >  3 files changed, 6 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > index caef72d38798..8fe0b5c63d3a 100644
> > --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> > +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> >  
> >  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
> >  
> > -	/*
> > -	 * uncore.lock serves a double purpose here. It allows us to
> > -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> > -	 * it protects the DSPARB registers from getting clobbered by
> > -	 * parallel updates from multiple pipes.
> > -	 *
> > -	 * intel_pipe_update_start() has already disabled interrupts
> > -	 * for us, so a plain spin_lock() is sufficient here.
> > -	 */
> 
> I was wondering if we need to preserve the comment about irqs,
> but since this is the only place using this lock, and it's never
> called from an irq handler a non-irq disabling spinlock will suffice
> anyway.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

thoughts on this: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-7567u/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-b-dp-1.html

maybe related to the usage of this uncore.lock in

drivers/gpu/drm/i915/display/intel_vblank.c

?

Should we create another spin lock and include both of these cases?
(Then the irq comment is relevant again :))

> 
> > -	spin_lock(&uncore->lock);
> > +	spin_lock(&dev_priv->display.wm.dsparb_lock);
> >  
> >  	switch (crtc->pipe) {
> >  	case PIPE_A:
> > @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> >  
> >  	intel_uncore_posting_read_fw(uncore, DSPARB);
> >  
> > -	spin_unlock(&uncore->lock);
> > +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
> >  }
> >  
> >  #undef VLV_FIFO
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> > index fdab7bb93a7d..68c6bfb91dbe 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> > @@ -253,6 +253,9 @@ struct intel_wm {
> >  	 */
> >  	struct mutex wm_mutex;
> >  
> > +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> > +	spinlock_t dsparb_lock;
> > +
> >  	bool ipc_enabled;
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> > index a53fd339e2cc..c78e36444a12 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
> >  	mutex_init(&dev_priv->display.pps.mutex);
> >  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
> >  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> > +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
> >  
> >  	i915_memcpy_init_early(dev_priv);
> >  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
> > -- 
> > 2.39.2
> 
> -- 
> Ville Syrjälä
> Intel

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Ville Syrjälä]

On Thu, Mar 09, 2023 at 05:03:52PM -0500, Rodrigo Vivi wrote:
> On Thu, Mar 09, 2023 at 12:03:19AM +0200, Ville Syrjälä wrote:
> > On Wed, Mar 08, 2023 at 11:58:58AM -0500, Rodrigo Vivi wrote:
> > > uncore->lock only protects the forcewake domain itself,
> > > not the register accesses.
> > > 
> > > uncore's _fw alternatives are for cases where the domains
> > > are not needed because we are sure that they are already
> > > awake.
> > > 
> > > So the move towards the uncore's _fw alternatives seems
> > > right, however using the uncore-lock to protect the dsparb
> > > registers seems an abuse of the uncore-lock.
> > > 
> > > Let's restore the previous individual lock and try to get
> > > rid of the direct uncore accesses from the display code.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
> > >  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
> > >  drivers/gpu/drm/i915/i915_driver.c                |  1 +
> > >  3 files changed, 6 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > index caef72d38798..8fe0b5c63d3a 100644
> > > --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> > >  
> > >  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
> > >  
> > > -	/*
> > > -	 * uncore.lock serves a double purpose here. It allows us to
> > > -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> > > -	 * it protects the DSPARB registers from getting clobbered by
> > > -	 * parallel updates from multiple pipes.
> > > -	 *
> > > -	 * intel_pipe_update_start() has already disabled interrupts
> > > -	 * for us, so a plain spin_lock() is sufficient here.
> > > -	 */
> > 
> > I was wondering if we need to preserve the comment about irqs,
> > but since this is the only place using this lock, and it's never
> > called from an irq handler a non-irq disabling spinlock will suffice
> > anyway.
> > 
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> thoughts on this: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-7567u/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-b-dp-1.html

This code doesn't run on that platform, so unrelated.

> 
> maybe related to the usage of this uncore.lock in
> 
> drivers/gpu/drm/i915/display/intel_vblank.c
> 
> ?
> 
> Should we create another spin lock and include both of these cases?
> (Then the irq comment is relevant again :))

We're already 4 spinlocks deep when in vblank code. Let's not add more ;)

> 
> > 
> > > -	spin_lock(&uncore->lock);
> > > +	spin_lock(&dev_priv->display.wm.dsparb_lock);
> > >  
> > >  	switch (crtc->pipe) {
> > >  	case PIPE_A:
> > > @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> > >  
> > >  	intel_uncore_posting_read_fw(uncore, DSPARB);
> > >  
> > > -	spin_unlock(&uncore->lock);
> > > +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
> > >  }
> > >  
> > >  #undef VLV_FIFO
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> > > index fdab7bb93a7d..68c6bfb91dbe 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> > > @@ -253,6 +253,9 @@ struct intel_wm {
> > >  	 */
> > >  	struct mutex wm_mutex;
> > >  
> > > +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> > > +	spinlock_t dsparb_lock;
> > > +
> > >  	bool ipc_enabled;
> > >  };
> > >  
> > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> > > index a53fd339e2cc..c78e36444a12 100644
> > > --- a/drivers/gpu/drm/i915/i915_driver.c
> > > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > > @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
> > >  	mutex_init(&dev_priv->display.pps.mutex);
> > >  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
> > >  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> > > +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
> > >  
> > >  	i915_memcpy_init_early(dev_priv);
> > >  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
> > > -- 
> > > 2.39.2
> > 
> > -- 
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Rodrigo Vivi]

On Fri, Mar 10, 2023 at 06:26:54PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 09, 2023 at 05:03:52PM -0500, Rodrigo Vivi wrote:
> > On Thu, Mar 09, 2023 at 12:03:19AM +0200, Ville Syrjälä wrote:
> > > On Wed, Mar 08, 2023 at 11:58:58AM -0500, Rodrigo Vivi wrote:
> > > > uncore->lock only protects the forcewake domain itself,
> > > > not the register accesses.
> > > > 
> > > > uncore's _fw alternatives are for cases where the domains
> > > > are not needed because we are sure that they are already
> > > > awake.
> > > > 
> > > > So the move towards the uncore's _fw alternatives seems
> > > > right, however using the uncore-lock to protect the dsparb
> > > > registers seems an abuse of the uncore-lock.
> > > > 
> > > > Let's restore the previous individual lock and try to get
> > > > rid of the direct uncore accesses from the display code.
> > > > 
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
> > > >  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
> > > >  drivers/gpu/drm/i915/i915_driver.c                |  1 +
> > > >  3 files changed, 6 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > > index caef72d38798..8fe0b5c63d3a 100644
> > > > --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > > +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > > > @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> > > >  
> > > >  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
> > > >  
> > > > -	/*
> > > > -	 * uncore.lock serves a double purpose here. It allows us to
> > > > -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> > > > -	 * it protects the DSPARB registers from getting clobbered by
> > > > -	 * parallel updates from multiple pipes.
> > > > -	 *
> > > > -	 * intel_pipe_update_start() has already disabled interrupts
> > > > -	 * for us, so a plain spin_lock() is sufficient here.
> > > > -	 */
> > > 
> > > I was wondering if we need to preserve the comment about irqs,
> > > but since this is the only place using this lock, and it's never
> > > called from an irq handler a non-irq disabling spinlock will suffice
> > > anyway.
> > > 
> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > thoughts on this: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114868v2/fi-kbl-7567u/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-b-dp-1.html
> 
> This code doesn't run on that platform, so unrelated.

oh! indeed.
okay, I just triggered a rerun to get the full round... luckly...

> 
> > 
> > maybe related to the usage of this uncore.lock in
> > 
> > drivers/gpu/drm/i915/display/intel_vblank.c
> > 
> > ?
> > 
> > Should we create another spin lock and include both of these cases?
> > (Then the irq comment is relevant again :))
> 
> We're already 4 spinlocks deep when in vblank code. Let's not add more ;)
> 
> > 
> > > 
> > > > -	spin_lock(&uncore->lock);
> > > > +	spin_lock(&dev_priv->display.wm.dsparb_lock);
> > > >  
> > > >  	switch (crtc->pipe) {
> > > >  	case PIPE_A:
> > > > @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> > > >  
> > > >  	intel_uncore_posting_read_fw(uncore, DSPARB);
> > > >  
> > > > -	spin_unlock(&uncore->lock);
> > > > +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
> > > >  }
> > > >  
> > > >  #undef VLV_FIFO
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> > > > index fdab7bb93a7d..68c6bfb91dbe 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> > > > @@ -253,6 +253,9 @@ struct intel_wm {
> > > >  	 */
> > > >  	struct mutex wm_mutex;
> > > >  
> > > > +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> > > > +	spinlock_t dsparb_lock;
> > > > +
> > > >  	bool ipc_enabled;
> > > >  };
> > > >  
> > > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> > > > index a53fd339e2cc..c78e36444a12 100644
> > > > --- a/drivers/gpu/drm/i915/i915_driver.c
> > > > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > > > @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
> > > >  	mutex_init(&dev_priv->display.pps.mutex);
> > > >  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
> > > >  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> > > > +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
> > > >  
> > > >  	i915_memcpy_init_early(dev_priv);
> > > >  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
> > > > -- 
> > > > 2.39.2
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Ville Syrjälä
> Intel

[Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Rodrigo Vivi]

uncore->lock only protects the forcewake domain itself,
not the register accesses.

uncore's _fw alternatives are for cases where the domains
are not needed because we are sure that they are already
awake.

So the move towards the uncore's _fw alternatives seems
right, however using the uncore-lock to protect the dsparb
registers seems an abuse of the uncore-lock.

Let's restore the previous individual lock and try to get
rid of the direct uncore accesses from the display code.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230308165859.235520-1-rodrigo.vivi@intel.com
---
 drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
 drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
 drivers/gpu/drm/i915/i915_driver.c                |  1 +
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
index caef72d38798..8fe0b5c63d3a 100644
--- a/drivers/gpu/drm/i915/display/i9xx_wm.c
+++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
@@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
 
-	/*
-	 * uncore.lock serves a double purpose here. It allows us to
-	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
-	 * it protects the DSPARB registers from getting clobbered by
-	 * parallel updates from multiple pipes.
-	 *
-	 * intel_pipe_update_start() has already disabled interrupts
-	 * for us, so a plain spin_lock() is sufficient here.
-	 */
-	spin_lock(&uncore->lock);
+	spin_lock(&dev_priv->display.wm.dsparb_lock);
 
 	switch (crtc->pipe) {
 	case PIPE_A:
@@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 	intel_uncore_posting_read_fw(uncore, DSPARB);
 
-	spin_unlock(&uncore->lock);
+	spin_unlock(&dev_priv->display.wm.dsparb_lock);
 }
 
 #undef VLV_FIFO
diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
index 0b5509f268a7..e4da8902c878 100644
--- a/drivers/gpu/drm/i915/display/intel_display_core.h
+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
@@ -264,6 +264,9 @@ struct intel_wm {
 	 */
 	struct mutex wm_mutex;
 
+	/* protects DSPARB registers on pre-g4x/vlv/chv */
+	spinlock_t dsparb_lock;
+
 	bool ipc_enabled;
 };
 
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 12b5296ee744..e90a0c0403a6 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	mutex_init(&dev_priv->display.pps.mutex);
 	mutex_init(&dev_priv->display.hdcp.comp_mutex);
 	spin_lock_init(&dev_priv->display.dkl.phy_lock);
+	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
 
 	i915_memcpy_init_early(dev_priv);
 	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
-- 
2.39.2

[Intel-gfx] [PATCH 2/2] drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.

[Rodrigo Vivi]

Let's get rid of the intel_uncore calls on display side.

v2: Fix multiple CHECK:PARENTHESIS_ALIGNMENT:
    Alignment should match open parenthesis

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230308165859.235520-2-rodrigo.vivi@intel.com
---
 drivers/gpu/drm/i915/display/i9xx_wm.c  | 366 ++++++++++++------------
 drivers/gpu/drm/i915/display/intel_de.h |  12 +
 2 files changed, 195 insertions(+), 183 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
index 8fe0b5c63d3a..22669b73d4c6 100644
--- a/drivers/gpu/drm/i915/display/i9xx_wm.c
+++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
@@ -6,6 +6,7 @@
 #include "i915_drv.h"
 #include "i9xx_wm.h"
 #include "intel_atomic.h"
+#include "intel_de.h"
 #include "intel_display.h"
 #include "intel_display_trace.h"
 #include "intel_mchbar_regs.h"
@@ -141,39 +142,39 @@ static bool _intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enabl
 	u32 val;
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		was_enabled = intel_uncore_read(&dev_priv->uncore, FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
-		intel_uncore_write(&dev_priv->uncore, FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0);
-		intel_uncore_posting_read(&dev_priv->uncore, FW_BLC_SELF_VLV);
+		was_enabled = intel_de_read(dev_priv, FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
+		intel_de_write(dev_priv, FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0);
+		intel_de_posting_read(dev_priv, FW_BLC_SELF_VLV);
 	} else if (IS_G4X(dev_priv) || IS_I965GM(dev_priv)) {
-		was_enabled = intel_uncore_read(&dev_priv->uncore, FW_BLC_SELF) & FW_BLC_SELF_EN;
-		intel_uncore_write(&dev_priv->uncore, FW_BLC_SELF, enable ? FW_BLC_SELF_EN : 0);
-		intel_uncore_posting_read(&dev_priv->uncore, FW_BLC_SELF);
+		was_enabled = intel_de_read(dev_priv, FW_BLC_SELF) & FW_BLC_SELF_EN;
+		intel_de_write(dev_priv, FW_BLC_SELF, enable ? FW_BLC_SELF_EN : 0);
+		intel_de_posting_read(dev_priv, FW_BLC_SELF);
 	} else if (IS_PINEVIEW(dev_priv)) {
-		val = intel_uncore_read(&dev_priv->uncore, DSPFW3);
+		val = intel_de_read(dev_priv, DSPFW3);
 		was_enabled = val & PINEVIEW_SELF_REFRESH_EN;
 		if (enable)
 			val |= PINEVIEW_SELF_REFRESH_EN;
 		else
 			val &= ~PINEVIEW_SELF_REFRESH_EN;
-		intel_uncore_write(&dev_priv->uncore, DSPFW3, val);
-		intel_uncore_posting_read(&dev_priv->uncore, DSPFW3);
+		intel_de_write(dev_priv, DSPFW3, val);
+		intel_de_posting_read(dev_priv, DSPFW3);
 	} else if (IS_I945G(dev_priv) || IS_I945GM(dev_priv)) {
-		was_enabled = intel_uncore_read(&dev_priv->uncore, FW_BLC_SELF) & FW_BLC_SELF_EN;
+		was_enabled = intel_de_read(dev_priv, FW_BLC_SELF) & FW_BLC_SELF_EN;
 		val = enable ? _MASKED_BIT_ENABLE(FW_BLC_SELF_EN) :
 			       _MASKED_BIT_DISABLE(FW_BLC_SELF_EN);
-		intel_uncore_write(&dev_priv->uncore, FW_BLC_SELF, val);
-		intel_uncore_posting_read(&dev_priv->uncore, FW_BLC_SELF);
+		intel_de_write(dev_priv, FW_BLC_SELF, val);
+		intel_de_posting_read(dev_priv, FW_BLC_SELF);
 	} else if (IS_I915GM(dev_priv)) {
 		/*
 		 * FIXME can't find a bit like this for 915G, and
 		 * yet it does have the related watermark in
 		 * FW_BLC_SELF. What's going on?
 		 */
-		was_enabled = intel_uncore_read(&dev_priv->uncore, INSTPM) & INSTPM_SELF_EN;
+		was_enabled = intel_de_read(dev_priv, INSTPM) & INSTPM_SELF_EN;
 		val = enable ? _MASKED_BIT_ENABLE(INSTPM_SELF_EN) :
 			       _MASKED_BIT_DISABLE(INSTPM_SELF_EN);
-		intel_uncore_write(&dev_priv->uncore, INSTPM, val);
-		intel_uncore_posting_read(&dev_priv->uncore, INSTPM);
+		intel_de_write(dev_priv, INSTPM, val);
+		intel_de_posting_read(dev_priv, INSTPM);
 	} else {
 		return false;
 	}
@@ -269,20 +270,20 @@ static void vlv_get_fifo_size(struct intel_crtc_state *crtc_state)
 
 	switch (pipe) {
 	case PIPE_A:
-		dsparb = intel_uncore_read(&dev_priv->uncore, DSPARB);
-		dsparb2 = intel_uncore_read(&dev_priv->uncore, DSPARB2);
+		dsparb = intel_de_read(dev_priv, DSPARB);
+		dsparb2 = intel_de_read(dev_priv, DSPARB2);
 		sprite0_start = VLV_FIFO_START(dsparb, dsparb2, 0, 0);
 		sprite1_start = VLV_FIFO_START(dsparb, dsparb2, 8, 4);
 		break;
 	case PIPE_B:
-		dsparb = intel_uncore_read(&dev_priv->uncore, DSPARB);
-		dsparb2 = intel_uncore_read(&dev_priv->uncore, DSPARB2);
+		dsparb = intel_de_read(dev_priv, DSPARB);
+		dsparb2 = intel_de_read(dev_priv, DSPARB2);
 		sprite0_start = VLV_FIFO_START(dsparb, dsparb2, 16, 8);
 		sprite1_start = VLV_FIFO_START(dsparb, dsparb2, 24, 12);
 		break;
 	case PIPE_C:
-		dsparb2 = intel_uncore_read(&dev_priv->uncore, DSPARB2);
-		dsparb3 = intel_uncore_read(&dev_priv->uncore, DSPARB3);
+		dsparb2 = intel_de_read(dev_priv, DSPARB2);
+		dsparb3 = intel_de_read(dev_priv, DSPARB3);
 		sprite0_start = VLV_FIFO_START(dsparb3, dsparb2, 0, 16);
 		sprite1_start = VLV_FIFO_START(dsparb3, dsparb2, 8, 20);
 		break;
@@ -300,7 +301,7 @@ static void vlv_get_fifo_size(struct intel_crtc_state *crtc_state)
 static int i9xx_get_fifo_size(struct drm_i915_private *dev_priv,
 			      enum i9xx_plane_id i9xx_plane)
 {
-	u32 dsparb = intel_uncore_read(&dev_priv->uncore, DSPARB);
+	u32 dsparb = intel_de_read(dev_priv, DSPARB);
 	int size;
 
 	size = dsparb & 0x7f;
@@ -316,7 +317,7 @@ static int i9xx_get_fifo_size(struct drm_i915_private *dev_priv,
 static int i830_get_fifo_size(struct drm_i915_private *dev_priv,
 			      enum i9xx_plane_id i9xx_plane)
 {
-	u32 dsparb = intel_uncore_read(&dev_priv->uncore, DSPARB);
+	u32 dsparb = intel_de_read(dev_priv, DSPARB);
 	int size;
 
 	size = dsparb & 0x1ff;
@@ -333,7 +334,7 @@ static int i830_get_fifo_size(struct drm_i915_private *dev_priv,
 static int i845_get_fifo_size(struct drm_i915_private *dev_priv,
 			      enum i9xx_plane_id i9xx_plane)
 {
-	u32 dsparb = intel_uncore_read(&dev_priv->uncore, DSPARB);
+	u32 dsparb = intel_de_read(dev_priv, DSPARB);
 	int size;
 
 	size = dsparb & 0x7f;
@@ -655,33 +656,33 @@ static void pnv_update_wm(struct drm_i915_private *dev_priv)
 		wm = intel_calculate_wm(pixel_rate, &pnv_display_wm,
 					pnv_display_wm.fifo_size,
 					cpp, latency->display_sr);
-		reg = intel_uncore_read(&dev_priv->uncore, DSPFW1);
+		reg = intel_de_read(dev_priv, DSPFW1);
 		reg &= ~DSPFW_SR_MASK;
 		reg |= FW_WM(wm, SR);
-		intel_uncore_write(&dev_priv->uncore, DSPFW1, reg);
+		intel_de_write(dev_priv, DSPFW1, reg);
 		drm_dbg_kms(&dev_priv->drm, "DSPFW1 register is %x\n", reg);
 
 		/* cursor SR */
 		wm = intel_calculate_wm(pixel_rate, &pnv_cursor_wm,
 					pnv_display_wm.fifo_size,
 					4, latency->cursor_sr);
-		intel_uncore_rmw(&dev_priv->uncore, DSPFW3, DSPFW_CURSOR_SR_MASK,
-				 FW_WM(wm, CURSOR_SR));
+		intel_de_rmw(dev_priv, DSPFW3, DSPFW_CURSOR_SR_MASK,
+			     FW_WM(wm, CURSOR_SR));
 
 		/* Display HPLL off SR */
 		wm = intel_calculate_wm(pixel_rate, &pnv_display_hplloff_wm,
 					pnv_display_hplloff_wm.fifo_size,
 					cpp, latency->display_hpll_disable);
-		intel_uncore_rmw(&dev_priv->uncore, DSPFW3, DSPFW_HPLL_SR_MASK, FW_WM(wm, HPLL_SR));
+		intel_de_rmw(dev_priv, DSPFW3, DSPFW_HPLL_SR_MASK, FW_WM(wm, HPLL_SR));
 
 		/* cursor HPLL off SR */
 		wm = intel_calculate_wm(pixel_rate, &pnv_cursor_hplloff_wm,
 					pnv_display_hplloff_wm.fifo_size,
 					4, latency->cursor_hpll_disable);
-		reg = intel_uncore_read(&dev_priv->uncore, DSPFW3);
+		reg = intel_de_read(dev_priv, DSPFW3);
 		reg &= ~DSPFW_HPLL_CURSOR_MASK;
 		reg |= FW_WM(wm, HPLL_CURSOR);
-		intel_uncore_write(&dev_priv->uncore, DSPFW3, reg);
+		intel_de_write(dev_priv, DSPFW3, reg);
 		drm_dbg_kms(&dev_priv->drm, "DSPFW3 register is %x\n", reg);
 
 		intel_set_memory_cxsr(dev_priv, true);
@@ -715,25 +716,25 @@ static void g4x_write_wm_values(struct drm_i915_private *dev_priv,
 	for_each_pipe(dev_priv, pipe)
 		trace_g4x_wm(intel_crtc_for_pipe(dev_priv, pipe), wm);
 
-	intel_uncore_write(&dev_priv->uncore, DSPFW1,
-			   FW_WM(wm->sr.plane, SR) |
-			   FW_WM(wm->pipe[PIPE_B].plane[PLANE_CURSOR], CURSORB) |
-			   FW_WM(wm->pipe[PIPE_B].plane[PLANE_PRIMARY], PLANEB) |
-			   FW_WM(wm->pipe[PIPE_A].plane[PLANE_PRIMARY], PLANEA));
-	intel_uncore_write(&dev_priv->uncore, DSPFW2,
-			   (wm->fbc_en ? DSPFW_FBC_SR_EN : 0) |
-			   FW_WM(wm->sr.fbc, FBC_SR) |
-			   FW_WM(wm->hpll.fbc, FBC_HPLL_SR) |
-			   FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE0], SPRITEB) |
-			   FW_WM(wm->pipe[PIPE_A].plane[PLANE_CURSOR], CURSORA) |
-			   FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE0], SPRITEA));
-	intel_uncore_write(&dev_priv->uncore, DSPFW3,
-			   (wm->hpll_en ? DSPFW_HPLL_SR_EN : 0) |
-			   FW_WM(wm->sr.cursor, CURSOR_SR) |
-			   FW_WM(wm->hpll.cursor, HPLL_CURSOR) |
-			   FW_WM(wm->hpll.plane, HPLL_SR));
-
-	intel_uncore_posting_read(&dev_priv->uncore, DSPFW1);
+	intel_de_write(dev_priv, DSPFW1,
+		       FW_WM(wm->sr.plane, SR) |
+		       FW_WM(wm->pipe[PIPE_B].plane[PLANE_CURSOR], CURSORB) |
+		       FW_WM(wm->pipe[PIPE_B].plane[PLANE_PRIMARY], PLANEB) |
+		       FW_WM(wm->pipe[PIPE_A].plane[PLANE_PRIMARY], PLANEA));
+	intel_de_write(dev_priv, DSPFW2,
+		       (wm->fbc_en ? DSPFW_FBC_SR_EN : 0) |
+		       FW_WM(wm->sr.fbc, FBC_SR) |
+		       FW_WM(wm->hpll.fbc, FBC_HPLL_SR) |
+		       FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE0], SPRITEB) |
+		       FW_WM(wm->pipe[PIPE_A].plane[PLANE_CURSOR], CURSORA) |
+		       FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE0], SPRITEA));
+	intel_de_write(dev_priv, DSPFW3,
+		       (wm->hpll_en ? DSPFW_HPLL_SR_EN : 0) |
+		       FW_WM(wm->sr.cursor, CURSOR_SR) |
+		       FW_WM(wm->hpll.cursor, HPLL_CURSOR) |
+		       FW_WM(wm->hpll.plane, HPLL_SR));
+
+	intel_de_posting_read(dev_priv, DSPFW1);
 }
 
 #define FW_WM_VLV(value, plane) \
@@ -747,11 +748,11 @@ static void vlv_write_wm_values(struct drm_i915_private *dev_priv,
 	for_each_pipe(dev_priv, pipe) {
 		trace_vlv_wm(intel_crtc_for_pipe(dev_priv, pipe), wm);
 
-		intel_uncore_write(&dev_priv->uncore, VLV_DDL(pipe),
-				   (wm->ddl[pipe].plane[PLANE_CURSOR] << DDL_CURSOR_SHIFT) |
-				   (wm->ddl[pipe].plane[PLANE_SPRITE1] << DDL_SPRITE_SHIFT(1)) |
-				   (wm->ddl[pipe].plane[PLANE_SPRITE0] << DDL_SPRITE_SHIFT(0)) |
-				   (wm->ddl[pipe].plane[PLANE_PRIMARY] << DDL_PLANE_SHIFT));
+		intel_de_write(dev_priv, VLV_DDL(pipe),
+			       (wm->ddl[pipe].plane[PLANE_CURSOR] << DDL_CURSOR_SHIFT) |
+			       (wm->ddl[pipe].plane[PLANE_SPRITE1] << DDL_SPRITE_SHIFT(1)) |
+			       (wm->ddl[pipe].plane[PLANE_SPRITE0] << DDL_SPRITE_SHIFT(0)) |
+			       (wm->ddl[pipe].plane[PLANE_PRIMARY] << DDL_PLANE_SHIFT));
 	}
 
 	/*
@@ -759,60 +760,60 @@ static void vlv_write_wm_values(struct drm_i915_private *dev_priv,
 	 * high order bits so that there are no out of bounds values
 	 * present in the registers during the reprogramming.
 	 */
-	intel_uncore_write(&dev_priv->uncore, DSPHOWM, 0);
-	intel_uncore_write(&dev_priv->uncore, DSPHOWM1, 0);
-	intel_uncore_write(&dev_priv->uncore, DSPFW4, 0);
-	intel_uncore_write(&dev_priv->uncore, DSPFW5, 0);
-	intel_uncore_write(&dev_priv->uncore, DSPFW6, 0);
-
-	intel_uncore_write(&dev_priv->uncore, DSPFW1,
-			   FW_WM(wm->sr.plane, SR) |
-			   FW_WM(wm->pipe[PIPE_B].plane[PLANE_CURSOR], CURSORB) |
-			   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_PRIMARY], PLANEB) |
-			   FW_WM_VLV(wm->pipe[PIPE_A].plane[PLANE_PRIMARY], PLANEA));
-	intel_uncore_write(&dev_priv->uncore, DSPFW2,
-			   FW_WM_VLV(wm->pipe[PIPE_A].plane[PLANE_SPRITE1], SPRITEB) |
-			   FW_WM(wm->pipe[PIPE_A].plane[PLANE_CURSOR], CURSORA) |
-			   FW_WM_VLV(wm->pipe[PIPE_A].plane[PLANE_SPRITE0], SPRITEA));
-	intel_uncore_write(&dev_priv->uncore, DSPFW3,
-			   FW_WM(wm->sr.cursor, CURSOR_SR));
+	intel_de_write(dev_priv, DSPHOWM, 0);
+	intel_de_write(dev_priv, DSPHOWM1, 0);
+	intel_de_write(dev_priv, DSPFW4, 0);
+	intel_de_write(dev_priv, DSPFW5, 0);
+	intel_de_write(dev_priv, DSPFW6, 0);
+
+	intel_de_write(dev_priv, DSPFW1,
+		       FW_WM(wm->sr.plane, SR) |
+		       FW_WM(wm->pipe[PIPE_B].plane[PLANE_CURSOR], CURSORB) |
+		       FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_PRIMARY], PLANEB) |
+		       FW_WM_VLV(wm->pipe[PIPE_A].plane[PLANE_PRIMARY], PLANEA));
+	intel_de_write(dev_priv, DSPFW2,
+		       FW_WM_VLV(wm->pipe[PIPE_A].plane[PLANE_SPRITE1], SPRITEB) |
+		       FW_WM(wm->pipe[PIPE_A].plane[PLANE_CURSOR], CURSORA) |
+		       FW_WM_VLV(wm->pipe[PIPE_A].plane[PLANE_SPRITE0], SPRITEA));
+	intel_de_write(dev_priv, DSPFW3,
+		       FW_WM(wm->sr.cursor, CURSOR_SR));
 
 	if (IS_CHERRYVIEW(dev_priv)) {
-		intel_uncore_write(&dev_priv->uncore, DSPFW7_CHV,
-				   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE1], SPRITED) |
-				   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE0], SPRITEC));
-		intel_uncore_write(&dev_priv->uncore, DSPFW8_CHV,
-				   FW_WM_VLV(wm->pipe[PIPE_C].plane[PLANE_SPRITE1], SPRITEF) |
-				   FW_WM_VLV(wm->pipe[PIPE_C].plane[PLANE_SPRITE0], SPRITEE));
-		intel_uncore_write(&dev_priv->uncore, DSPFW9_CHV,
-				   FW_WM_VLV(wm->pipe[PIPE_C].plane[PLANE_PRIMARY], PLANEC) |
-				   FW_WM(wm->pipe[PIPE_C].plane[PLANE_CURSOR], CURSORC));
-		intel_uncore_write(&dev_priv->uncore, DSPHOWM,
-				   FW_WM(wm->sr.plane >> 9, SR_HI) |
-				   FW_WM(wm->pipe[PIPE_C].plane[PLANE_SPRITE1] >> 8, SPRITEF_HI) |
-				   FW_WM(wm->pipe[PIPE_C].plane[PLANE_SPRITE0] >> 8, SPRITEE_HI) |
-				   FW_WM(wm->pipe[PIPE_C].plane[PLANE_PRIMARY] >> 8, PLANEC_HI) |
-				   FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE1] >> 8, SPRITED_HI) |
-				   FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE0] >> 8, SPRITEC_HI) |
-				   FW_WM(wm->pipe[PIPE_B].plane[PLANE_PRIMARY] >> 8, PLANEB_HI) |
-				   FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE1] >> 8, SPRITEB_HI) |
-				   FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE0] >> 8, SPRITEA_HI) |
-				   FW_WM(wm->pipe[PIPE_A].plane[PLANE_PRIMARY] >> 8, PLANEA_HI));
+		intel_de_write(dev_priv, DSPFW7_CHV,
+			       FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE1], SPRITED) |
+			       FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE0], SPRITEC));
+		intel_de_write(dev_priv, DSPFW8_CHV,
+			       FW_WM_VLV(wm->pipe[PIPE_C].plane[PLANE_SPRITE1], SPRITEF) |
+			       FW_WM_VLV(wm->pipe[PIPE_C].plane[PLANE_SPRITE0], SPRITEE));
+		intel_de_write(dev_priv, DSPFW9_CHV,
+			       FW_WM_VLV(wm->pipe[PIPE_C].plane[PLANE_PRIMARY], PLANEC) |
+			       FW_WM(wm->pipe[PIPE_C].plane[PLANE_CURSOR], CURSORC));
+		intel_de_write(dev_priv, DSPHOWM,
+			       FW_WM(wm->sr.plane >> 9, SR_HI) |
+			       FW_WM(wm->pipe[PIPE_C].plane[PLANE_SPRITE1] >> 8, SPRITEF_HI) |
+			       FW_WM(wm->pipe[PIPE_C].plane[PLANE_SPRITE0] >> 8, SPRITEE_HI) |
+			       FW_WM(wm->pipe[PIPE_C].plane[PLANE_PRIMARY] >> 8, PLANEC_HI) |
+			       FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE1] >> 8, SPRITED_HI) |
+			       FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE0] >> 8, SPRITEC_HI) |
+			       FW_WM(wm->pipe[PIPE_B].plane[PLANE_PRIMARY] >> 8, PLANEB_HI) |
+			       FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE1] >> 8, SPRITEB_HI) |
+			       FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE0] >> 8, SPRITEA_HI) |
+			       FW_WM(wm->pipe[PIPE_A].plane[PLANE_PRIMARY] >> 8, PLANEA_HI));
 	} else {
-		intel_uncore_write(&dev_priv->uncore, DSPFW7,
-				   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE1], SPRITED) |
-				   FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE0], SPRITEC));
-		intel_uncore_write(&dev_priv->uncore, DSPHOWM,
-				   FW_WM(wm->sr.plane >> 9, SR_HI) |
-				   FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE1] >> 8, SPRITED_HI) |
-				   FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE0] >> 8, SPRITEC_HI) |
-				   FW_WM(wm->pipe[PIPE_B].plane[PLANE_PRIMARY] >> 8, PLANEB_HI) |
-				   FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE1] >> 8, SPRITEB_HI) |
-				   FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE0] >> 8, SPRITEA_HI) |
-				   FW_WM(wm->pipe[PIPE_A].plane[PLANE_PRIMARY] >> 8, PLANEA_HI));
+		intel_de_write(dev_priv, DSPFW7,
+			       FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE1], SPRITED) |
+			       FW_WM_VLV(wm->pipe[PIPE_B].plane[PLANE_SPRITE0], SPRITEC));
+		intel_de_write(dev_priv, DSPHOWM,
+			       FW_WM(wm->sr.plane >> 9, SR_HI) |
+			       FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE1] >> 8, SPRITED_HI) |
+			       FW_WM(wm->pipe[PIPE_B].plane[PLANE_SPRITE0] >> 8, SPRITEC_HI) |
+			       FW_WM(wm->pipe[PIPE_B].plane[PLANE_PRIMARY] >> 8, PLANEB_HI) |
+			       FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE1] >> 8, SPRITEB_HI) |
+			       FW_WM(wm->pipe[PIPE_A].plane[PLANE_SPRITE0] >> 8, SPRITEA_HI) |
+			       FW_WM(wm->pipe[PIPE_A].plane[PLANE_PRIMARY] >> 8, PLANEA_HI));
 	}
 
-	intel_uncore_posting_read(&dev_priv->uncore, DSPFW1);
+	intel_de_posting_read(dev_priv, DSPFW1);
 }
 
 #undef FW_WM_VLV
@@ -1751,7 +1752,6 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 				   struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct intel_uncore *uncore = &dev_priv->uncore;
 	const struct intel_crtc_state *crtc_state =
 		intel_atomic_get_new_crtc_state(state, crtc);
 	const struct vlv_fifo_state *fifo_state =
@@ -1775,8 +1775,8 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 	switch (crtc->pipe) {
 	case PIPE_A:
-		dsparb = intel_uncore_read_fw(uncore, DSPARB);
-		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
+		dsparb = intel_de_read_fw(dev_priv, DSPARB);
+		dsparb2 = intel_de_read_fw(dev_priv, DSPARB2);
 
 		dsparb &= ~(VLV_FIFO(SPRITEA, 0xff) |
 			    VLV_FIFO(SPRITEB, 0xff));
@@ -1788,12 +1788,12 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 		dsparb2 |= (VLV_FIFO(SPRITEA_HI, sprite0_start >> 8) |
 			   VLV_FIFO(SPRITEB_HI, sprite1_start >> 8));
 
-		intel_uncore_write_fw(uncore, DSPARB, dsparb);
-		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
+		intel_de_write_fw(dev_priv, DSPARB, dsparb);
+		intel_de_write_fw(dev_priv, DSPARB2, dsparb2);
 		break;
 	case PIPE_B:
-		dsparb = intel_uncore_read_fw(uncore, DSPARB);
-		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
+		dsparb = intel_de_read_fw(dev_priv, DSPARB);
+		dsparb2 = intel_de_read_fw(dev_priv, DSPARB2);
 
 		dsparb &= ~(VLV_FIFO(SPRITEC, 0xff) |
 			    VLV_FIFO(SPRITED, 0xff));
@@ -1805,12 +1805,12 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 		dsparb2 |= (VLV_FIFO(SPRITEC_HI, sprite0_start >> 8) |
 			   VLV_FIFO(SPRITED_HI, sprite1_start >> 8));
 
-		intel_uncore_write_fw(uncore, DSPARB, dsparb);
-		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
+		intel_de_write_fw(dev_priv, DSPARB, dsparb);
+		intel_de_write_fw(dev_priv, DSPARB2, dsparb2);
 		break;
 	case PIPE_C:
-		dsparb3 = intel_uncore_read_fw(uncore, DSPARB3);
-		dsparb2 = intel_uncore_read_fw(uncore, DSPARB2);
+		dsparb3 = intel_de_read_fw(dev_priv, DSPARB3);
+		dsparb2 = intel_de_read_fw(dev_priv, DSPARB2);
 
 		dsparb3 &= ~(VLV_FIFO(SPRITEE, 0xff) |
 			     VLV_FIFO(SPRITEF, 0xff));
@@ -1822,14 +1822,14 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 		dsparb2 |= (VLV_FIFO(SPRITEE_HI, sprite0_start >> 8) |
 			   VLV_FIFO(SPRITEF_HI, sprite1_start >> 8));
 
-		intel_uncore_write_fw(uncore, DSPARB3, dsparb3);
-		intel_uncore_write_fw(uncore, DSPARB2, dsparb2);
+		intel_de_write_fw(dev_priv, DSPARB3, dsparb3);
+		intel_de_write_fw(dev_priv, DSPARB2, dsparb2);
 		break;
 	default:
 		break;
 	}
 
-	intel_uncore_posting_read_fw(uncore, DSPARB);
+	intel_de_posting_read_fw(dev_priv, DSPARB);
 
 	spin_unlock(&dev_priv->display.wm.dsparb_lock);
 }
@@ -2053,14 +2053,14 @@ static void i965_update_wm(struct drm_i915_private *dev_priv)
 		    srwm);
 
 	/* 965 has limitations... */
-	intel_uncore_write(&dev_priv->uncore, DSPFW1, FW_WM(srwm, SR) |
-		   FW_WM(8, CURSORB) |
-		   FW_WM(8, PLANEB) |
-		   FW_WM(8, PLANEA));
-	intel_uncore_write(&dev_priv->uncore, DSPFW2, FW_WM(8, CURSORA) |
-		   FW_WM(8, PLANEC_OLD));
+	intel_de_write(dev_priv, DSPFW1, FW_WM(srwm, SR) |
+		       FW_WM(8, CURSORB) |
+		       FW_WM(8, PLANEB) |
+		       FW_WM(8, PLANEA));
+	intel_de_write(dev_priv, DSPFW2, FW_WM(8, CURSORA) |
+		       FW_WM(8, PLANEC_OLD));
 	/* update cursor SR watermark */
-	intel_uncore_write(&dev_priv->uncore, DSPFW3, FW_WM(cursor_sr, CURSOR_SR));
+	intel_de_write(dev_priv, DSPFW3, FW_WM(cursor_sr, CURSOR_SR));
 
 	if (cxsr_enabled)
 		intel_set_memory_cxsr(dev_priv, true);
@@ -2201,10 +2201,10 @@ static void i9xx_update_wm(struct drm_i915_private *dev_priv)
 			srwm = 1;
 
 		if (IS_I945G(dev_priv) || IS_I945GM(dev_priv))
-			intel_uncore_write(&dev_priv->uncore, FW_BLC_SELF,
-				   FW_BLC_SELF_FIFO_MASK | (srwm & 0xff));
+			intel_de_write(dev_priv, FW_BLC_SELF,
+				       FW_BLC_SELF_FIFO_MASK | (srwm & 0xff));
 		else
-			intel_uncore_write(&dev_priv->uncore, FW_BLC_SELF, srwm & 0x3f);
+			intel_de_write(dev_priv, FW_BLC_SELF, srwm & 0x3f);
 	}
 
 	drm_dbg_kms(&dev_priv->drm,
@@ -2218,8 +2218,8 @@ static void i9xx_update_wm(struct drm_i915_private *dev_priv)
 	fwater_lo = fwater_lo | (1 << 24) | (1 << 8);
 	fwater_hi = fwater_hi | (1 << 8);
 
-	intel_uncore_write(&dev_priv->uncore, FW_BLC, fwater_lo);
-	intel_uncore_write(&dev_priv->uncore, FW_BLC2, fwater_hi);
+	intel_de_write(dev_priv, FW_BLC, fwater_lo);
+	intel_de_write(dev_priv, FW_BLC2, fwater_hi);
 
 	if (crtc)
 		intel_set_memory_cxsr(dev_priv, true);
@@ -2239,13 +2239,13 @@ static void i845_update_wm(struct drm_i915_private *dev_priv)
 				       &i845_wm_info,
 				       i845_get_fifo_size(dev_priv, PLANE_A),
 				       4, pessimal_latency_ns);
-	fwater_lo = intel_uncore_read(&dev_priv->uncore, FW_BLC) & ~0xfff;
+	fwater_lo = intel_de_read(dev_priv, FW_BLC) & ~0xfff;
 	fwater_lo |= (3<<8) | planea_wm;
 
 	drm_dbg_kms(&dev_priv->drm,
 		    "Setting FIFO watermarks - A: %d\n", planea_wm);
 
-	intel_uncore_write(&dev_priv->uncore, FW_BLC, fwater_lo);
+	intel_de_write(dev_priv, FW_BLC, fwater_lo);
 }
 
 /* latency must be in 0.1us units. */
@@ -2603,7 +2603,7 @@ static void hsw_read_wm_latency(struct drm_i915_private *i915, u16 wm[])
 
 	i915->display.wm.num_levels = 5;
 
-	sskpd = intel_uncore_read64(&i915->uncore, MCH_SSKPD);
+	sskpd = intel_de_read64(i915, MCH_SSKPD);
 
 	wm[0] = REG_FIELD_GET64(SSKPD_NEW_WM0_MASK_HSW, sskpd);
 	if (wm[0] == 0)
@@ -2620,7 +2620,7 @@ static void snb_read_wm_latency(struct drm_i915_private *i915, u16 wm[])
 
 	i915->display.wm.num_levels = 4;
 
-	sskpd = intel_uncore_read(&i915->uncore, MCH_SSKPD);
+	sskpd = intel_de_read(i915, MCH_SSKPD);
 
 	wm[0] = REG_FIELD_GET(SSKPD_WM0_MASK_SNB, sskpd);
 	wm[1] = REG_FIELD_GET(SSKPD_WM1_MASK_SNB, sskpd);
@@ -2634,7 +2634,7 @@ static void ilk_read_wm_latency(struct drm_i915_private *i915, u16 wm[])
 
 	i915->display.wm.num_levels = 3;
 
-	mltr = intel_uncore_read(&i915->uncore, MLTR_ILK);
+	mltr = intel_de_read(i915, MLTR_ILK);
 
 	/* ILK primary LP0 latency is 700 ns */
 	wm[0] = 7;
@@ -3163,17 +3163,17 @@ static bool _ilk_disable_lp_wm(struct drm_i915_private *dev_priv,
 
 	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] & WM_LP_ENABLE) {
 		previous->wm_lp[2] &= ~WM_LP_ENABLE;
-		intel_uncore_write(&dev_priv->uncore, WM3_LP_ILK, previous->wm_lp[2]);
+		intel_de_write(dev_priv, WM3_LP_ILK, previous->wm_lp[2]);
 		changed = true;
 	}
 	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] & WM_LP_ENABLE) {
 		previous->wm_lp[1] &= ~WM_LP_ENABLE;
-		intel_uncore_write(&dev_priv->uncore, WM2_LP_ILK, previous->wm_lp[1]);
+		intel_de_write(dev_priv, WM2_LP_ILK, previous->wm_lp[1]);
 		changed = true;
 	}
 	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] & WM_LP_ENABLE) {
 		previous->wm_lp[0] &= ~WM_LP_ENABLE;
-		intel_uncore_write(&dev_priv->uncore, WM1_LP_ILK, previous->wm_lp[0]);
+		intel_de_write(dev_priv, WM1_LP_ILK, previous->wm_lp[0]);
 		changed = true;
 	}
 
@@ -3202,44 +3202,44 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
 	_ilk_disable_lp_wm(dev_priv, dirty);
 
 	if (dirty & WM_DIRTY_PIPE(PIPE_A))
-		intel_uncore_write(&dev_priv->uncore, WM0_PIPE_ILK(PIPE_A), results->wm_pipe[0]);
+		intel_de_write(dev_priv, WM0_PIPE_ILK(PIPE_A), results->wm_pipe[0]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_B))
-		intel_uncore_write(&dev_priv->uncore, WM0_PIPE_ILK(PIPE_B), results->wm_pipe[1]);
+		intel_de_write(dev_priv, WM0_PIPE_ILK(PIPE_B), results->wm_pipe[1]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_C))
-		intel_uncore_write(&dev_priv->uncore, WM0_PIPE_ILK(PIPE_C), results->wm_pipe[2]);
+		intel_de_write(dev_priv, WM0_PIPE_ILK(PIPE_C), results->wm_pipe[2]);
 
 	if (dirty & WM_DIRTY_DDB) {
 		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-			intel_uncore_rmw(&dev_priv->uncore, WM_MISC, WM_MISC_DATA_PARTITION_5_6,
-					 results->partitioning == INTEL_DDB_PART_1_2 ? 0 :
-					 WM_MISC_DATA_PARTITION_5_6);
+			intel_de_rmw(dev_priv, WM_MISC, WM_MISC_DATA_PARTITION_5_6,
+				     results->partitioning == INTEL_DDB_PART_1_2 ? 0 :
+				     WM_MISC_DATA_PARTITION_5_6);
 		else
-			intel_uncore_rmw(&dev_priv->uncore, DISP_ARB_CTL2, DISP_DATA_PARTITION_5_6,
-					 results->partitioning == INTEL_DDB_PART_1_2 ? 0 :
-					 DISP_DATA_PARTITION_5_6);
+			intel_de_rmw(dev_priv, DISP_ARB_CTL2, DISP_DATA_PARTITION_5_6,
+				     results->partitioning == INTEL_DDB_PART_1_2 ? 0 :
+				     DISP_DATA_PARTITION_5_6);
 	}
 
 	if (dirty & WM_DIRTY_FBC)
-		intel_uncore_rmw(&dev_priv->uncore, DISP_ARB_CTL, DISP_FBC_WM_DIS,
-				 results->enable_fbc_wm ? 0 : DISP_FBC_WM_DIS);
+		intel_de_rmw(dev_priv, DISP_ARB_CTL, DISP_FBC_WM_DIS,
+			     results->enable_fbc_wm ? 0 : DISP_FBC_WM_DIS);
 
 	if (dirty & WM_DIRTY_LP(1) &&
 	    previous->wm_lp_spr[0] != results->wm_lp_spr[0])
-		intel_uncore_write(&dev_priv->uncore, WM1S_LP_ILK, results->wm_lp_spr[0]);
+		intel_de_write(dev_priv, WM1S_LP_ILK, results->wm_lp_spr[0]);
 
 	if (DISPLAY_VER(dev_priv) >= 7) {
 		if (dirty & WM_DIRTY_LP(2) && previous->wm_lp_spr[1] != results->wm_lp_spr[1])
-			intel_uncore_write(&dev_priv->uncore, WM2S_LP_IVB, results->wm_lp_spr[1]);
+			intel_de_write(dev_priv, WM2S_LP_IVB, results->wm_lp_spr[1]);
 		if (dirty & WM_DIRTY_LP(3) && previous->wm_lp_spr[2] != results->wm_lp_spr[2])
-			intel_uncore_write(&dev_priv->uncore, WM3S_LP_IVB, results->wm_lp_spr[2]);
+			intel_de_write(dev_priv, WM3S_LP_IVB, results->wm_lp_spr[2]);
 	}
 
 	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] != results->wm_lp[0])
-		intel_uncore_write(&dev_priv->uncore, WM1_LP_ILK, results->wm_lp[0]);
+		intel_de_write(dev_priv, WM1_LP_ILK, results->wm_lp[0]);
 	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] != results->wm_lp[1])
-		intel_uncore_write(&dev_priv->uncore, WM2_LP_ILK, results->wm_lp[1]);
+		intel_de_write(dev_priv, WM2_LP_ILK, results->wm_lp[1]);
 	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] != results->wm_lp[2])
-		intel_uncore_write(&dev_priv->uncore, WM3_LP_ILK, results->wm_lp[2]);
+		intel_de_write(dev_priv, WM3_LP_ILK, results->wm_lp[2]);
 
 	dev_priv->display.wm.hw = *results;
 }
@@ -3337,7 +3337,7 @@ static void ilk_pipe_wm_get_hw_state(struct intel_crtc *crtc)
 	struct intel_pipe_wm *active = &crtc_state->wm.ilk.optimal;
 	enum pipe pipe = crtc->pipe;
 
-	hw->wm_pipe[pipe] = intel_uncore_read(&dev_priv->uncore, WM0_PIPE_ILK(pipe));
+	hw->wm_pipe[pipe] = intel_de_read(dev_priv, WM0_PIPE_ILK(pipe));
 
 	memset(active, 0, sizeof(*active));
 
@@ -3502,13 +3502,13 @@ static void g4x_read_wm_values(struct drm_i915_private *dev_priv,
 {
 	u32 tmp;
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW1);
+	tmp = intel_de_read(dev_priv, DSPFW1);
 	wm->sr.plane = _FW_WM(tmp, SR);
 	wm->pipe[PIPE_B].plane[PLANE_CURSOR] = _FW_WM(tmp, CURSORB);
 	wm->pipe[PIPE_B].plane[PLANE_PRIMARY] = _FW_WM(tmp, PLANEB);
 	wm->pipe[PIPE_A].plane[PLANE_PRIMARY] = _FW_WM(tmp, PLANEA);
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW2);
+	tmp = intel_de_read(dev_priv, DSPFW2);
 	wm->fbc_en = tmp & DSPFW_FBC_SR_EN;
 	wm->sr.fbc = _FW_WM(tmp, FBC_SR);
 	wm->hpll.fbc = _FW_WM(tmp, FBC_HPLL_SR);
@@ -3516,7 +3516,7 @@ static void g4x_read_wm_values(struct drm_i915_private *dev_priv,
 	wm->pipe[PIPE_A].plane[PLANE_CURSOR] = _FW_WM(tmp, CURSORA);
 	wm->pipe[PIPE_A].plane[PLANE_SPRITE0] = _FW_WM(tmp, SPRITEA);
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW3);
+	tmp = intel_de_read(dev_priv, DSPFW3);
 	wm->hpll_en = tmp & DSPFW_HPLL_SR_EN;
 	wm->sr.cursor = _FW_WM(tmp, CURSOR_SR);
 	wm->hpll.cursor = _FW_WM(tmp, HPLL_CURSOR);
@@ -3530,7 +3530,7 @@ static void vlv_read_wm_values(struct drm_i915_private *dev_priv,
 	u32 tmp;
 
 	for_each_pipe(dev_priv, pipe) {
-		tmp = intel_uncore_read(&dev_priv->uncore, VLV_DDL(pipe));
+		tmp = intel_de_read(dev_priv, VLV_DDL(pipe));
 
 		wm->ddl[pipe].plane[PLANE_PRIMARY] =
 			(tmp >> DDL_PLANE_SHIFT) & (DDL_PRECISION_HIGH | DRAIN_LATENCY_MASK);
@@ -3542,34 +3542,34 @@ static void vlv_read_wm_values(struct drm_i915_private *dev_priv,
 			(tmp >> DDL_SPRITE_SHIFT(1)) & (DDL_PRECISION_HIGH | DRAIN_LATENCY_MASK);
 	}
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW1);
+	tmp = intel_de_read(dev_priv, DSPFW1);
 	wm->sr.plane = _FW_WM(tmp, SR);
 	wm->pipe[PIPE_B].plane[PLANE_CURSOR] = _FW_WM(tmp, CURSORB);
 	wm->pipe[PIPE_B].plane[PLANE_PRIMARY] = _FW_WM_VLV(tmp, PLANEB);
 	wm->pipe[PIPE_A].plane[PLANE_PRIMARY] = _FW_WM_VLV(tmp, PLANEA);
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW2);
+	tmp = intel_de_read(dev_priv, DSPFW2);
 	wm->pipe[PIPE_A].plane[PLANE_SPRITE1] = _FW_WM_VLV(tmp, SPRITEB);
 	wm->pipe[PIPE_A].plane[PLANE_CURSOR] = _FW_WM(tmp, CURSORA);
 	wm->pipe[PIPE_A].plane[PLANE_SPRITE0] = _FW_WM_VLV(tmp, SPRITEA);
 
-	tmp = intel_uncore_read(&dev_priv->uncore, DSPFW3);
+	tmp = intel_de_read(dev_priv, DSPFW3);
 	wm->sr.cursor = _FW_WM(tmp, CURSOR_SR);
 
 	if (IS_CHERRYVIEW(dev_priv)) {
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPFW7_CHV);
+		tmp = intel_de_read(dev_priv, DSPFW7_CHV);
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE1] = _FW_WM_VLV(tmp, SPRITED);
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE0] = _FW_WM_VLV(tmp, SPRITEC);
 
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPFW8_CHV);
+		tmp = intel_de_read(dev_priv, DSPFW8_CHV);
 		wm->pipe[PIPE_C].plane[PLANE_SPRITE1] = _FW_WM_VLV(tmp, SPRITEF);
 		wm->pipe[PIPE_C].plane[PLANE_SPRITE0] = _FW_WM_VLV(tmp, SPRITEE);
 
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPFW9_CHV);
+		tmp = intel_de_read(dev_priv, DSPFW9_CHV);
 		wm->pipe[PIPE_C].plane[PLANE_PRIMARY] = _FW_WM_VLV(tmp, PLANEC);
 		wm->pipe[PIPE_C].plane[PLANE_CURSOR] = _FW_WM(tmp, CURSORC);
 
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPHOWM);
+		tmp = intel_de_read(dev_priv, DSPHOWM);
 		wm->sr.plane |= _FW_WM(tmp, SR_HI) << 9;
 		wm->pipe[PIPE_C].plane[PLANE_SPRITE1] |= _FW_WM(tmp, SPRITEF_HI) << 8;
 		wm->pipe[PIPE_C].plane[PLANE_SPRITE0] |= _FW_WM(tmp, SPRITEE_HI) << 8;
@@ -3581,11 +3581,11 @@ static void vlv_read_wm_values(struct drm_i915_private *dev_priv,
 		wm->pipe[PIPE_A].plane[PLANE_SPRITE0] |= _FW_WM(tmp, SPRITEA_HI) << 8;
 		wm->pipe[PIPE_A].plane[PLANE_PRIMARY] |= _FW_WM(tmp, PLANEA_HI) << 8;
 	} else {
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPFW7);
+		tmp = intel_de_read(dev_priv, DSPFW7);
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE1] = _FW_WM_VLV(tmp, SPRITED);
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE0] = _FW_WM_VLV(tmp, SPRITEC);
 
-		tmp = intel_uncore_read(&dev_priv->uncore, DSPHOWM);
+		tmp = intel_de_read(dev_priv, DSPHOWM);
 		wm->sr.plane |= _FW_WM(tmp, SR_HI) << 9;
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE1] |= _FW_WM(tmp, SPRITED_HI) << 8;
 		wm->pipe[PIPE_B].plane[PLANE_SPRITE0] |= _FW_WM(tmp, SPRITEC_HI) << 8;
@@ -3606,7 +3606,7 @@ static void g4x_wm_get_hw_state(struct drm_i915_private *dev_priv)
 
 	g4x_read_wm_values(dev_priv, wm);
 
-	wm->cxsr = intel_uncore_read(&dev_priv->uncore, FW_BLC_SELF) & FW_BLC_SELF_EN;
+	wm->cxsr = intel_de_read(dev_priv, FW_BLC_SELF) & FW_BLC_SELF_EN;
 
 	for_each_intel_crtc(&dev_priv->drm, crtc) {
 		struct intel_crtc_state *crtc_state =
@@ -3755,7 +3755,7 @@ static void vlv_wm_get_hw_state(struct drm_i915_private *dev_priv)
 
 	vlv_read_wm_values(dev_priv, wm);
 
-	wm->cxsr = intel_uncore_read(&dev_priv->uncore, FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
+	wm->cxsr = intel_de_read(dev_priv, FW_BLC_SELF_VLV) & FW_CSPWRDWNEN;
 	wm->level = VLV_WM_LEVEL_PM2;
 
 	if (IS_CHERRYVIEW(dev_priv)) {
@@ -3905,9 +3905,9 @@ static void vlv_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915)
  */
 static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
 {
-	intel_uncore_rmw(&dev_priv->uncore, WM3_LP_ILK, WM_LP_ENABLE, 0);
-	intel_uncore_rmw(&dev_priv->uncore, WM2_LP_ILK, WM_LP_ENABLE, 0);
-	intel_uncore_rmw(&dev_priv->uncore, WM1_LP_ILK, WM_LP_ENABLE, 0);
+	intel_de_rmw(dev_priv, WM3_LP_ILK, WM_LP_ENABLE, 0);
+	intel_de_rmw(dev_priv, WM2_LP_ILK, WM_LP_ENABLE, 0);
+	intel_de_rmw(dev_priv, WM1_LP_ILK, WM_LP_ENABLE, 0);
 
 	/*
 	 * Don't touch WM_LP_SPRITE_ENABLE here.
@@ -3925,27 +3925,27 @@ static void ilk_wm_get_hw_state(struct drm_i915_private *dev_priv)
 	for_each_intel_crtc(&dev_priv->drm, crtc)
 		ilk_pipe_wm_get_hw_state(crtc);
 
-	hw->wm_lp[0] = intel_uncore_read(&dev_priv->uncore, WM1_LP_ILK);
-	hw->wm_lp[1] = intel_uncore_read(&dev_priv->uncore, WM2_LP_ILK);
-	hw->wm_lp[2] = intel_uncore_read(&dev_priv->uncore, WM3_LP_ILK);
+	hw->wm_lp[0] = intel_de_read(dev_priv, WM1_LP_ILK);
+	hw->wm_lp[1] = intel_de_read(dev_priv, WM2_LP_ILK);
+	hw->wm_lp[2] = intel_de_read(dev_priv, WM3_LP_ILK);
 
-	hw->wm_lp_spr[0] = intel_uncore_read(&dev_priv->uncore, WM1S_LP_ILK);
+	hw->wm_lp_spr[0] = intel_de_read(dev_priv, WM1S_LP_ILK);
 	if (DISPLAY_VER(dev_priv) >= 7) {
-		hw->wm_lp_spr[1] = intel_uncore_read(&dev_priv->uncore, WM2S_LP_IVB);
-		hw->wm_lp_spr[2] = intel_uncore_read(&dev_priv->uncore, WM3S_LP_IVB);
+		hw->wm_lp_spr[1] = intel_de_read(dev_priv, WM2S_LP_IVB);
+		hw->wm_lp_spr[2] = intel_de_read(dev_priv, WM3S_LP_IVB);
 	}
 
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		hw->partitioning = (intel_uncore_read(&dev_priv->uncore, WM_MISC) &
+		hw->partitioning = (intel_de_read(dev_priv, WM_MISC) &
 				    WM_MISC_DATA_PARTITION_5_6) ?
 			INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
 	else if (IS_IVYBRIDGE(dev_priv))
-		hw->partitioning = (intel_uncore_read(&dev_priv->uncore, DISP_ARB_CTL2) &
+		hw->partitioning = (intel_de_read(dev_priv, DISP_ARB_CTL2) &
 				    DISP_DATA_PARTITION_5_6) ?
 			INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
 
 	hw->enable_fbc_wm =
-		!(intel_uncore_read(&dev_priv->uncore, DISP_ARB_CTL) & DISP_FBC_WM_DIS);
+		!(intel_de_read(dev_priv, DISP_ARB_CTL) & DISP_FBC_WM_DIS);
 }
 
 static const struct intel_wm_funcs ilk_wm_funcs = {
diff --git a/drivers/gpu/drm/i915/display/intel_de.h b/drivers/gpu/drm/i915/display/intel_de.h
index 42552d8c151e..0c64245a181d 100644
--- a/drivers/gpu/drm/i915/display/intel_de.h
+++ b/drivers/gpu/drm/i915/display/intel_de.h
@@ -22,6 +22,12 @@ intel_de_read8(struct drm_i915_private *i915, i915_reg_t reg)
 	return intel_uncore_read8(&i915->uncore, reg);
 }
 
+static inline u8
+intel_de_read64(struct drm_i915_private *i915, i915_reg_t reg)
+{
+	return intel_uncore_read64(&i915->uncore, reg);
+}
+
 static inline u64
 intel_de_read64_2x32(struct drm_i915_private *i915,
 		     i915_reg_t lower_reg, i915_reg_t upper_reg)
@@ -104,6 +110,12 @@ intel_de_read_fw(struct drm_i915_private *i915, i915_reg_t reg)
 	return val;
 }
 
+static inline void
+intel_de_posting_read_fw(struct drm_i915_private *i915, i915_reg_t reg)
+{
+	intel_uncore_posting_read_fw(&i915->uncore, reg);
+}
+
 static inline void
 intel_de_write_fw(struct drm_i915_private *i915, i915_reg_t reg, u32 val)
 {
-- 
2.39.2

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915/display: Restore dsparb_lock.

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/display: Restore dsparb_lock.
URL   : https://patchwork.freedesktop.org/series/115675/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/display: Restore dsparb_lock.

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 4063 bytes --]

== Series Details ==

Series: series starting with [1/2] drm/i915/display: Restore dsparb_lock.
URL   : https://patchwork.freedesktop.org/series/115675/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12921 -> Patchwork_115675v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/index.html

Participating hosts (37 -> 36)
------------------------------

  Missing    (1): fi-kbl-soraka 

Known issues
------------

  Here are the changes found in Patchwork_115675v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@execlists:
    - fi-bsw-n3050:       [PASS][1] -> [ABORT][2] ([i915#7911] / [i915#7913])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/fi-bsw-n3050/igt@i915_selftest@live@execlists.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/fi-bsw-n3050/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@requests:
    - bat-rpls-1:         [PASS][3] -> [ABORT][4] ([i915#7911] / [i915#7982])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/bat-rpls-1/igt@i915_selftest@live@requests.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/bat-rpls-1/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@slpc:
    - bat-rpls-2:         NOTRUN -> [DMESG-FAIL][5] ([i915#6367] / [i915#7913] / [i915#7996])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/bat-rpls-2/igt@i915_selftest@live@slpc.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - bat-rpls-2:         NOTRUN -> [SKIP][6] ([i915#7828])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/bat-rpls-2/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - bat-rpls-2:         NOTRUN -> [SKIP][7] ([i915#1845])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/bat-rpls-2/igt@kms_pipe_crc_basic@suspend-read-crc.html

  
#### Possible fixes ####

  * igt@i915_pm_rps@basic-api:
    - bat-dg2-11:         [FAIL][8] ([i915#8308]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/bat-dg2-11/igt@i915_pm_rps@basic-api.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/bat-dg2-11/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-2:         [ABORT][10] ([i915#4983] / [i915#7913]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/bat-rpls-2/igt@i915_selftest@live@reset.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/bat-rpls-2/igt@i915_selftest@live@reset.html

  
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7911]: https://gitlab.freedesktop.org/drm/intel/issues/7911
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7982]: https://gitlab.freedesktop.org/drm/intel/issues/7982
  [i915#7996]: https://gitlab.freedesktop.org/drm/intel/issues/7996
  [i915#8308]: https://gitlab.freedesktop.org/drm/intel/issues/8308


Build changes
-------------

  * Linux: CI_DRM_12921 -> Patchwork_115675v1

  CI-20190529: 20190529
  CI_DRM_12921: 3de6040ce9900a94ec626662d5c6a227b37eeb1c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7221: 4b77c6d85024d22ca521d510f8eee574128fe04f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_115675v1: 3de6040ce9900a94ec626662d5c6a227b37eeb1c @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

ce61ba3bddad drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.
fac81539ff60 drm/i915/display: Restore dsparb_lock.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/index.html

[-- Attachment #2: Type: text/html, Size: 5035 bytes --]

[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/display: Restore dsparb_lock.

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 12740 bytes --]

== Series Details ==

Series: series starting with [1/2] drm/i915/display: Restore dsparb_lock.
URL   : https://patchwork.freedesktop.org/series/115675/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12921_full -> Patchwork_115675v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts

Known issues
------------

  Here are the changes found in Patchwork_115675v1_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-glk:          [PASS][1] -> [FAIL][2] ([i915#2842]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-glk5/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-glk5/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - shard-glk:          NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-glk8/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@i915_pm_rps@reset:
    - shard-snb:          [PASS][4] -> [DMESG-FAIL][5] ([i915#8319])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-snb5/igt@i915_pm_rps@reset.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-snb7/igt@i915_pm_rps@reset.html

  * igt@kms_ccs@pipe-b-missing-ccs-buffer-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#3886]) +2 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-apl6/igt@kms_ccs@pipe-b-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-glk:          [PASS][7] -> [FAIL][8] ([i915#2346]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-glk7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-glk5/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-apl:          [PASS][9] -> [FAIL][10] ([i915#2346])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-apl3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-apl3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_flip@flip-vs-suspend-interruptible@b-dp1:
    - shard-apl:          [PASS][11] -> [ABORT][12] ([i915#180])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-apl1/igt@kms_flip@flip-vs-suspend-interruptible@b-dp1.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-apl2/igt@kms_flip@flip-vs-suspend-interruptible@b-dp1.html

  * igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-25@pipe-c-dp-1:
    - shard-apl:          NOTRUN -> [SKIP][13] ([fdo#109271]) +76 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-apl7/igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-25@pipe-c-dp-1.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area:
    - shard-apl:          NOTRUN -> [SKIP][14] ([fdo#109271] / [i915#658])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-apl7/igt@kms_psr2_sf@plane-move-sf-dmg-area.html

  * igt@vc4/vc4_perfmon@create-perfmon-exceed:
    - shard-glk:          NOTRUN -> [SKIP][15] ([fdo#109271]) +20 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-glk8/igt@vc4/vc4_perfmon@create-perfmon-exceed.html

  
#### Possible fixes ####

  * igt@gem_ctx_exec@basic-nohangcheck:
    - {shard-tglu}:       [FAIL][16] ([i915#6268]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-tglu-7/igt@gem_ctx_exec@basic-nohangcheck.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-tglu-7/igt@gem_ctx_exec@basic-nohangcheck.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - {shard-tglu}:       [FAIL][18] ([i915#2842]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-tglu-3/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-tglu-6/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-apl:          [ABORT][20] ([i915#5566]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-apl6/igt@gen9_exec_parse@allowed-all.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-apl7/igt@gen9_exec_parse@allowed-all.html

  * igt@i915_pm_rps@basic-api:
    - {shard-dg1}:        [FAIL][22] ([i915#8308]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-dg1-17/igt@i915_pm_rps@basic-api.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-dg1-15/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@gt_heartbeat:
    - shard-apl:          [DMESG-FAIL][24] ([i915#5334]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-apl2/igt@i915_selftest@live@gt_heartbeat.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-apl2/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes:
    - shard-apl:          [ABORT][26] ([i915#180]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-apl2/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes.html

  * {igt@perf@oa-exponents@0-rcs0}:
    - shard-glk:          [ABORT][28] ([i915#5213]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12921/shard-glk3/igt@perf@oa-exponents@0-rcs0.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/shard-glk8/igt@perf@oa-exponents@0-rcs0.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [IGT#2]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/2
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1755]: https://gitlab.freedesktop.org/drm/intel/issues/1755
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2876]: https://gitlab.freedesktop.org/drm/intel/issues/2876
  [i915#3063]: https://gitlab.freedesktop.org/drm/intel/issues/3063
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3318]: https://gitlab.freedesktop.org/drm/intel/issues/3318
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#433]: https://gitlab.freedesktop.org/drm/intel/issues/433
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4767]: https://gitlab.freedesktop.org/drm/intel/issues/4767
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4859]: https://gitlab.freedesktop.org/drm/intel/issues/4859
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#4879]: https://gitlab.freedesktop.org/drm/intel/issues/4879
  [i915#4880]: https://gitlab.freedesktop.org/drm/intel/issues/4880
  [i915#4884]: https://gitlab.freedesktop.org/drm/intel/issues/4884
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5213]: https://gitlab.freedesktop.org/drm/intel/issues/5213
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
  [i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
  [i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6493]: https://gitlab.freedesktop.org/drm/intel/issues/6493
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
  [i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7701]: https://gitlab.freedesktop.org/drm/intel/issues/7701
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
  [i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211
  [i915#8229]: https://gitlab.freedesktop.org/drm/intel/issues/8229
  [i915#8308]: https://gitlab.freedesktop.org/drm/intel/issues/8308
  [i915#8319]: https://gitlab.freedesktop.org/drm/intel/issues/8319


Build changes
-------------

  * Linux: CI_DRM_12921 -> Patchwork_115675v1

  CI-20190529: 20190529
  CI_DRM_12921: 3de6040ce9900a94ec626662d5c6a227b37eeb1c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7221: 4b77c6d85024d22ca521d510f8eee574128fe04f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_115675v1: 3de6040ce9900a94ec626662d5c6a227b37eeb1c @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115675v1/index.html

[-- Attachment #2: Type: text/html, Size: 9542 bytes --]

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Jani Nikula]

On Mon, 27 Mar 2023, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> uncore->lock only protects the forcewake domain itself,
> not the register accesses.
>
> uncore's _fw alternatives are for cases where the domains
> are not needed because we are sure that they are already
> awake.
>
> So the move towards the uncore's _fw alternatives seems
> right, however using the uncore-lock to protect the dsparb
> registers seems an abuse of the uncore-lock.
>
> Let's restore the previous individual lock and try to get
> rid of the direct uncore accesses from the display code.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20230308165859.235520-1-rodrigo.vivi@intel.com
> ---
>  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
>  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
>  drivers/gpu/drm/i915/i915_driver.c                |  1 +
>  3 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> index caef72d38798..8fe0b5c63d3a 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  
>  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
>  
> -	/*
> -	 * uncore.lock serves a double purpose here. It allows us to
> -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> -	 * it protects the DSPARB registers from getting clobbered by
> -	 * parallel updates from multiple pipes.
> -	 *
> -	 * intel_pipe_update_start() has already disabled interrupts
> -	 * for us, so a plain spin_lock() is sufficient here.
> -	 */
> -	spin_lock(&uncore->lock);
> +	spin_lock(&dev_priv->display.wm.dsparb_lock);
>  
>  	switch (crtc->pipe) {
>  	case PIPE_A:
> @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  
>  	intel_uncore_posting_read_fw(uncore, DSPARB);
>  
> -	spin_unlock(&uncore->lock);
> +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
>  }
>  
>  #undef VLV_FIFO
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 0b5509f268a7..e4da8902c878 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -264,6 +264,9 @@ struct intel_wm {
>  	 */
>  	struct mutex wm_mutex;
>  
> +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> +	spinlock_t dsparb_lock;
> +
>  	bool ipc_enabled;
>  };
>  
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 12b5296ee744..e90a0c0403a6 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>  	mutex_init(&dev_priv->display.pps.mutex);
>  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
>  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);

Can we do this in i9xx_wm_init() instead?


>  
>  	i915_memcpy_init_early(dev_priv);
>  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Restore dsparb_lock.

[Rodrigo Vivi]

On Tue, Mar 28, 2023 at 07:22:24PM +0300, Jani Nikula wrote:
> On Mon, 27 Mar 2023, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > uncore->lock only protects the forcewake domain itself,
> > not the register accesses.
> >
> > uncore's _fw alternatives are for cases where the domains
> > are not needed because we are sure that they are already
> > awake.
> >
> > So the move towards the uncore's _fw alternatives seems
> > right, however using the uncore-lock to protect the dsparb
> > registers seems an abuse of the uncore-lock.
> >
> > Let's restore the previous individual lock and try to get
> > rid of the direct uncore accesses from the display code.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Link: https://patchwork.freedesktop.org/patch/msgid/20230308165859.235520-1-rodrigo.vivi@intel.com
> > ---
> >  drivers/gpu/drm/i915/display/i9xx_wm.c            | 13 ++-----------
> >  drivers/gpu/drm/i915/display/intel_display_core.h |  3 +++
> >  drivers/gpu/drm/i915/i915_driver.c                |  1 +
> >  3 files changed, 6 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > index caef72d38798..8fe0b5c63d3a 100644
> > --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
> > +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
> > @@ -1771,16 +1771,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> >  
> >  	trace_vlv_fifo_size(crtc, sprite0_start, sprite1_start, fifo_size);
> >  
> > -	/*
> > -	 * uncore.lock serves a double purpose here. It allows us to
> > -	 * use the less expensive I915_{READ,WRITE}_FW() functions, and
> > -	 * it protects the DSPARB registers from getting clobbered by
> > -	 * parallel updates from multiple pipes.
> > -	 *
> > -	 * intel_pipe_update_start() has already disabled interrupts
> > -	 * for us, so a plain spin_lock() is sufficient here.
> > -	 */
> > -	spin_lock(&uncore->lock);
> > +	spin_lock(&dev_priv->display.wm.dsparb_lock);
> >  
> >  	switch (crtc->pipe) {
> >  	case PIPE_A:
> > @@ -1840,7 +1831,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> >  
> >  	intel_uncore_posting_read_fw(uncore, DSPARB);
> >  
> > -	spin_unlock(&uncore->lock);
> > +	spin_unlock(&dev_priv->display.wm.dsparb_lock);
> >  }
> >  
> >  #undef VLV_FIFO
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> > index 0b5509f268a7..e4da8902c878 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> > @@ -264,6 +264,9 @@ struct intel_wm {
> >  	 */
> >  	struct mutex wm_mutex;
> >  
> > +	/* protects DSPARB registers on pre-g4x/vlv/chv */
> > +	spinlock_t dsparb_lock;
> > +
> >  	bool ipc_enabled;
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> > index 12b5296ee744..e90a0c0403a6 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -223,6 +223,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
> >  	mutex_init(&dev_priv->display.pps.mutex);
> >  	mutex_init(&dev_priv->display.hdcp.comp_mutex);
> >  	spin_lock_init(&dev_priv->display.dkl.phy_lock);
> > +	spin_lock_init(&dev_priv->display.wm.dsparb_lock);
> 
> Can we do this in i9xx_wm_init() instead?

I was going to modify it here right now, but then I noticed
the cases above and remembered why I have put it here.
All the display locks are getting set in here.

Probably better to move with this patch as is and then add
a new on top moving the various locks to its individual inits?

> 
> 
> >  
> >  	i915_memcpy_init_early(dev_priv);
> >  	intel_runtime_pm_init_early(&dev_priv->runtime_pm);
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center