Re: [PATCH 3/7] drm/i915: Have plane->get_hw_state() return the current pipe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2018-01-30 at 22:38 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> Like we do for encoder let's make the plane->get_hw_state() return
> the pipe to which the plane is currently attached. We don't currently
> allow planes to move between the pipes, but perhaps one day we will.
> 
> In either case this makes the code more uniform and perhaps makes
> intel_plane_mapping_ok() slightly more clear.
> 
> Note that for i965 and g4x planes A and B still have pipe select bits
> but they're hardwired to pipe A and B respectively. This means we can
> safely interpret those bits just like on gen2/3. This allows the
> same readout code work for plane C (which can still be assigned
> to eiter pipe on i965) should we ever expose it.
> 
> g4x no longer allows moving the cursor planes between the pipes,
> but the pipe select bits can still be set in the register. Thus
> we have to ignore those bits. OTOH i965 still allows the cursors
> to move between pipes thus we have to trust the bits there.
> 
Reviewed-by: Mika Kahola <mika.kahola@xxxxxxxxx>

> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  2 +
>  drivers/gpu/drm/i915/intel_display.c | 71
> ++++++++++++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_drv.h     |  4 +-
>  drivers/gpu/drm/i915/intel_sprite.c  | 40 ++++++++++++--------
>  4 files changed, 79 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 64933fd74cb6..ebb41f279134 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5908,6 +5908,8 @@ enum {
>  #define   CURSOR_MODE_128_ARGB_AX ((1 << 5) |
> CURSOR_MODE_128_32B_AX)
>  #define   CURSOR_MODE_256_ARGB_AX ((1 << 5) |
> CURSOR_MODE_256_32B_AX)
>  #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
> +#define   MCURSOR_PIPE_SELECT_MASK	(0x3 << 28)
> +#define   MCURSOR_PIPE_SELECT_SHIFT	28
>  #define   MCURSOR_PIPE_SELECT(pipe)	((pipe) << 28)
>  #define   MCURSOR_GAMMA_ENABLE  (1 << 26)
>  #define   CURSOR_ROTATE_180	(1<<15)
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 6ffc1d088d7a..feae6bd51a44 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1224,7 +1224,10 @@ void assert_pipe(struct drm_i915_private
> *dev_priv,
>  
>  static void assert_plane(struct intel_plane *plane, bool state)
>  {
> -	bool cur_state = plane->get_hw_state(plane);
> +	enum pipe pipe;
> +	bool cur_state;
> +
> +	cur_state = plane->get_hw_state(plane, &pipe);
>  
>  	I915_STATE_WARN(cur_state != state,
>  			"%s assertion failure (expected %s, current
> %s)\n",
> @@ -3326,24 +3329,33 @@ static void i9xx_disable_plane(struct
> intel_plane *plane,
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  }
>  
> -static bool i9xx_plane_get_hw_state(struct intel_plane *plane)
> +static bool i9xx_plane_get_hw_state(struct intel_plane *plane,
> +				    enum pipe *pipe)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane-
> >base.dev);
>  	enum intel_display_power_domain power_domain;
>  	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
> -	enum pipe pipe = plane->pipe;
>  	bool ret;
> +	u32 val;
>  
>  	/*
>  	 * Not 100% correct for planes that can move between pipes,
>  	 * but that's only the case for gen2-4 which don't have any
>  	 * display power wells.
>  	 */
> -	power_domain = POWER_DOMAIN_PIPE(pipe);
> +	power_domain = POWER_DOMAIN_PIPE(plane->pipe);
>  	if (!intel_display_power_get_if_enabled(dev_priv,
> power_domain))
>  		return false;
>  
> -	ret = I915_READ(DSPCNTR(i9xx_plane)) & DISPLAY_PLANE_ENABLE;
> +	val = I915_READ(DSPCNTR(i9xx_plane));
> +
> +	ret = val & DISPLAY_PLANE_ENABLE;
> +
> +	if (INTEL_GEN(dev_priv) >= 5)
> +		*pipe = plane->pipe;
> +	else
> +		*pipe = (val & DISPPLANE_SEL_PIPE_MASK) >>
> +			DISPPLANE_SEL_PIPE_SHIFT;
>  
>  	intel_display_power_put(dev_priv, power_domain);
>  
> @@ -7482,16 +7494,18 @@ i9xx_get_initial_plane_config(struct
> intel_crtc *crtc,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_plane *plane = to_intel_plane(crtc-
> >base.primary);
>  	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
> -	enum pipe pipe = crtc->pipe;
> +	enum pipe pipe;
>  	u32 val, base, offset;
>  	int fourcc, pixel_format;
>  	unsigned int aligned_height;
>  	struct drm_framebuffer *fb;
>  	struct intel_framebuffer *intel_fb;
>  
> -	if (!plane->get_hw_state(plane))
> +	if (!plane->get_hw_state(plane, &pipe))
>  		return;
>  
> +	WARN_ON(pipe != crtc->pipe);
> +
>  	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
>  	if (!intel_fb) {
>  		DRM_DEBUG_KMS("failed to alloc fb\n");
> @@ -8512,16 +8526,18 @@ skylake_get_initial_plane_config(struct
> intel_crtc *crtc,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_plane *plane = to_intel_plane(crtc-
> >base.primary);
>  	enum plane_id plane_id = plane->id;
> -	enum pipe pipe = crtc->pipe;
> +	enum pipe pipe;
>  	u32 val, base, offset, stride_mult, tiling, alpha;
>  	int fourcc, pixel_format;
>  	unsigned int aligned_height;
>  	struct drm_framebuffer *fb;
>  	struct intel_framebuffer *intel_fb;
>  
> -	if (!plane->get_hw_state(plane))
> +	if (!plane->get_hw_state(plane, &pipe))
>  		return;
>  
> +	WARN_ON(pipe != crtc->pipe);
> +
>  	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
>  	if (!intel_fb) {
>  		DRM_DEBUG_KMS("failed to alloc fb\n");
> @@ -9502,7 +9518,8 @@ static void i845_disable_cursor(struct
> intel_plane *plane,
>  	i845_update_cursor(plane, NULL, NULL);
>  }
>  
> -static bool i845_cursor_get_hw_state(struct intel_plane *plane)
> +static bool i845_cursor_get_hw_state(struct intel_plane *plane,
> +				     enum pipe *pipe)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane-
> >base.dev);
>  	enum intel_display_power_domain power_domain;
> @@ -9514,6 +9531,8 @@ static bool i845_cursor_get_hw_state(struct
> intel_plane *plane)
>  
>  	ret = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE;
>  
> +	*pipe = PIPE_A;
> +
>  	intel_display_power_put(dev_priv, power_domain);
>  
>  	return ret;
> @@ -9713,23 +9732,32 @@ static void i9xx_disable_cursor(struct
> intel_plane *plane,
>  	i9xx_update_cursor(plane, NULL, NULL);
>  }
>  
> -static bool i9xx_cursor_get_hw_state(struct intel_plane *plane)
> +static bool i9xx_cursor_get_hw_state(struct intel_plane *plane,
> +				     enum pipe *pipe)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane-
> >base.dev);
>  	enum intel_display_power_domain power_domain;
> -	enum pipe pipe = plane->pipe;
>  	bool ret;
> +	u32 val;
>  
>  	/*
>  	 * Not 100% correct for planes that can move between pipes,
>  	 * but that's only the case for gen2-3 which don't have any
>  	 * display power wells.
>  	 */
> -	power_domain = POWER_DOMAIN_PIPE(pipe);
> +	power_domain = POWER_DOMAIN_PIPE(plane->pipe);
>  	if (!intel_display_power_get_if_enabled(dev_priv,
> power_domain))
>  		return false;
>  
> -	ret = I915_READ(CURCNTR(pipe)) & CURSOR_MODE;
> +	val = I915_READ(CURCNTR(plane->pipe));
> +
> +	ret = val & CURSOR_MODE;
> +
> +	if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv))
> +		*pipe = plane->pipe;
> +	else
> +		*pipe = (val & MCURSOR_PIPE_SELECT_MASK) >>
> +			MCURSOR_PIPE_SELECT_SHIFT;
>  
>  	intel_display_power_put(dev_priv, power_domain);
>  
> @@ -14755,12 +14783,12 @@ void i830_disable_pipe(struct
> drm_i915_private *dev_priv, enum pipe pipe)
>  static bool intel_plane_mapping_ok(struct intel_crtc *crtc,
>  				   struct intel_plane *plane)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
> -	u32 val = I915_READ(DSPCNTR(i9xx_plane));
> +	enum pipe pipe;
>  
> -	return (val & DISPLAY_PLANE_ENABLE) == 0 ||
> -		(val & DISPPLANE_SEL_PIPE_MASK) ==
> DISPPLANE_SEL_PIPE(crtc->pipe);
> +	if (!plane->get_hw_state(plane, &pipe))
> +		return true;
> +
> +	return pipe == crtc->pipe;
>  }
>  
>  static void
> @@ -14959,7 +14987,10 @@ static void readout_plane_state(struct
> intel_crtc *crtc)
>  	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
>  		struct intel_plane_state *plane_state =
>  			to_intel_plane_state(plane->base.state);
> -		bool visible = plane->get_hw_state(plane);
> +		enum pipe pipe;
> +		bool visible;
> +
> +		visible = plane->get_hw_state(plane, &pipe);
>  
>  		intel_set_plane_visible(crtc_state, plane_state,
> visible);
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 8335d27f4156..d80eae7a69ba 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -946,7 +946,7 @@ struct intel_plane {
>  			     const struct intel_plane_state
> *plane_state);
>  	void (*disable_plane)(struct intel_plane *plane,
>  			      struct intel_crtc *crtc);
> -	bool (*get_hw_state)(struct intel_plane *plane);
> +	bool (*get_hw_state)(struct intel_plane *plane, enum pipe
> *pipe);
>  	int (*check_plane)(struct intel_plane *plane,
>  			   struct intel_crtc_state *crtc_state,
>  			   struct intel_plane_state *state);
> @@ -2023,7 +2023,7 @@ void skl_update_plane(struct intel_plane
> *plane,
>  		      const struct intel_crtc_state *crtc_state,
>  		      const struct intel_plane_state *plane_state);
>  void skl_disable_plane(struct intel_plane *plane, struct intel_crtc
> *crtc);
> -bool skl_plane_get_hw_state(struct intel_plane *plane);
> +bool skl_plane_get_hw_state(struct intel_plane *plane, enum pipe
> *pipe);
>  bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
>  		       enum pipe pipe, enum plane_id plane_id);
>  
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index aea21a9abf6c..86a31535fce3 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -326,19 +326,21 @@ skl_disable_plane(struct intel_plane *plane,
> struct intel_crtc *crtc)
>  }
>  
>  bool
> -skl_plane_get_hw_state(struct intel_plane *plane)
> +skl_plane_get_hw_state(struct intel_plane *plane,
> +		       enum pipe *pipe)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane-
> >base.dev);
>  	enum intel_display_power_domain power_domain;
>  	enum plane_id plane_id = plane->id;
> -	enum pipe pipe = plane->pipe;
>  	bool ret;
>  
> -	power_domain = POWER_DOMAIN_PIPE(pipe);
> +	power_domain = POWER_DOMAIN_PIPE(plane->pipe);
>  	if (!intel_display_power_get_if_enabled(dev_priv,
> power_domain))
>  		return false;
>  
> -	ret = I915_READ(PLANE_CTL(pipe, plane_id)) &
> PLANE_CTL_ENABLE;
> +	ret = I915_READ(PLANE_CTL(plane->pipe, plane_id)) &
> PLANE_CTL_ENABLE;
> +
> +	*pipe = plane->pipe;
>  
>  	intel_display_power_put(dev_priv, power_domain);
>  
> @@ -523,19 +525,21 @@ vlv_disable_plane(struct intel_plane *plane,
> struct intel_crtc *crtc)
>  }
>  
>  static bool
> -vlv_plane_get_hw_state(struct intel_plane *plane)
> +vlv_plane_get_hw_state(struct intel_plane *plane,
> +		       enum pipe *pipe)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane-
> >base.dev);
>  	enum intel_display_power_domain power_domain;
>  	enum plane_id plane_id = plane->id;
> -	enum pipe pipe = plane->pipe;
>  	bool ret;
>  
> -	power_domain = POWER_DOMAIN_PIPE(pipe);
> +	power_domain = POWER_DOMAIN_PIPE(plane->pipe);
>  	if (!intel_display_power_get_if_enabled(dev_priv,
> power_domain))
>  		return false;
>  
> -	ret = I915_READ(SPCNTR(pipe, plane_id)) & SP_ENABLE;
> +	ret = I915_READ(SPCNTR(plane->pipe, plane_id)) & SP_ENABLE;
> +
> +	*pipe = plane->pipe;
>  
>  	intel_display_power_put(dev_priv, power_domain);
>  
> @@ -683,18 +687,20 @@ ivb_disable_plane(struct intel_plane *plane,
> struct intel_crtc *crtc)
>  }
>  
>  static bool
> -ivb_plane_get_hw_state(struct intel_plane *plane)
> +ivb_plane_get_hw_state(struct intel_plane *plane,
> +		       enum pipe *pipe)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane-
> >base.dev);
>  	enum intel_display_power_domain power_domain;
> -	enum pipe pipe = plane->pipe;
>  	bool ret;
>  
> -	power_domain = POWER_DOMAIN_PIPE(pipe);
> +	power_domain = POWER_DOMAIN_PIPE(plane->pipe);
>  	if (!intel_display_power_get_if_enabled(dev_priv,
> power_domain))
>  		return false;
>  
> -	ret =  I915_READ(SPRCTL(pipe)) & SPRITE_ENABLE;
> +	ret =  I915_READ(SPRCTL(plane->pipe)) & SPRITE_ENABLE;
> +
> +	*pipe = plane->pipe;
>  
>  	intel_display_power_put(dev_priv, power_domain);
>  
> @@ -833,18 +839,20 @@ g4x_disable_plane(struct intel_plane *plane,
> struct intel_crtc *crtc)
>  }
>  
>  static bool
> -g4x_plane_get_hw_state(struct intel_plane *plane)
> +g4x_plane_get_hw_state(struct intel_plane *plane,
> +		       enum pipe *pipe)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane-
> >base.dev);
>  	enum intel_display_power_domain power_domain;
> -	enum pipe pipe = plane->pipe;
>  	bool ret;
>  
> -	power_domain = POWER_DOMAIN_PIPE(pipe);
> +	power_domain = POWER_DOMAIN_PIPE(plane->pipe);
>  	if (!intel_display_power_get_if_enabled(dev_priv,
> power_domain))
>  		return false;
>  
> -	ret = I915_READ(DVSCNTR(pipe)) & DVS_ENABLE;
> +	ret = I915_READ(DVSCNTR(plane->pipe)) & DVS_ENABLE;
> +
> +	*pipe = plane->pipe;
>  
>  	intel_display_power_put(dev_priv, power_domain);
>  
-- 
Mika Kahola - Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux