On 2018-06-14 12:57 PM, Michel Dänzer wrote:

Hi Leo,


sorry for the delay.


Appreciate the review, it's not a small change by any means :)


On 2018-06-01 06:03 PM, [email protected] wrote:
From: "Leo (Sunpeng) Li" <[email protected]>

This ended up being different enough from v2 to warrant a new patchset. Per
Michel's suggestions, there have been various optimizations and cleanups.
Here's what's changed:

* Cache DRM color management property IDs at pre-init,
     * instead of querying DRM each time we need to modify color properties.

* Remove drmmode_update_cm_props().
     * Update color properties in drmmode_output_get_property() instead.
     * This also makes old calls to update_cm_props redundant.

* Get rid of fake CRTCs.
     * Previously, we were allocating a fake CRTC to configure color props on
       outputs that don't have a CRTC.
     * Instead, rr_configure_and_change_cm_property() can be easily modified to
       accept NULL CRTCs.

Is it currently ever the case in the hardware / kernel, or expected to
ever be, that colour management is supported for some but not all CRTCs
of a GPU?


This was more of an effort to align with what DRM allows, which is
per-CRTC cm support and LUT sizes. I'm not aware of any current or
future hardware where this is the case though. It was a relatively
simple thing to implement, so I thought, might as well.

If not, the LUT sizes could be tracked once instead of per CRTC, and at
least the (DE)GAMMA_LUT_SIZE properties could always return the proper
values, even if the output currently isn't associated with a CRTC.


My original take was that it's best to support what the framework
allows. But it does sound like this would have more utility to the
client, and simplify the code as well.


Other than that, I'm going to send some minor feedback on patches 2 & 3.
If you prefer, I could fix up those and other cosmetic issues before
pushing the patches.


I can incorporate those along with the edit above.

Thanks!
Leo


_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to