linux-kernel - Re: [PATCH v3 05/16] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170706083413.ihujro5p7qlazg3a@phenom.ffwll.local>
Date:   Thu, 6 Jul 2017 10:34:13 +0200
From:   Daniel Vetter <daniel@...ll.ch>
To:     Peter Rosin <peda@...ntia.se>
Cc:     linux-kernel@...r.kernel.org,
        Boris Brezillon <boris.brezillon@...e-electrons.com>,
        dri-devel@...ts.freedesktop.org,
        Daniel Vetter <daniel.vetter@...el.com>
Subject: Re: [PATCH v3 05/16] drm/fb-helper: do a generic fb_setcmap helper
 in terms of crtc .gamma_set

> >> @@ -1369,27 +1362,57 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
> >>  		memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(*g));
> >>  		memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(*b));
> >>  
> >> -		for (j = 0; j < cmap->len; j++) {
> >> -			u16 hred, hgreen, hblue, htransp = 0xffff;
> >> +		ret = crtc->funcs->gamma_set(crtc, r, g, b,
> >> +					     crtc->gamma_size, crtc_state);
> > 
> > I guess my description of what I have in mind wasn't really clear. I think
> > a proper atomic commit should never reuse one of the old hooks
> > (->gamma_set) here, that's just confusing. Instead what I had in mind is
> > to do the proper adjusting that gamma_set does here in this function, i.e.
> > 
> > - create the new blob, fill it with the cmap data
> > 
> > - assign that blob to the crtc state:
> > 
> > 	drm_atomic_replace_property_blob(&crtc_state->gamma_lut,
> > 					 new_table, &temp);
> 
> That function is static, and...

Missed these comments here. I think we should just make them unstatic,
that might also explain why gamma_set is going this indirect path. Means
we need some kerneldoc, but that's not much work. Ping the original author
if you have questions.

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