On Mon, Dec 05, 2016 at 12:11:46PM +0100, Thierry Reding wrote: > On Mon, Dec 05, 2016 at 09:16:27AM +0100, Daniel Vetter wrote: > > On Mon, Dec 05, 2016 at 08:57:43AM +0100, Thierry Reding wrote: > > > On Sat, Dec 03, 2016 at 04:35:24AM +0000, Sharma, Shashank wrote: > > > > Hi Thierry, > > > > > > > > If you can please have a look on this patch, I had written one to parse HF-VSDB, which was covering SCDC detection too. > > > > https://patchwork.kernel.org/patch/9452259/ > > > > > > I think there had been pushback before about caching capabilities from > > > EDID, so from that point of view my patch is more inline with existing > > > EDID parsing API. > > > > Hm, where was that pushback? We do have a bit a mess between explicitly > > parsing stuff (e.g. eld) and stuffing parsed data into drm_display_info. > > You did object to a very similar patch some time ago that did a similar > thing for DPCD stuff. And also Villa had commented on an earlier patch > from Jose about concerns of bloating core structures: > > https://patchwork.freedesktop.org/patch/104806/ DPCD I complained about because somehow we ended up with 2 sets of helpers, one filling a struct and the others returning directly. I objected to the fact that there's 2 (and imo your patch duplicated even more), not that I think one approach is clearly inferior to the other. Demanding that there's some real users is also a valid point. > > I think long-term stuffing it into drm_display_info is probably better, > > since then we only have 1 interaction point between the probe code and the > > atomic_check code. That should be useful for eventually fixing the lack of > > locking between the two, if I ever get around to that ;-) > > I don't really have objections to caching the results of parsing, it's > what I had proposed and what seemed most natural back when I was working > on the DPCD helpers. But if we now agree that this is the preferred way > to do things, then we should at least agree that it applies to all areas > for the sake of consistency. > > Also, it might be worth looking into improving the structures, and maybe > adding new ones to order things more conveniently or at least group them > in some logical way. In my opinion some of our data structures are > becoming somewhat... unwieldy. We're pretty good at consuming fairly invasive refactorings in drm-misc. So it just boils down to get some agreement on what things should look like (+1 from my side to parsing stuff into structs as a general idea), and then massaging all the existing users of the "wrong" interface using cocci and sed. Unfortunately "just" ;-) > > > Other than that the patches are mostly equivalent, except yours parses > > > more information than just the SCDC bits. > > > > So merge patch 1 from your series + Shashank's parsing patch? Everyone > > agrees and can you pls cross-r-b stamp so I can apply it all? > > I think I spotted a mistake in Shashank's parsing patch. Let me take > another look. > > If we can agree on a common way forward on how to deal with this kind of > static data, I have no objections to caching data for the duration of a > hotplug period. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
