[RFC PATCH v3 0/3] Support for Solid Fill Planes

[Jessica Zhang]

On 1/31/2023 4:49 AM, Pekka Paalanen wrote:
> On Tue, 31 Jan 2023 11:21:18 +0000
> Simon Ser <contact at emersion.fr> wrote:
> 
>> On Tuesday, January 31st, 2023 at 12:13, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>>
>>> On Tue, 31 Jan 2023 10:06:39 +0000
>>> Simon Ser <contact at emersion.fr> wrote:
>>>    
>>>> On Tuesday, January 31st, 2023 at 10:25, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>>>>    
>>>>> indeed, what about simply using a 1x1 framebuffer for real? Why was that
>>>>> approach rejected?
>>>>
>>>> Ideally we don't want to allocate any GPU memory for the solid-fill
>>>> stuff. And if we special-case 1x1 FB creation to not be backed by real
>>>> GPU memory then we hit several situations where user-space expects a
>>>> real FB but there isn't: for instance, GETFB2 converts from FB object
>>>> ID to GEM handles. Even if we make GETFB2 fail and accept that this
>>>> breaks user-space, then there is no way for user-space to recover the
>>>> FB color for flicker-free transitions and such.
>>>>
>>>> This is all purely from a uAPI PoV, completely ignoring the potential
>>>> issues with the internal kernel abstractions which might not be suitable
>>>> for this either.
>>>
>>> I mean a real 1x1 buffer: a dumb buffer.
>>>
>>> It would be absolutely compatible with anything existing, because it is
>>> a real FB. As a dumb buffer it would be trivial to write into and read
>>> out. As 1x1 it would be tiny (one page?). Even if something needs to
>>> raw-access uncached memory over 33 MHz PCI bus or whatever the worst
>>> case is, it's just one pixel, so it's fast enough, right? And it only
>>> needs to be read once when set, like USB display drivers do. The driver
>>> does not need to manually apply any color operations, because none are
>>> supported in this special case.
>>>
>>> One can put all these limitations and even pixel format in the plane
>>> property that tells userspace that a 1x1 FB works here.
>>>
>>> To recap, the other alternatives under discussion I see right now are:
>>>
>>> - this proposal of dedicated fill color property
>>> - stuffing something new into FB_ID property
>>>
>>> There is also the question of other kinds of plane content sources like
>>> live camera feeds where userspace won't be shovelling each frame
>>> individually like we do now.
>>>
>>> 1x1 dumb buffer is not as small and lean as a dedicated fill color
>>> property, but the UAPI design questions seem to be much less. What's
>>> the best trade-off and for whom?
>>
>> By "real memory" yes I mean the 1 page.
>>
>> Using a real buffer also brings back other discussions, e.g. the one about
>> which pixel formats to accept.
> 
> Yeah, which is why I wrote: "One can put all these limitations and even
> pixel format in the plane property". It doesn't even need to be a
> variable in the UAPI, it can be hardcoded in the UAPI doc.
> 
> Please, do not understand this as me strongly advocating for the real FB
> approach! I just don't want that option to be misunderstood.
> 
> I don't really care which design is chosen, but I do care about
> documenting why other designs were rejected. If the rejection reasons
> were false, they should be revised, even if the decision does not
> change.

Hi Pekka/Daniel,

Looks like the general sentiment is to keep solid fill as a separate 
property, so I will stick with that implementation for v4.

I can document the reason why we chose this approach over 1x1 FB in the 
cover letter, but to summarize here:

Allocating an FB for solid_fill brings in unnecessary overhead (ex. 
having to allocate memory for the FB). In addition, since memory fetch 
is disabled when solid fill is enabled, having a separate property that 
doesn't do any memory allocation for solid fill better reflects the 
behavior of this feature within driver.

We also wanted to avoid having FB_ID accept a property blob as it would 
involve loosening some drm_property checks, which could cause issues 
with other property ioctls.



Also, re: other plane sources -- FWIW, I have tried implementing a 
source enum as Ville suggested, but ultimately dropped the change as it 
would require userspace to set properties in a specific order (i.e. to 
enable solid_fill, userspace would have to first set FB_ID to NULL then 
set SOLID_FILL).

I'm not sure how much of a can of worms that would be for userspace, but 
if you're fine with having that as a requirement the I can re-add the code.

Thanks,

Jessica Zhang

> 
> 
> Thanks,
> pq