diff mbox

drm: imx: convert to drm_crtc_send_vblank_event()

Message ID E1a1XGd-0005uT-C9@rmk-PC.arm.linux.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Russell King Nov. 25, 2015, 10:25 a.m. UTC
ipu_crtc_handle_pageflip() was calling drm_send_vblank_event() with
a pipe argument of -1.  Commit cc1ef118fc09 ("drm/irq: Make pipe
unsigned and name consistent") now makes this error obvious, as we
now may get a warning from:

	if (WARN_ON(pipe >= dev->num_crtcs))

in drm_vblank_count_and_time().  Prior to this change, we would end
up making out-of-bounds array accesses via:

	struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
and
	*vblanktime = vblanktimestamp(dev, pipe, cur_vblank);

So, this has been broken for a very long time, and is not a result
of the above commit.  Since we don't care about the staging versions,
I've tagged this with the earliest mainline commit where we do care,
even though this commit did not introduce the bug.

Fixes: 6556f7f82b9c ("drm: imx: Move imx-drm driver out of staging")
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/gpu/drm/imx/ipuv3-crtc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Philipp Zabel Nov. 25, 2015, 11:06 a.m. UTC | #1
Hi Russell,

Am Mittwoch, den 25.11.2015, 10:25 +0000 schrieb Russell King:
> ipu_crtc_handle_pageflip() was calling drm_send_vblank_event() with
> a pipe argument of -1.  Commit cc1ef118fc09 ("drm/irq: Make pipe
> unsigned and name consistent") now makes this error obvious, as we
> now may get a warning from:
> 
> 	if (WARN_ON(pipe >= dev->num_crtcs))
> 
> in drm_vblank_count_and_time().  Prior to this change, we would end
> up making out-of-bounds array accesses via:
> 
> 	struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
> and
> 	*vblanktime = vblanktimestamp(dev, pipe, cur_vblank);
> 
> So, this has been broken for a very long time, and is not a result
> of the above commit.  Since we don't care about the staging versions,
> I've tagged this with the earliest mainline commit where we do care,
> even though this commit did not introduce the bug.
> 
> Fixes: 6556f7f82b9c ("drm: imx: Move imx-drm driver out of staging")
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

thank you for the patch. Thierry had sent a fix previously [1], but I
don't think it is applied yet. If nobody minds, I'd like to add this to
imx-drm/fixes as opposed to the older patch.

[1] https://patchwork.kernel.org/patch/7258101/

regards
Philipp
Daniel Vetter Nov. 25, 2015, 11:30 a.m. UTC | #2
On Wed, Nov 25, 2015 at 10:25:39AM +0000, Russell King wrote:
> ipu_crtc_handle_pageflip() was calling drm_send_vblank_event() with
> a pipe argument of -1.  Commit cc1ef118fc09 ("drm/irq: Make pipe
> unsigned and name consistent") now makes this error obvious, as we
> now may get a warning from:
> 
> 	if (WARN_ON(pipe >= dev->num_crtcs))
> 
> in drm_vblank_count_and_time().  Prior to this change, we would end
> up making out-of-bounds array accesses via:
> 
> 	struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
> and
> 	*vblanktime = vblanktimestamp(dev, pipe, cur_vblank);
> 
> So, this has been broken for a very long time, and is not a result
> of the above commit.  Since we don't care about the staging versions,
> I've tagged this with the earliest mainline commit where we do care,
> even though this commit did not introduce the bug.
> 
> Fixes: 6556f7f82b9c ("drm: imx: Move imx-drm driver out of staging")
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/imx/ipuv3-crtc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 7bc8301fafff..7ce1df0f476a 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -212,7 +212,8 @@ static void ipu_crtc_handle_pageflip(struct ipu_crtc *ipu_crtc)
>  
>  	spin_lock_irqsave(&drm->event_lock, flags);
>  	if (ipu_crtc->page_flip_event)
> -		drm_send_vblank_event(drm, -1, ipu_crtc->page_flip_event);
> +		drm_crtc_send_vblank_event(&ipu_crtc->base,
> +					   ipu_crtc->page_flip_event);
>  	ipu_crtc->page_flip_event = NULL;
>  	imx_drm_crtc_vblank_put(ipu_crtc->imx_crtc);
>  	spin_unlock_irqrestore(&drm->event_lock, flags);
> -- 
> 2.1.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 7bc8301fafff..7ce1df0f476a 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -212,7 +212,8 @@  static void ipu_crtc_handle_pageflip(struct ipu_crtc *ipu_crtc)
 
 	spin_lock_irqsave(&drm->event_lock, flags);
 	if (ipu_crtc->page_flip_event)
-		drm_send_vblank_event(drm, -1, ipu_crtc->page_flip_event);
+		drm_crtc_send_vblank_event(&ipu_crtc->base,
+					   ipu_crtc->page_flip_event);
 	ipu_crtc->page_flip_event = NULL;
 	imx_drm_crtc_vblank_put(ipu_crtc->imx_crtc);
 	spin_unlock_irqrestore(&drm->event_lock, flags);