Re: [PATCH V12 14/49] drm/vkms: Add enumerated 1D curve colorop

[Louis Chauvet <louis.chauvet@bootlin.com>]

Le 31/10/2025 à 01:04, Alex Hung a écrit :
> 
> 
> On 10/30/25 08:50, Louis Chauvet wrote:
>>
>> This patch LGTM, but can you add the [1] (preparatory patch to reduce
>> the diff in this patch) and [2] (fixup to this patch, add a module
>> parameter to enable/disable the pipeline and disable it by default for
>> configfs devices) so it will make it easier to implement configfs for
>> color pipeline without breaking uAPI?
>>
>> [1]:https://paste.sr.ht/~fomys/16118ef2b5604226a7607db8a41941a27a43f168
>> [2]:https://paste.sr.ht/~fomys/a98191b09ff7290dc6768bf7d54e789984cd3250
>>
>> With those patches: Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
>>
>> Thanks a lot for your series!
> 
> Hi Louis,
> 
> Thanks for reviewing.
> 
> Do you want to add these patch on top of this patch as below?
> 
>     e1c34c68dc33 fixup! drm/vkms: Add enumerated 1D curve colorop
>     68251534ebd1 drm/vkms: Pass plane_cfg to plane initialization
>     8160438be2b5 drm/vkms: Add enumerated 1D curve colorop
> 
> I noticed "fixup! drm/vkms: Add enumerated 1D curve colorop" has no
> commit messages though.

drm/vkms: Add config for default plane pipeline

With the introduction of color pipeline in VKMS, the default device may 
have planes with color pipelines. To avoid breaking existing uAPI, 
create a kernel argument to disable them by default and a vkms_config 
field to configure the plane.

This field is not definitive and will be replaced once the uAPI will be 
able to configure color pipelines. For now devices created with ConfigFS 
will not have any color pipeline so we can decide later how the uAPI 
will look like.

> Or do you prefer them to squash them together?

For me 68251534ebd1 is independent so keep it separate.

But for e1c34c68dc33 I don't think this is important to squash or keep 
them separate. You can do as you want.

If you want to split it, I think you can put e1c34c68dc33 before 
8160438be2b5 so you don't have to change the call to 
vkms_initialize_colorops(&plane->base);.

Thanks a lot,
Louis Chauvet

-- 
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com