Re: [Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper

2016-04-18 Thread Daniel Vetter 
On Mon, Apr 18, 2016 at 11:07:43AM +0100, Lionel Landwerlin wrote:
> On 12/04/16 17:07, Daniel Vetter wrote:
> >On Tue, Apr 12, 2016 at 01:51:02PM +0100, Lionel Landwerlin wrote:
> >>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
> >Nah, the problem is that we check fro DRIVER_ATOMIC in
> >drm_atomic_get_property, and because i915 is still not fully atomic that
> >gives us the wrong answer.
> >
> >Can you pls double-check that enabling i915 atomic (i915.nuclear_flip=1)
> >also fixes the bug?
> >
> >i915 is the only driver still transitioning, I definitely don't want to
> >have hacks all over for it.
> 
> Sorry for the late answer, there is no issue if we're running with
> i915.nuclear_flip=1.
> Should we go with Bob's patch then? :
> 
> https://patchwork.freedesktop.org/patch/80112/

Needs a giant hulking FIXME: Rip out when i915 is DRIVER_ATOMIC, plus cc
Maarten so he doesn't forget. But yeah seems much better.

> 
> >
> >>>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.
> >Needs a Testcase: igt/kms_pipe_color/$name line in the commit message. How
> >a bug was discovered and tested is a crucial part of a complete commit
> >message.
> >-Daniel
> 
> Will do.

Thanks, Daniel

> 
> >
> >>
> 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 
> Cc: Bob Paauwe 
> Cc: 
> Signed-off-by: Lionel Landwerlin 
> ---
>   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(>base,
> + config->degamma_lut_property, 0);
> + drm_object_property_set_value(>base,
> + config->ctm_property, 0);
> + drm_object_property_set_value(>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-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper

2016-04-18 Thread Lionel Landwerlin 

On 12/04/16 17:07, Daniel Vetter wrote:

On Tue, Apr 12, 2016 at 01:51:02PM +0100, Lionel Landwerlin wrote:

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

Nah, the problem is that we check fro DRIVER_ATOMIC in
drm_atomic_get_property, and because i915 is still not fully atomic that
gives us the wrong answer.

Can you pls double-check that enabling i915 atomic (i915.nuclear_flip=1)
also fixes the bug?

i915 is the only driver still transitioning, I definitely don't want to
have hacks all over for it.


Sorry for the late answer, there is no issue if we're running with 
i915.nuclear_flip=1.

Should we go with Bob's patch then? :

https://patchwork.freedesktop.org/patch/80112/




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.

Needs a Testcase: igt/kms_pipe_color/$name line in the commit message. How
a bug was discovered and tested is a crucial part of a complete commit
message.
-Daniel


Will do.






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 
Cc: Bob Paauwe 
Cc: 
Signed-off-by: Lionel Landwerlin 
---
  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(>base,
+   config->degamma_lut_property, 0);
+   drm_object_property_set_value(>base,
+   config->ctm_property, 0);
+   drm_object_property_set_value(>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-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper

2016-04-12 Thread Daniel Vetter 
On Tue, Apr 12, 2016 at 01:51:02PM +0100, Lionel Landwerlin wrote:
> 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

Nah, the problem is that we check fro DRIVER_ATOMIC in
drm_atomic_get_property, and because i915 is still not fully atomic that
gives us the wrong answer.

Can you pls double-check that enabling i915 atomic (i915.nuclear_flip=1)
also fixes the bug?

i915 is the only driver still transitioning, I definitely don't want to
have hacks all over for it.

> >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.

Needs a Testcase: igt/kms_pipe_color/$name line in the commit message. How
a bug was discovered and tested is a crucial part of a complete commit
message.
-Daniel

> 
> 
> >>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 
> >>Cc: Bob Paauwe 
> >>Cc: 
> >>Signed-off-by: Lionel Landwerlin 
> >>---
> >>  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(>base,
> >>+   config->degamma_lut_property, 0);
> >>+   drm_object_property_set_value(>base,
> >>+   config->ctm_property, 0);
> >>+   drm_object_property_set_value(>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-de...@lists.freedesktop.org
> >>https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper

2016-04-12 Thread Lionel Landwerlin 

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 
Cc: Bob Paauwe 
Cc: 
Signed-off-by: Lionel Landwerlin 
---
  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(>base,

+   config->degamma_lut_property, 0);
+   drm_object_property_set_value(>base,
+   config->ctm_property, 0);
+   drm_object_property_set_value(>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-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper

2016-04-12 Thread Daniel Vetter 
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?

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

> 
> 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 
> Cc: Bob Paauwe 
> Cc: 
> Signed-off-by: Lionel Landwerlin 
> ---
>  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(>base,
> + config->degamma_lut_property, 0);
> + drm_object_property_set_value(>base,
> + config->ctm_property, 0);
> + drm_object_property_set_value(>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-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper

2016-04-11 Thread Bob Paauwe 
On Mon, 11 Apr 2016 14:43:39 +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.
> 
> v2: Update non atomic values only if commit succeeds (Bob Paauwe)
> 
> v3: Do not access crtc_state after commit, use crtc->state (Maarten
> Lankhorst)
> 

Reviewed-by: Bob Paauwe 
Tested-by; Bob Paauwe 

> Cc: Maarten Lankhorst 
> Cc: Bob Paauwe 
> Cc: 
> Signed-off-by: Lionel Landwerlin 
> ---
>  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(>base,
> + config->degamma_lut_property, 0);
> + drm_object_property_set_value(>base,
> + config->ctm_property, 0);
> + drm_object_property_set_value(>base,
> + config->gamma_lut_property,
> + crtc->state->gamma_lut->base.id);
> +
>   /* Driver takes ownership of state on successful commit. */
>  
>   drm_property_unreference_blob(blob);



-- 
--
Bob Paauwe  
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper

2016-04-11 Thread Lionel Landwerlin 
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.

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 
Cc: Bob Paauwe 
Cc: 
Signed-off-by: Lionel Landwerlin 
---
 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(>base,
+   config->degamma_lut_property, 0);
+   drm_object_property_set_value(>base,
+   config->ctm_property, 0);
+   drm_object_property_set_value(>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

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper

2016-04-11 Thread Maarten Lankhorst 
Op 11-04-16 om 12:37 schreef Lionel Landwerlin:
> 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.
>
> v2: Update non atomic values only if commit succeeds (Bob Paauwe)
>
> Cc: Maarten Lankhorst 
> Cc: Bob Paauwe 
> Cc: 
> Signed-off-by: Lionel Landwerlin 
> ---
>  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..65e55ce 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(>base,
> + config->degamma_lut_property, 0);
> + drm_object_property_set_value(>base,
> + config->ctm_property, 0);
> + drm_object_property_set_value(>base,
> + config->gamma_lut_property,
> + crtc_state->gamma_lut->base.id);
> +
>   /* Driver takes ownership of state on successful commit. */
>  
>   drm_property_unreference_blob(blob);
You can't use crtc_state after drm_atomic_commit as it's freed, you need to use 
crtc->state.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper

2016-04-11 Thread Lionel Landwerlin 
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.

v2: Update non atomic values only if commit succeeds (Bob Paauwe)

Cc: Maarten Lankhorst 
Cc: Bob Paauwe 
Cc: 
Signed-off-by: Lionel Landwerlin 
---
 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..65e55ce 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(>base,
+   config->degamma_lut_property, 0);
+   drm_object_property_set_value(>base,
+   config->ctm_property, 0);
+   drm_object_property_set_value(>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

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper

2016-04-08 Thread Bob Paauwe 
On Fri, 8 Apr 2016 18:17:51 +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.
> 
> Cc: Maarten Lankhorst 
> Cc: Bob Paauwe 
> Cc: 
> Signed-off-by: Lionel Landwerlin 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 7bf678e..4aacd44 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2964,16 +2964,22 @@ retry:
>   config->degamma_lut_property, 0);
>   if (ret)
>   goto fail;
> + drm_object_property_set_value(>base,
> + config->degamma_lut_property, 0);
>  
>   ret = drm_atomic_crtc_set_property(crtc, crtc_state,
>   config->ctm_property, 0);
>   if (ret)
>   goto fail;
> + drm_object_property_set_value(>base,
> + config->ctm_property, 0);
>  
>   ret = drm_atomic_crtc_set_property(crtc, crtc_state,
>   config->gamma_lut_property, blob->base.id);
>   if (ret)
>   goto fail;
> + drm_object_property_set_value(>base,
> + config->gamma_lut_property, blob->base.id);
>  

This is similar to what I originally did to fix this problem.
But if the commit below fails, you've now changed the non atomic
values, but the atomic values would be reverted back to the original
state so they're out of sync again.  So just moving the set_value calls
to after the commit succeeds might be a better solution.

>   ret = drm_atomic_commit(state);
>   if (ret)



-- 
--
Bob Paauwe  
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm: atomic: fix legacy gamma set helper

2016-04-08 Thread Lionel Landwerlin 
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.

Cc: Maarten Lankhorst 
Cc: Bob Paauwe 
Cc: 
Signed-off-by: Lionel Landwerlin 
---
 drivers/gpu/drm/drm_atomic_helper.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 7bf678e..4aacd44 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2964,16 +2964,22 @@ retry:
config->degamma_lut_property, 0);
if (ret)
goto fail;
+   drm_object_property_set_value(>base,
+   config->degamma_lut_property, 0);
 
ret = drm_atomic_crtc_set_property(crtc, crtc_state,
config->ctm_property, 0);
if (ret)
goto fail;
+   drm_object_property_set_value(>base,
+   config->ctm_property, 0);
 
ret = drm_atomic_crtc_set_property(crtc, crtc_state,
config->gamma_lut_property, blob->base.id);
if (ret)
goto fail;
+   drm_object_property_set_value(>base,
+   config->gamma_lut_property, blob->base.id);
 
ret = drm_atomic_commit(state);
if (ret)
-- 
2.8.0.rc3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx