On 12/16/15 10:37, Daniel Vetter wrote:
> On Tue, Dec 15, 2015 at 09:03:14PM +0200, Jyri Sarha wrote:
>> >From: Tomi Valkeinen<tomi.valkeinen at ti.com>
>> >
>> >LCDC hardware does not support fb pitch that is different (i.e. larger)
>> >than the screen size. The driver currently does no checks for this, and
>> >the results of too big pitch are are flickering and lower fps.
>> >
>> >This issue easily happens when using libdrm's modetest tool with non-32
>> >bpp modes. As modetest always allocated 4 bytes per pixel, it implies a
>> >bigger pitch for 16 or 24 bpp modes.
>> >
>> >This patch adds a check to reject pitches the hardware cannot support.
>> >
>> >Signed-off-by: Tomi Valkeinen<tomi.valkeinen at ti.com>
>> >Signed-off-by: Darren Etheridge<detheridge at ti.com>
>> >Signed-off-by: Jyri Sarha<jsarha at ti.com>
>> >---
>> > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 31 +++++++++++++++++++++++++++++++
>> > 1 file changed, 31 insertions(+)
>> >
>> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> >b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> >index 7b687ae..105f286 100644
>> >--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> >@@ -151,6 +151,22 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
>> > kfree(tilcdc_crtc);
>> > }
>> >
>> >+static int tilcdc_verify_fb(struct drm_crtc *crtc, struct drm_framebuffer
>> >*fb)
>> >+{
>> >+ struct drm_device *dev = crtc->dev;
>> >+ unsigned int depth, bpp;
>> >+
>> >+ drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp);
>> >+
>> >+ if (fb->pitches[0] != crtc->mode.hdisplay * bpp / 8) {
>> >+ dev_err(dev->dev,
>> >+ "Invalid pitch: fb and crtc widths must be the same");
>> >+ return -EINVAL;
>> >+ }
>> >+
>> >+ return 0;
> This should be done in framebuffer_create instead if tilcdc has this
> requirement everywhere. No point in allowing userspace to create an fb you
> can't use. Only if you have planes with different limits should this be
> checked after fb creation.
>
That would not work because we do not know the mode of the crtc when the
framebuffer is going to be used.
> Also with atomic you'd only need to place this in one function, even if
> you have different per-plane limitations ... hint, hint;-)
I am working on atomic modeset for tilcdc, but I am still a newbie on
DRM front so it takes some time :).
Cheers,
Jyri
