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
