On Wed, Jul 12, 2017 at 09:47:27AM +0200, Peter Rosin wrote: > On 2017-07-12 09:03, Daniel Vetter wrote: > > On Tue, Jul 11, 2017 at 02:12:26PM +0200, Peter Rosin wrote: > >> On 2017-07-11 10:10, Daniel Vetter wrote: > >>> Tiny nit you might want to improve (since you need to respin for my naming > >>> bikeshed of the property_replace_blob function anyway): Properties are > >>> refcounting and invariant, which means you can just create the property > >>> once, and then use it for all the CRTC. Slightly cleaner code. > >> > >> Yes, I thought about that, but ended up not. The reason is that as far > >> as I could tell, all involved crtc need not have the same original > >> gamma_lut. Sure, if all crtc have the same history, that should be the > >> case, but isn't it possible to tie things together one way first and > >> set some clut, then "rewire" things so that the crtc no longer have the > >> same history? > >> > >> But if you in the light of that still think it's wise to set the same > >> clut for all crtc I will factor that part out of the loop. > > > > Blob properties are invariant, if you want to change a lut you _have_ to > > create a new blob property. They're also reference-counted, which means > > users of a blob property can come&go as they wish, it will only get freed > > when the last one is released. > > > > So even when you change the lut of 1 CRTC the other CRTCs will be able to > > keep using the existing lut blob property unchanged. That's the beauty of > > having refcounted objects with invariant data over their lifetime, makes a > > lot of things a lot simpler. drm_framebuffer work the same (only their > > metadata is invariant, the data of the actual backing storage can change > > ofc, but not where that backing storage is). Allows you to do simple > > pointer comparison of objects to check whether their equal or something > > has changed. > > > > tldr; sharing blobs is perfectly safe and how this is designed to work. > > Yes, I get that, but that wasn't my problem. At all. > > Say that you have a driver with two crtc, A and B. Then this happens: > > 1. A gets a clut with, say, only various red colors. > 2. B gets a different clut with various green colors. > 3. Someone ties things up so that one fbdev is used on both A and B. > I don't know if this is possible, but if it is, the two crtc now > have different cluts. That's the default for fbdev on top of kms. fbdev doesn't have a concept of multi-screen. Some things are mapped to the first output only (like vblank waits). > 4. Via fbdev, only part of the clut is updated for this A/B combo. > > If A and B starts sharing clut in 4, the part that is not updated is > clobbered for either crtc A or B. > > (updating only part of the clut is only possible with fbdev, AFAICT) Meh. Like, completely meh :-) > Yes, it's a fringe thing to cater to... Yeah, let's shrug this under the table. fbdev doesn't work for multi-screen, ending up with strange behaviour is totally fine. -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
