[PATCH 0/2] drm/gma500: Add page flip support on psb/cdv

[PATCH 0/2] drm/gma500: Add page flip support on psb/cdv

[Paul Kocialkowski]

This series brings-in the required bits to implement page flip support
on poulsbo and cedartrail. Page flip support is required to run weston
with the GMA500 driver.

This is only legacy page flip support, not a conversion of the driver to
atomic DRM.

Paul Kocialkowski (2):
  drm/gma500: Add missing call to allow enabling vblank on psb/cdv
  drm/gma500: Add page flip support on psb/cdv

 drivers/gpu/drm/gma500/cdv_intel_display.c |  1 +
 drivers/gpu/drm/gma500/gma_display.c       | 48 ++++++++++++++++++++++
 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, 74 insertions(+), 3 deletions(-)

-- 
2.23.0

[PATCH 0/2] drm/gma500: Add page flip support on psb/cdv

[Paul Kocialkowski]

This series brings-in the required bits to implement page flip support
on poulsbo and cedartrail. Page flip support is required to run weston
with the GMA500 driver.

This is only legacy page flip support, not a conversion of the driver to
atomic DRM.

Paul Kocialkowski (2):
  drm/gma500: Add missing call to allow enabling vblank on psb/cdv
  drm/gma500: Add page flip support on psb/cdv

 drivers/gpu/drm/gma500/cdv_intel_display.c |  1 +
 drivers/gpu/drm/gma500/gma_display.c       | 48 ++++++++++++++++++++++
 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, 74 insertions(+), 3 deletions(-)

-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/gma500: Add missing call to allow enabling vblank on psb/cdv

[Paul Kocialkowski]

This adds a missing call to drm_crtc_vblank_on to the common DPMS helper
(used by poulsbo and cedartrail), which is called in the CRTC enable path.

With that call, it becomes possible to enable vblank when needed.
It is already balanced by a drm_crtc_vblank_off call in the helper.

Other platforms (oaktrail and medfield) use a dedicated DPMS helper that
does not have the proper vblank on/off hooks. They are not added in this
commit due to lack of hardware to test it with.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/gma500/gma_display.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index e20ccb5d10fd..bc07ae2a9a1d 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -255,6 +255,8 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode)
 		/* Give the overlay scaler a chance to enable
 		 * if it's on this pipe */
 		/* psb_intel_crtc_dpms_video(crtc, true); TODO */
+
+		drm_crtc_vblank_on(crtc);
 		break;
 	case DRM_MODE_DPMS_OFF:
 		if (!gma_crtc->active)
-- 
2.23.0

[PATCH 1/2] drm/gma500: Add missing call to allow enabling vblank on psb/cdv

[Paul Kocialkowski]

This adds a missing call to drm_crtc_vblank_on to the common DPMS helper
(used by poulsbo and cedartrail), which is called in the CRTC enable path.

With that call, it becomes possible to enable vblank when needed.
It is already balanced by a drm_crtc_vblank_off call in the helper.

Other platforms (oaktrail and medfield) use a dedicated DPMS helper that
does not have the proper vblank on/off hooks. They are not added in this
commit due to lack of hardware to test it with.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/gma500/gma_display.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index e20ccb5d10fd..bc07ae2a9a1d 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -255,6 +255,8 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode)
 		/* Give the overlay scaler a chance to enable
 		 * if it's on this pipe */
 		/* psb_intel_crtc_dpms_video(crtc, true); TODO */
+
+		drm_crtc_vblank_on(crtc);
 		break;
 	case DRM_MODE_DPMS_OFF:
 		if (!gma_crtc->active)
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/gma500: Add missing call to allow enabling vblank on psb/cdv

[Patrik Jakobsson]

On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> This adds a missing call to drm_crtc_vblank_on to the common DPMS helper
> (used by poulsbo and cedartrail), which is called in the CRTC enable path.
>
> With that call, it becomes possible to enable vblank when needed.
> It is already balanced by a drm_crtc_vblank_off call in the helper.
>
> Other platforms (oaktrail and medfield) use a dedicated DPMS helper that
> does not have the proper vblank on/off hooks. They are not added in this
> commit due to lack of hardware to test it with.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Don't think we ever found a need for having vblanks enabled... until
now. I'll have a look if something can be done for Oaktrail since I
have hw.

Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

> ---
>  drivers/gpu/drm/gma500/gma_display.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
> index e20ccb5d10fd..bc07ae2a9a1d 100644
> --- a/drivers/gpu/drm/gma500/gma_display.c
> +++ b/drivers/gpu/drm/gma500/gma_display.c
> @@ -255,6 +255,8 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode)
>                 /* Give the overlay scaler a chance to enable
>                  * if it's on this pipe */
>                 /* psb_intel_crtc_dpms_video(crtc, true); TODO */
> +
> +               drm_crtc_vblank_on(crtc);
>                 break;
>         case DRM_MODE_DPMS_OFF:
>                 if (!gma_crtc->active)
> --
> 2.23.0
>

Re: [PATCH 1/2] drm/gma500: Add missing call to allow enabling vblank on psb/cdv

[Patrik Jakobsson]

On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> This adds a missing call to drm_crtc_vblank_on to the common DPMS helper
> (used by poulsbo and cedartrail), which is called in the CRTC enable path.
>
> With that call, it becomes possible to enable vblank when needed.
> It is already balanced by a drm_crtc_vblank_off call in the helper.
>
> Other platforms (oaktrail and medfield) use a dedicated DPMS helper that
> does not have the proper vblank on/off hooks. They are not added in this
> commit due to lack of hardware to test it with.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Don't think we ever found a need for having vblanks enabled... until
now. I'll have a look if something can be done for Oaktrail since I
have hw.

Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

> ---
>  drivers/gpu/drm/gma500/gma_display.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
> index e20ccb5d10fd..bc07ae2a9a1d 100644
> --- a/drivers/gpu/drm/gma500/gma_display.c
> +++ b/drivers/gpu/drm/gma500/gma_display.c
> @@ -255,6 +255,8 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode)
>                 /* Give the overlay scaler a chance to enable
>                  * if it's on this pipe */
>                 /* psb_intel_crtc_dpms_video(crtc, true); TODO */
> +
> +               drm_crtc_vblank_on(crtc);
>                 break;
>         case DRM_MODE_DPMS_OFF:
>                 if (!gma_crtc->active)
> --
> 2.23.0
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/gma500: Add missing call to allow enabling vblank on psb/cdv

[Paul Kocialkowski]

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

Hi,

On Wed 06 Nov 19, 16:23, Patrik Jakobsson wrote:
> On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> >
> > This adds a missing call to drm_crtc_vblank_on to the common DPMS helper
> > (used by poulsbo and cedartrail), which is called in the CRTC enable path.
> >
> > With that call, it becomes possible to enable vblank when needed.
> > It is already balanced by a drm_crtc_vblank_off call in the helper.
> >
> > Other platforms (oaktrail and medfield) use a dedicated DPMS helper that
> > does not have the proper vblank on/off hooks. They are not added in this
> > commit due to lack of hardware to test it with.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> 
> Don't think we ever found a need for having vblanks enabled... until
> now. I'll have a look if something can be done for Oaktrail since I
> have hw.

Neat, thanks!

IIRC the DPMS paths that don't use gma_crtc_dpms also lack the proper
drm_crtc_vblank_on/off calls so that's probably something to start with :)

Thanks for the review on these patches. I may have more fixes coming up.

Cheers,

Paul

> Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> 
> > ---
> >  drivers/gpu/drm/gma500/gma_display.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
> > index e20ccb5d10fd..bc07ae2a9a1d 100644
> > --- a/drivers/gpu/drm/gma500/gma_display.c
> > +++ b/drivers/gpu/drm/gma500/gma_display.c
> > @@ -255,6 +255,8 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode)
> >                 /* Give the overlay scaler a chance to enable
> >                  * if it's on this pipe */
> >                 /* psb_intel_crtc_dpms_video(crtc, true); TODO */
> > +
> > +               drm_crtc_vblank_on(crtc);
> >                 break;
> >         case DRM_MODE_DPMS_OFF:
> >                 if (!gma_crtc->active)
> > --
> > 2.23.0
> >

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] drm/gma500: Add missing call to allow enabling vblank on psb/cdv

[Paul Kocialkowski]

[-- Attachment #1.1: Type: text/plain, Size: 2126 bytes --]

Hi,

On Wed 06 Nov 19, 16:23, Patrik Jakobsson wrote:
> On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> >
> > This adds a missing call to drm_crtc_vblank_on to the common DPMS helper
> > (used by poulsbo and cedartrail), which is called in the CRTC enable path.
> >
> > With that call, it becomes possible to enable vblank when needed.
> > It is already balanced by a drm_crtc_vblank_off call in the helper.
> >
> > Other platforms (oaktrail and medfield) use a dedicated DPMS helper that
> > does not have the proper vblank on/off hooks. They are not added in this
> > commit due to lack of hardware to test it with.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> 
> Don't think we ever found a need for having vblanks enabled... until
> now. I'll have a look if something can be done for Oaktrail since I
> have hw.

Neat, thanks!

IIRC the DPMS paths that don't use gma_crtc_dpms also lack the proper
drm_crtc_vblank_on/off calls so that's probably something to start with :)

Thanks for the review on these patches. I may have more fixes coming up.

Cheers,

Paul

> Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> 
> > ---
> >  drivers/gpu/drm/gma500/gma_display.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
> > index e20ccb5d10fd..bc07ae2a9a1d 100644
> > --- a/drivers/gpu/drm/gma500/gma_display.c
> > +++ b/drivers/gpu/drm/gma500/gma_display.c
> > @@ -255,6 +255,8 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode)
> >                 /* Give the overlay scaler a chance to enable
> >                  * if it's on this pipe */
> >                 /* psb_intel_crtc_dpms_video(crtc, true); TODO */
> > +
> > +               drm_crtc_vblank_on(crtc);
> >                 break;
> >         case DRM_MODE_DPMS_OFF:
> >                 if (!gma_crtc->active)
> > --
> > 2.23.0
> >

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Paul Kocialkowski]

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@bootlin.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

[PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Paul Kocialkowski]

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@bootlin.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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Patrik Jakobsson]

On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
<paul.kocialkowski@bootlin.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@bootlin.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@gmail.com>

Re: [PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Patrik Jakobsson]

On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
<paul.kocialkowski@bootlin.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@bootlin.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@gmail.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Daniel Vetter]

On Wed, Nov 06, 2019 at 04:24:59PM +0100, Patrik Jakobsson wrote:
> On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
> <paul.kocialkowski@bootlin.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@bootlin.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@gmail.com>

I'm assuming you'll also push these?

Always confusing when maintainer/committers r-b but don't say anything
about pushing the patch. Good chances it'll fall through cracks if that
happens.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Daniel Vetter]

On Wed, Nov 06, 2019 at 04:24:59PM +0100, Patrik Jakobsson wrote:
> On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
> <paul.kocialkowski@bootlin.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@bootlin.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@gmail.com>

I'm assuming you'll also push these?

Always confusing when maintainer/committers r-b but don't say anything
about pushing the patch. Good chances it'll fall through cracks if that
happens.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Patrik Jakobsson]

On Thu, Nov 7, 2019 at 9:31 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Nov 06, 2019 at 04:24:59PM +0100, Patrik Jakobsson wrote:
> > On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.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@bootlin.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@gmail.com>
>
> I'm assuming you'll also push these?
>
> Always confusing when maintainer/committers r-b but don't say anything
> about pushing the patch. Good chances it'll fall through cracks if that
> happens.
> -Daniel

Ah sorry, I also find it confusing. I'll push these.

-Patrik

> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Re: [PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Patrik Jakobsson]

On Thu, Nov 7, 2019 at 9:31 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Nov 06, 2019 at 04:24:59PM +0100, Patrik Jakobsson wrote:
> > On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.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@bootlin.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@gmail.com>
>
> I'm assuming you'll also push these?
>
> Always confusing when maintainer/committers r-b but don't say anything
> about pushing the patch. Good chances it'll fall through cracks if that
> happens.
> -Daniel

Ah sorry, I also find it confusing. I'll push these.

-Patrik

> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Daniel Vetter]

On Thu, Nov 7, 2019 at 10:08 AM Patrik Jakobsson
<patrik.r.jakobsson@gmail.com> wrote:
>
> On Thu, Nov 7, 2019 at 9:31 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Wed, Nov 06, 2019 at 04:24:59PM +0100, Patrik Jakobsson wrote:
> > > On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.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@bootlin.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@gmail.com>
> >
> > I'm assuming you'll also push these?
> >
> > Always confusing when maintainer/committers r-b but don't say anything
> > about pushing the patch. Good chances it'll fall through cracks if that
> > happens.
> > -Daniel
>
> Ah sorry, I also find it confusing. I'll push these.

Thanks.

Also for a quick check whether someone is committer or not:
https://people.freedesktop.org/~seanpaul/whomisc.html

Once we're on gitlab it should be a lot easier since the list of
committers is all there in the web ui.
-Daniel

>
> -Patrik
>
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Daniel Vetter]

On Thu, Nov 7, 2019 at 10:08 AM Patrik Jakobsson
<patrik.r.jakobsson@gmail.com> wrote:
>
> On Thu, Nov 7, 2019 at 9:31 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Wed, Nov 06, 2019 at 04:24:59PM +0100, Patrik Jakobsson wrote:
> > > On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.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@bootlin.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@gmail.com>
> >
> > I'm assuming you'll also push these?
> >
> > Always confusing when maintainer/committers r-b but don't say anything
> > about pushing the patch. Good chances it'll fall through cracks if that
> > happens.
> > -Daniel
>
> Ah sorry, I also find it confusing. I'll push these.

Thanks.

Also for a quick check whether someone is committer or not:
https://people.freedesktop.org/~seanpaul/whomisc.html

Once we're on gitlab it should be a lot easier since the list of
committers is all there in the web ui.
-Daniel

>
> -Patrik
>
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Patrik Jakobsson]

On Thu, Nov 7, 2019 at 10:21 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Thu, Nov 7, 2019 at 10:08 AM Patrik Jakobsson
> <patrik.r.jakobsson@gmail.com> wrote:
> >
> > On Thu, Nov 7, 2019 at 9:31 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> > >
> > > On Wed, Nov 06, 2019 at 04:24:59PM +0100, Patrik Jakobsson wrote:
> > > > On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.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@bootlin.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@gmail.com>
> > >
> > > I'm assuming you'll also push these?
> > >
> > > Always confusing when maintainer/committers r-b but don't say anything
> > > about pushing the patch. Good chances it'll fall through cracks if that
> > > happens.
> > > -Daniel
> >
> > Ah sorry, I also find it confusing. I'll push these.
>
> Thanks.
>
> Also for a quick check whether someone is committer or not:
> https://people.freedesktop.org/~seanpaul/whomisc.html

Great, I've been looking for a list like that.

>
> Once we're on gitlab it should be a lot easier since the list of
> committers is all there in the web ui.
> -Daniel
>
> >
> > -Patrik
> >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 2/2] drm/gma500: Add page flip support on psb/cdv

[Patrik Jakobsson]

On Thu, Nov 7, 2019 at 10:21 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Thu, Nov 7, 2019 at 10:08 AM Patrik Jakobsson
> <patrik.r.jakobsson@gmail.com> wrote:
> >
> > On Thu, Nov 7, 2019 at 9:31 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> > >
> > > On Wed, Nov 06, 2019 at 04:24:59PM +0100, Patrik Jakobsson wrote:
> > > > On Wed, Nov 6, 2019 at 10:44 AM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.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@bootlin.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@gmail.com>
> > >
> > > I'm assuming you'll also push these?
> > >
> > > Always confusing when maintainer/committers r-b but don't say anything
> > > about pushing the patch. Good chances it'll fall through cracks if that
> > > happens.
> > > -Daniel
> >
> > Ah sorry, I also find it confusing. I'll push these.
>
> Thanks.
>
> Also for a quick check whether someone is committer or not:
> https://people.freedesktop.org/~seanpaul/whomisc.html

Great, I've been looking for a list like that.

>
> Once we're on gitlab it should be a lot easier since the list of
> committers is all there in the web ui.
> -Daniel
>
> >
> > -Patrik
> >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel