[Intel-gfx] [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)

[Maarten Lankhorst]

Op 19-04-16 om 18:20 schreef Bob Paauwe:
> On Tue, 19 Apr 2016 08:05:26 +0200
> Maarten Lankhorst <maarten.lankhorst at linux.intel.com> wrote:
>
>> Op 18-04-16 om 18:47 schreef Bob Paauwe:
>>> The i915 driver is now using atomic properties and atomic commit
>>> to handle the legacy set gamma IOCTL. However, if the driver is
>>> configured without atomic (nuclear_pageflip = false), it won't
>>> update the legacy properties for degamma_lut, gamma_lut and ctm
>>> leaving them out of sync with the atomic version of the properties.
>>>
>>> Until the driver is full atomic, make sure we update the non-atomic
>>> version of the properties.
>>>
>>> v2: Update the comment with a FIXME.  (Daniel)
>>>
>>> igt-testcase: kms_pipe_color / legacy-gamma-reset-pipeX
>>> Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>>> Cc: Daniel Vetter <daniel at ffwll.ch>
>>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>> Signed-off-by: Bob Paauwe <bob.j.paauwe at intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 38 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 5155efb6..a97bbff 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -13885,8 +13885,45 @@ out:
>>>  
>>>  #undef for_each_intel_crtc_masked
>>>  
>>> +/*
>>> + * FIXME: Remove this once i915 is fully DRIVER_ATOMIC by calling
>>> + *        drm_atomic_helper_legacy_gamma_set() directly.
>>> + */
>>> +static void intel_atomic_legacy_gamma_set(struct drm_crtc *crtc,
>>> +					  u16 *red, u16 *green, u16 *blue,
>>> +					  uint32_t start, uint32_t size)
>>> +{
>>> +	struct drm_device *dev = crtc->dev;
>>> +	struct drm_mode_config *config = &dev->mode_config;
>>> +	struct drm_crtc_state *state;
>>> +
>>> +	drm_atomic_helper_legacy_gamma_set(crtc, red, green, blue, start, size);
>>>  
>> On a side note, the gamma_set function pointer should really be an int, because it may fail with -EINTR.
> ???  from drm_crtc.h
>
> void (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
>                           uint32_t start, uint32_t size);
>
> Or are you saying that the gamma_set should be capable of returning an
> error that can be propagated up through the IOCTL?
>
Yes, because it may fail with -EINTR.

~Maarten