mbox series

[0/2] Add "BACKGROUND_COLOR" drm property

Message ID 20210707084557.22443-1-raphael.gallais-pou@foss.st.com (mailing list archive)
Headers show
Series Add "BACKGROUND_COLOR" drm property | expand

Message

Raphael Gallais-Pou July 7, 2021, 8:48 a.m. UTC
Hello,

This series aims to add a generic background color property for CRTC as
discussed in the conversation:
https://patchwork.freedesktop.org/patch/439585/

This patch "drm: add crtc background color property" is strongly inspired
of Matthew Roper's.  Please see:
https://patchwork.freedesktop.org/patch/333632/

Raphael Gallais-Pou (2):
  drm: add crtc background color property
  drm/stm: ltdc: add crtc background color property support

 drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
 drivers/gpu/drm/drm_atomic_uapi.c         |  4 ++
 drivers/gpu/drm/drm_blend.c               | 34 +++++++++++++++-
 drivers/gpu/drm/drm_mode_config.c         |  6 +++
 drivers/gpu/drm/stm/ltdc.c                | 48 ++++++++++++++++++++---
 include/drm/drm_blend.h                   |  1 +
 include/drm/drm_crtc.h                    | 12 ++++++
 include/drm/drm_mode_config.h             |  5 +++
 include/uapi/drm/drm_mode.h               | 28 +++++++++++++
 9 files changed, 132 insertions(+), 7 deletions(-)

Comments

Simon Ser July 7, 2021, 9:03 a.m. UTC | #1
Hi,

Thanks for working on this. Do you have plans for user-space
implementations and IGT?

Thanks,

Simon
Daniel Vetter July 7, 2021, 11:42 a.m. UTC | #2
On Wed, Jul 07, 2021 at 09:03:03AM +0000, Simon Ser wrote:
> Hi,
> 
> Thanks for working on this. Do you have plans for user-space
> implementations and IGT?

Note that these parts are mandatory, and there's a patch floating around
further clarifying what's all expected for new properties:

https://lore.kernel.org/dri-devel/20210706161244.1038592-1-maxime@cerno.tech/

Cheers, Daniel

> 
> Thanks,
> 
> Simon
Raphael Gallais-Pou July 9, 2021, 9:09 a.m. UTC | #3
On 7/7/21 1:42 PM, Daniel Vetter wrote:
> On Wed, Jul 07, 2021 at 09:03:03AM +0000, Simon Ser wrote:
>> Hi,
>>
>> Thanks for working on this. Do you have plans for user-space
>> implementations and IGT?
> Note that these parts are mandatory, and there's a patch floating around
> further clarifying what's all expected for new properties:
>
> https://lore.kernel.org/dri-devel/20210706161244.1038592-1-maxime@cerno.tech/


Hi,


We don't usually test with piglit and igt-gpu-tools. Instead, modetest 
utility of the libdrm is used quite often (as is it the case in order to 
test this property).


We plan to port those tools on our platform before implementing this 
kind of tests, but it will require a bit more time.

An analysis is currently ongoing.


Furthermore, do you have any advice on top of documentation for 
implementing such tests ?


I was also thinking about implementing an option into modetest to ease 
the use of this drm property (support of hexadecimal values for properties).


Regards,

Raphaël

>
> Cheers, Daniel
>
>> Thanks,
>>
>> Simon
Simon Ser July 9, 2021, 9:23 a.m. UTC | #4
On Friday, July 9th, 2021 at 11:09, Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com> wrote:

> We don't usually test with piglit and igt-gpu-tools. Instead, modetest
> utility of the libdrm is used quite often (as is it the case in order to
> test this property).

Just to make it extra clear: regardless of how you bring up your driver
implementation, without an IGT test and real-world open-source user-space
patches that make use of the new prop, your patches *cannot* be merged.

If you're planning to add support for the new prop to an open-source KMS
client, please add a link to the patches in your kernel submission. If
you don't have plans to use the new prop in an open-source KMS client,
let us know and we can discuss what the best candidate would be.

> I was also thinking about implementing an option into modetest to ease
> the use of this drm property (support of hexadecimal values for properties).

(For the record, modeset doesn't count as a real-world user-space usage: it's
just a toy implementation, just a test tool.)

Simon
Raphael Gallais-Pou July 12, 2021, 3:50 p.m. UTC | #5
On 7/9/21 11:23 AM, Simon Ser wrote:
> On Friday, July 9th, 2021 at 11:09, Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com> wrote:
>
>> We don't usually test with piglit and igt-gpu-tools. Instead, modetest
>> utility of the libdrm is used quite often (as is it the case in order to
>> test this property).
> Just to make it extra clear: regardless of how you bring up your driver
> implementation, without an IGT test and real-world open-source user-space
> patches that make use of the new prop, your patches *cannot* be merged.
>
> If you're planning to add support for the new prop to an open-source KMS
> client, please add a link to the patches in your kernel submission. If
> you don't have plans to use the new prop in an open-source KMS client,
> let us know and we can discuss what the best candidate would be.

Hi,

Thanks for your feedback :)


We do not have any KMS client currently using this property apart from proprietary solutions made by consumers.

This property was originally made to display a logo picture when the screen is in idle (low power/low DDR usage).


Regards,

Raphaël


>> I was also thinking about implementing an option into modetest to ease
>> the use of this drm property (support of hexadecimal values for properties).
> (For the record, modeset doesn't count as a real-world user-space usage: it's
> just a toy implementation, just a test tool.)
>
> Simon