[PATCH 04/33] drm/amd/display: Use 4096 lut entries

[Michel Dänzer]

On 2018-02-27 05:16 PM, Harry Wentland wrote:
> On 2018-02-27 11:08 AM, Michel Dänzer wrote:
>> On 2018-02-27 04:54 PM, Leo Li wrote:
>>> On 2018-02-27 05:34 AM, Michel Dänzer wrote:
>>>> On 2018-02-26 09:15 PM, Harry Wentland wrote:
>>>>> From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com>
>>>>>
>>>>> Non-legacy LUT size should reflect hw capability. Change size from 256
>>>>> to 4096.
>>>>>
>>>>> However, X doesn't seem to play with legacy LUTs of such size.
>>>>> Therefore, check for legacy lut when updating DC states, and update
>>>>> accordingly.
>>>>>
>>>>> v2: Use a macro for the maximum drm LUT value.
>>>>>
>>>>> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li at amd.com>
>>>>> Reviewed-by: Harry Wentland <Harry.Wentland at amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  2 +-
>>>>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  4 +-
>>>>>   .../drm/amd/display/amdgpu_dm/amdgpu_dm_color.c    | 77
>>>>> ++++++++++++++++------
>>>>>   3 files changed, 61 insertions(+), 22 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> index bb9405daa087..f782518fc424 100644
>>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> @@ -3228,7 +3228,7 @@ static int amdgpu_dm_crtc_init(struct
>>>>> amdgpu_display_manager *dm,
>>>>>       dm->adev->mode_info.crtcs[crtc_index] = acrtc;
>>>>>       drm_crtc_enable_color_mgmt(&acrtc->base, MAX_COLOR_LUT_ENTRIES,
>>>>>                      true, MAX_COLOR_LUT_ENTRIES);
>>>>> -    drm_mode_crtc_set_gamma_size(&acrtc->base, MAX_COLOR_LUT_ENTRIES);
>>>>> +    drm_mode_crtc_set_gamma_size(&acrtc->base,
>>>>> MAX_COLOR_LEGACY_LUT_ENTRIES);
>>>>
>>>> This means userspace can't set gamma ramps with more than 256 entries,
>>>> since drm_mode_gamma_set_ioctl returns an error if the sizes don't
>>>> match. We should just fix userspace which tries to use the wrong size.
>>>
>>> It seems X hard-codes 256 entries for the legacy lut, ignoring what the
>>> kernel requests:
>>>
>>> xf86CrtcCreate in xserver/hw/xfree86/modes/xf86Crtc.c (~line 120):
>>>
>>> /* Preallocate gamma at a sensible size. */
>>> crtc->gamma_size = 256;
>>>
>>>
>>> I confirmed this as well via a printk on the user LUT size in
>>> drm_mode_gamma_set_ioctl (after reverting to 4096 gamma size, of course).
>>>
>>> I'm not sure if modifying that line in X will be straightforward.
>>
>> It could be overridden by the DDX driver. However, there is the issue of
>> using an older DDX driver with a newer kernel.
>>
>>
>>> It's worth noting that drm_mode_crtc_set_gamma_size() only sets the legacy
>>> gamma size. Non-legacy gamma (de/regamma) sizes are reported via a mode
>>> property, and does not conflict with legacy size. Therefore, reporting a
>>> different size for legacy shouldn't be an issue, AFAIK; as long as we
>>> check the DRM lut size and update DC accordingly, which we're doing.
>>
>> Makes sense, but it means that gamma will not work for 30-bit colour in
>> Xorg until the DDX driver uses the new gamma mechanism, which might
>> require switching to the atomic KMS API as well. Is that okay?
>>
> 
> That's still current behavior, so this patch is not changing anything there. We could increase it but how would we deal with old DDX/X that assumes 256? Would it make sense instead to teach X to set the new gamma_lut property?

Maybe. Can it be used without using the atomic KMS UAPI in general?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer