[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170626092737.443oaolwa2zisiya@phenom.ffwll.local>
Date: Mon, 26 Jun 2017 11:27:37 +0200
From: Daniel Vetter <daniel@...ll.ch>
To: Peter Rosin <peda@...ntia.se>
Cc: linux-kernel@...r.kernel.org, nouveau@...ts.freedesktop.org,
dri-devel@...ts.freedesktop.org,
virtualization@...ts.linux-foundation.org,
amd-gfx@...ts.freedesktop.org,
Daniel Vetter <daniel.vetter@...el.com>,
Boris Brezillon <boris.brezillon@...e-electrons.com>,
Russell King <linux@...linux.org.uk>,
Ben Skeggs <bskeggs@...hat.com>,
Dave Airlie <airlied@...hat.com>,
intel-gfx@...ts.freedesktop.org,
Vincent Abriou <vincent.abriou@...com>,
Philippe Cornu <philippe.cornu@...com>,
Yannick Fertre <yannick.fertre@...com>,
Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>,
Gerd Hoffmann <kraxel@...hat.com>
Subject: Re: [PATCH v2 01/14] drm/fb-helper: keep the .gamma_store updated in
drm_fb_helper_setcmap
On Thu, Jun 22, 2017 at 08:06:24AM +0200, Peter Rosin wrote:
> I think the gamma_store can end up invalid on error. But the way I read
> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
> this pesky legacy fbdev stuff be any better?
>
> Signed-off-by: Peter Rosin <peda@...ntia.se>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 574af01..25315fb 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1223,12 +1223,16 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
> const struct drm_crtc_helper_funcs *crtc_funcs;
> u16 *red, *green, *blue, *transp;
> struct drm_crtc *crtc;
> + u16 *r, *g, *b;
> int i, j, rc = 0;
> int start;
>
> if (oops_in_progress)
> return -EBUSY;
>
> + if (cmap->start + cmap->len < cmap->start)
> + return -EINVAL;
Doesn't the fbdev core check this for us?
> +
> drm_modeset_lock_all(dev);
> if (!drm_fb_helper_is_bound(fb_helper)) {
> drm_modeset_unlock_all(dev);
> @@ -1245,6 +1249,29 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
> transp = cmap->transp;
> start = cmap->start;
>
> + if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
I think switching to a positive check of visual == PSEUDOCOLOR is a bit
clearer. Also this makes more sense once we move over to the main
gamma_set function, since as-is it's unclear that setcolreg only does
something for PSEUDOCOLOR. But interim I guess that's ok.
-Daniel
> + if (!crtc->gamma_size) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + if (cmap->start + cmap->len > crtc->gamma_size) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + r = crtc->gamma_store;
> + g = r + crtc->gamma_size;
> + b = g + crtc->gamma_size;
> +
> + memcpy(r + cmap->start, cmap->red,
> + cmap->len * sizeof(u16));
> + memcpy(g + cmap->start, cmap->green,
> + cmap->len * sizeof(u16));
> + memcpy(b + cmap->start, cmap->blue,
> + cmap->len * sizeof(u16));
> + }
> +
> for (j = 0; j < cmap->len; j++) {
> u16 hred, hgreen, hblue, htransp = 0xffff;
>
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Powered by blists - more mailing lists