Subject: Re: [PATCH v4 03/14] drm/fb-helper: separate the fb_setcmap helper
 into atomic and legacy paths
To: linux-kernel@vger.kernel.org,
        Boris Brezillon <boris.brezillon@free-electrons.com>,
        dri-devel@lists.freedesktop.org,
        Daniel Vetter <daniel.vetter@intel.com>
References: <1499343648-29695-1-git-send-email-peda@axentia.se>
 <1499343648-29695-4-git-send-email-peda@axentia.se>
 <20170711081013.leqxhbp455pydqcr@phenom.ffwll.local>
 <4ecd88e4-371e-e969-9641-0c8c1b0ff81e@axentia.se>
 <20170712070352.gyjgnlpngocshuiw@phenom.ffwll.local>
From: Peter Rosin <peda@axentia.se>
Organization: Axentia Technologies AB
Message-ID: <45e64660-b903-6f2b-42f9-55a42e478810@axentia.se>
Date: Wed, 12 Jul 2017 09:47:27 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <20170712070352.gyjgnlpngocshuiw@phenom.ffwll.local>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jul 2017 07:47:31.6598 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0202MB2545
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2542
Lines: 54

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.
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)

Yes, it's a fringe thing to cater to...

Cheers,
Peter