Em seg, 2020-07-20 às 17:01 +0530, Karthik B S escreveu:
> Set the Async Address Update Enable bit in plane ctl
> when async flip is requested.
>
> v2: -Move the Async flip enablement to individual patch (Paulo)
>
> v3: -Rebased.
>
> v4: -Add separate plane hook for async flip case (Ville)
>
> v5: -Rebased.
>
> Signed-off-by: Karthik B S <karthik.b.s@xxxxxxxxx>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 6 +++++
> drivers/gpu/drm/i915/display/intel_sprite.c | 25 ++++++++++++++++++++
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> 3 files changed, 32 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index b8ff032195d9..4773f39e5924 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4766,6 +4766,12 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> u32 plane_ctl;
>
> + /* During Async flip, no other updates are allowed */
My understanding is that this function is fully setting the right bits
based on the chosen config (instead of doing read-modify-write), and
the checks for "other updates" were done before. So the logic
implemented here of early returning doesn't make sense.
> + if (crtc_state->uapi.async_flip) {
> + plane_ctl |= PLANE_CTL_ASYNC_FLIP;
I wonder why gcc does not complain we're ORing with an unitialized
value.
> + return plane_ctl;
> + }
> +
> plane_ctl = PLANE_CTL_ENABLE;
It seems to be the return above means we'll never even try to enable
the plane, we're only relying on the fact that plane_ctl is not zero
initialize so maybe bit 31 is already set.
>
> if (INTEL_GEN(dev_priv) < 10 && !IS_GEMINILAKE(dev_priv)) {
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index c26ca029fc0a..3747482e8fa3 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -603,6 +603,24 @@ icl_program_input_csc(struct intel_plane *plane,
> PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 2), 0x0);
> }
>
> +static void
> +skl_program_async_surface_address(struct drm_i915_private *dev_priv,
> + const struct intel_plane_state *plane_state,
> + enum pipe pipe, enum plane_id plane_id,
> + u32 surf_addr)
> +{
> + unsigned long irqflags;
> + u32 plane_ctl = plane_state->ctl;
> +
> + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> +
> + intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl);
> + intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id),
> + intel_plane_ggtt_offset(plane_state) + surf_addr);
> +
> + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> +}
> +
> static void
> skl_program_plane(struct intel_plane *plane,
> const struct intel_crtc_state *crtc_state,
> @@ -631,6 +649,13 @@ skl_program_plane(struct intel_plane *plane,
> u32 keymsk, keymax;
> u32 plane_ctl = plane_state->ctl;
>
> + /* During Async flip, no other updates are allowed */
> + if (crtc_state->uapi.async_flip) {
> + skl_program_async_surface_address(dev_priv, plane_state,
> + pipe, plane_id, surf_addr);
> + return;
> + }
I'd vote for us to keep the "don't rewrite registers that shouldn't
change" part on its own commit, since it's just an optimization. It
could even go at the end of the series. But perhaps this is simple
enough and not needed.
> +
> plane_ctl |= skl_plane_ctl_crtc(crtc_state);
>
> if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8cee06314d5d..19aad4199874 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6935,6 +6935,7 @@ enum {
> #define PLANE_CTL_TILED_X (1 << 10)
> #define PLANE_CTL_TILED_Y (4 << 10)
> #define PLANE_CTL_TILED_YF (5 << 10)
> +#define PLANE_CTL_ASYNC_FLIP (1 << 9)
> #define PLANE_CTL_FLIP_HORIZONTAL (1 << 8)
> #define PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE (1 << 4) /* TGL+ */
> #define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
