[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMeQTsa=SWXHt8ZvToa9x5qc6W29B6B4Ssvixs3nd-w=+dYGzA@mail.gmail.com>
Date: Wed, 6 Nov 2019 16:24:59 +0100
From: Patrik Jakobsson <patrik.r.jakobsson@...il.com>
To: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
Cc: dri-devel <dri-devel@...ts.freedesktop.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>,
James Hilliard <james.hilliard1@...il.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH 2/2] drm/gma500: Add page flip support on psb/cdv
On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
<paul.kocialkowski@...tlin.com> wrote:
>
> Legacy (non-atomic) page flip support is added to the driver by using the
> mode_set_base CRTC function, that allows configuring a new framebuffer for
> display. Since the function requires the primary plane's fb to be set
> already, this is done prior to calling the function in the page flip helper
> and reverted if the flip fails.
>
> The vblank interrupt handler is also refactored to support passing an event.
> The PIPE_TE_STATUS bit is also considered to indicate vblank on medfield
> only, as explained in psb_enable_vblank.
>
> It was tested by running weston on both poulsbo and cedartrail.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
> ---
> drivers/gpu/drm/gma500/cdv_intel_display.c | 1 +
> drivers/gpu/drm/gma500/gma_display.c | 46 ++++++++++++++++++++++
> drivers/gpu/drm/gma500/gma_display.h | 6 +++
> drivers/gpu/drm/gma500/psb_intel_display.c | 1 +
> drivers/gpu/drm/gma500/psb_intel_drv.h | 3 ++
> drivers/gpu/drm/gma500/psb_irq.c | 18 +++++++--
> 6 files changed, 72 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c b/drivers/gpu/drm/gma500/cdv_intel_display.c
> index 8b784947ed3b..7109d3d19be0 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_display.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_display.c
> @@ -979,6 +979,7 @@ const struct drm_crtc_funcs cdv_intel_crtc_funcs = {
> .gamma_set = gma_crtc_gamma_set,
> .set_config = gma_crtc_set_config,
> .destroy = gma_crtc_destroy,
> + .page_flip = gma_crtc_page_flip,
> };
>
> const struct gma_clock_funcs cdv_clock_funcs = {
> diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
> index bc07ae2a9a1d..17f136985d21 100644
> --- a/drivers/gpu/drm/gma500/gma_display.c
> +++ b/drivers/gpu/drm/gma500/gma_display.c
> @@ -503,6 +503,52 @@ void gma_crtc_destroy(struct drm_crtc *crtc)
> kfree(gma_crtc);
> }
>
> +int gma_crtc_page_flip(struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + struct drm_pending_vblank_event *event,
> + uint32_t page_flip_flags,
> + struct drm_modeset_acquire_ctx *ctx)
> +{
> + struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
> + struct drm_framebuffer *current_fb = crtc->primary->fb;
> + struct drm_framebuffer *old_fb = crtc->primary->old_fb;
> + const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
> + struct drm_device *dev = crtc->dev;
> + unsigned long flags;
> + int ret;
> +
> + if (!crtc_funcs->mode_set_base)
> + return -EINVAL;
> +
> + /* Using mode_set_base requires the new fb to be set already. */
> + crtc->primary->fb = fb;
> +
> + if (event) {
> + spin_lock_irqsave(&dev->event_lock, flags);
> +
> + WARN_ON(drm_crtc_vblank_get(crtc) != 0);
> +
> + gma_crtc->page_flip_event = event;
> +
> + /* Call this locked if we want an event at vblank interrupt. */
> + ret = crtc_funcs->mode_set_base(crtc, crtc->x, crtc->y, old_fb);
> + if (ret) {
> + gma_crtc->page_flip_event = NULL;
> + drm_crtc_vblank_put(crtc);
> + }
> +
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + } else {
> + ret = crtc_funcs->mode_set_base(crtc, crtc->x, crtc->y, old_fb);
> + }
> +
> + /* Restore previous fb in case of failure. */
> + if (ret)
> + crtc->primary->fb = current_fb;
> +
> + return ret;
> +}
> +
> int gma_crtc_set_config(struct drm_mode_set *set,
> struct drm_modeset_acquire_ctx *ctx)
> {
> diff --git a/drivers/gpu/drm/gma500/gma_display.h b/drivers/gpu/drm/gma500/gma_display.h
> index fdbd7ecaa59c..7bd6c1ee8b21 100644
> --- a/drivers/gpu/drm/gma500/gma_display.h
> +++ b/drivers/gpu/drm/gma500/gma_display.h
> @@ -11,6 +11,7 @@
> #define _GMA_DISPLAY_H_
>
> #include <linux/pm_runtime.h>
> +#include <drm/drm_vblank.h>
>
> struct drm_encoder;
> struct drm_mode_set;
> @@ -71,6 +72,11 @@ extern void gma_crtc_prepare(struct drm_crtc *crtc);
> extern void gma_crtc_commit(struct drm_crtc *crtc);
> extern void gma_crtc_disable(struct drm_crtc *crtc);
> extern void gma_crtc_destroy(struct drm_crtc *crtc);
> +extern int gma_crtc_page_flip(struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + struct drm_pending_vblank_event *event,
> + uint32_t page_flip_flags,
> + struct drm_modeset_acquire_ctx *ctx);
> extern int gma_crtc_set_config(struct drm_mode_set *set,
> struct drm_modeset_acquire_ctx *ctx);
>
> diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
> index 4256410535f0..fed3b563e62e 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_display.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_display.c
> @@ -432,6 +432,7 @@ const struct drm_crtc_funcs psb_intel_crtc_funcs = {
> .gamma_set = gma_crtc_gamma_set,
> .set_config = gma_crtc_set_config,
> .destroy = gma_crtc_destroy,
> + .page_flip = gma_crtc_page_flip,
> };
>
> const struct gma_clock_funcs psb_clock_funcs = {
> diff --git a/drivers/gpu/drm/gma500/psb_intel_drv.h b/drivers/gpu/drm/gma500/psb_intel_drv.h
> index cdf10333d1c2..16c6136f778b 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_drv.h
> +++ b/drivers/gpu/drm/gma500/psb_intel_drv.h
> @@ -12,6 +12,7 @@
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_encoder.h>
> #include <drm/drm_probe_helper.h>
> +#include <drm/drm_vblank.h>
> #include <linux/gpio.h>
> #include "gma_display.h"
>
> @@ -182,6 +183,8 @@ struct gma_crtc {
> struct psb_intel_crtc_state *crtc_state;
>
> const struct gma_clock_funcs *clock_funcs;
> +
> + struct drm_pending_vblank_event *page_flip_event;
> };
>
> #define to_gma_crtc(x) \
> diff --git a/drivers/gpu/drm/gma500/psb_irq.c b/drivers/gpu/drm/gma500/psb_irq.c
> index e6265fb85626..f787a51f6335 100644
> --- a/drivers/gpu/drm/gma500/psb_irq.c
> +++ b/drivers/gpu/drm/gma500/psb_irq.c
> @@ -165,11 +165,23 @@ static void mid_pipe_event_handler(struct drm_device *dev, int pipe)
> "%s, can't clear status bits for pipe %d, its value = 0x%x.\n",
> __func__, pipe, PSB_RVDC32(pipe_stat_reg));
>
> - if (pipe_stat_val & PIPE_VBLANK_STATUS)
> - drm_handle_vblank(dev, pipe);
> + if (pipe_stat_val & PIPE_VBLANK_STATUS ||
> + (IS_MFLD(dev) && pipe_stat_val & PIPE_TE_STATUS)) {
> + struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
> + struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
> + unsigned long flags;
>
> - if (pipe_stat_val & PIPE_TE_STATUS)
> drm_handle_vblank(dev, pipe);
> +
> + spin_lock_irqsave(&dev->event_lock, flags);
> + if (gma_crtc->page_flip_event) {
> + drm_crtc_send_vblank_event(crtc,
> + gma_crtc->page_flip_event);
> + gma_crtc->page_flip_event = NULL;
> + drm_crtc_vblank_put(crtc);
> + }
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + }
> }
>
> /*
> --
> 2.23.0
>
Looks good!
Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@...il.com>
Powered by blists - more mailing lists