On 12/04/16 12:57, Daniel Vetter wrote: > On Mon, Apr 11, 2016 at 02:43:39PM +0100, Lionel Landwerlin wrote: >> Color management properties are a bit of an odd use case because >> they're not marked as atomic properties. Currently we're not updating >> the non atomic values so the drm_crtc_state is out of sync with the >> values stored in the crtc object. > tbh sounds like this is the bug here - why does the lookup of the correct > values stored in the crtc_state drm_atomic_crtc_get_property() not work?
This is only a problem when the kernel space modifies property values. User space ioctls do the right thing and update both places : https://cgit.freedesktop.org/drm-intel/tree/drivers/gpu/drm/drm_crtc.c#n4916 > > Duct-taping over this bug in every place we update these properties > (you're just patching one of many) won't scale. > > Also: Where's the igt for this failure? > -Daniel There is : https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/tests/kms_pipe_color.c#n554 That's how we caught it. >> v2: Update non atomic values only if commit succeeds (Bob Paauwe) >> >> v3: Do not access crtc_state after commit, use crtc->state (Maarten >> Lankhorst) >> >> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com> >> Cc: Bob Paauwe <bob.j.paauwe at intel.com> >> Cc: <dri-devel at lists.freedesktop.org> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com> >> --- >> drivers/gpu/drm/drm_atomic_helper.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c >> b/drivers/gpu/drm/drm_atomic_helper.c >> index 7bf678e..13b86cf 100644 >> --- a/drivers/gpu/drm/drm_atomic_helper.c >> +++ b/drivers/gpu/drm/drm_atomic_helper.c >> @@ -2979,6 +2979,14 @@ retry: >> if (ret) >> goto fail; >> >> + drm_object_property_set_value(&crtc->base, >> + config->degamma_lut_property, 0); >> + drm_object_property_set_value(&crtc->base, >> + config->ctm_property, 0); >> + drm_object_property_set_value(&crtc->base, >> + config->gamma_lut_property, >> + crtc->state->gamma_lut->base.id); >> + >> /* Driver takes ownership of state on successful commit. */ >> >> drm_property_unreference_blob(blob); >> -- >> 2.8.0.rc3 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel