[PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL.

[PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL.

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

igt-testcase: kms_pipe_color / legacy-gamma-reset-pipeX
Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5155efb6..ff09a18 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13885,8 +13885,44 @@ out:
 
 #undef for_each_intel_crtc_masked
 
+/*
+ * TODO: Remove this once we have full atomic implmented.
+ */
+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);
+
+	/*
+	 * Make sure we update the legacy properties so this works when
+	 * atomic is not enabled.
+	 */
+
+	state = crtc->state;
+
+	drm_object_property_set_value(&crtc->base,
+				      config->degamma_lut_property,
+				      (state->degamma_lut) ?
+				      state->degamma_lut->base.id : 0);
+
+	drm_object_property_set_value(&crtc->base,
+				      config->ctm_property,
+				      (state->ctm) ?
+				      state->ctm->base.id : 0);
+
+	drm_object_property_set_value(&crtc->base,
+				      config->gamma_lut_property,
+				      (state->gamma_lut) ?
+				      state->gamma_lut->base.id : 0);
+}
+
 static const struct drm_crtc_funcs intel_crtc_funcs = {
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
+	.gamma_set = intel_atomic_legacy_gamma_set,
 	.set_config = drm_atomic_helper_set_config,
 	.set_property = drm_atomic_helper_crtc_set_property,
 	.destroy = intel_crtc_destroy,
-- 
2.5.5

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

✗ Fi.CI.BAT: failure for drm/i915: Set legacy properties when using legacy gamma set IOCTL.

[Patchwork]

== Series Details ==

Series: drm/i915: Set legacy properties when using legacy gamma set IOCTL.
URL   : https://patchwork.freedesktop.org/series/5466/
State : failure

== Summary ==

Series 5466v1 drm/i915: Set legacy properties when using legacy gamma set IOCTL.
http://patchwork.freedesktop.org/api/1.0/series/5466/revisions/1/mbox/

Test core_auth:
        Subgroup basic-auth:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_basic:
        Subgroup create-fd-close:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_cs_prefetch:
        Subgroup basic-default:
                skip       -> INCOMPLETE (bdw-nuci7)
Test gem_cs_tlb:
        Subgroup basic-default:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_ctx_basic:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_ctx_param_basic:
        Subgroup basic:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup invalid-ctx-get:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup invalid-param-set:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup invalid-size-get:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup non-root-set:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup root-set-no-zeromap-disabled:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_exec_basic:
        Subgroup basic-bsd:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup basic-render:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup gtt-bsd2:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup gtt-vebox:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup readonly-render:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_exec_nop:
        Subgroup basic:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_exec_parse:
        Subgroup basic-allowed:
                skip       -> INCOMPLETE (bdw-nuci7)
Test gem_exec_store:
        Subgroup basic-all:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup basic-blt:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup basic-render:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_exec_suspend:
        Subgroup basic-s3:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test gem_flink_basic:
        Subgroup basic:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_mmap_gtt:
        Subgroup basic-short:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup basic-small-bo-tiledy:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup basic-write-read:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_render_tiled_blits:
        Subgroup basic:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_storedw_loop:
        Subgroup basic-bsd2:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_sync:
        Subgroup basic-bsd:
                dmesg-warn -> PASS       (bsw-nuc-2)
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup basic-render:
                pass       -> INCOMPLETE (bdw-nuci7) UNSTABLE
        Subgroup basic-vebox:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_tiled_fence_blits:
        Subgroup basic:
                pass       -> INCOMPLETE (bdw-nuci7)
Test gem_tiled_pread_basic:
                pass       -> INCOMPLETE (bdw-nuci7)
Test kms_addfb_basic:
        Subgroup addfb25-framebuffer-vs-set-tiling:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup addfb25-x-tiled:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup addfb25-y-tiled-small:
                skip       -> INCOMPLETE (bdw-nuci7)
        Subgroup bad-pitch-128:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup bad-pitch-32:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup bad-pitch-65536:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup small-bo:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup too-wide:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup unused-offsets:
                pass       -> INCOMPLETE (bdw-nuci7)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
Test kms_force_connector_basic:
        Subgroup prune-stale-modes:
                pass       -> SKIP       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup read-crc-pipe-c-frame-sequence:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> INCOMPLETE (skl-nuci5)
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                pass       -> INCOMPLETE (bdw-nuci7)
Test kms_sink_crc_basic:
                skip       -> INCOMPLETE (bdw-nuci7)
Test pm_rpm:
        Subgroup basic-rte:
                dmesg-warn -> PASS       (bsw-nuc-2)
                pass       -> INCOMPLETE (bdw-nuci7)
Test prime_self_import:
        Subgroup basic-llseek-size:
                pass       -> INCOMPLETE (bdw-nuci7)
        Subgroup basic-with_fd_dup:
                pass       -> INCOMPLETE (bdw-nuci7)

bdw-nuci7        total:196  pass:141  dwarn:0   dfail:0   fail:0   skip:8  
bdw-ultra        total:196  pass:175  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:196  pass:159  dwarn:0   dfail:0   fail:0   skip:37 
byt-nuc          total:196  pass:161  dwarn:0   dfail:0   fail:0   skip:35 
hsw-brixbox      total:196  pass:174  dwarn:0   dfail:0   fail:0   skip:22 
ilk-hp8440p      total:196  pass:130  dwarn:1   dfail:0   fail:0   skip:65 
ivb-t430s        total:196  pass:171  dwarn:0   dfail:0   fail:0   skip:25 
skl-i7k-2        total:196  pass:173  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:6    pass:5    dwarn:0   dfail:0   fail:0   skip:0  
snb-dellxps      total:196  pass:162  dwarn:0   dfail:0   fail:0   skip:34 
snb-x220t        total:196  pass:162  dwarn:0   dfail:0   fail:1   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1849/

949884a57b51aa158e3ae9afe1f08130cdb7a3ef drm-intel-nightly: 2016y-04m-08d-10h-45m-28s UTC integration manifest
144898b87150b905358689aeeeba8befac63e10e drm/i915: Set legacy properties when using legacy gamma set IOCTL.

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

Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL.

[Lionel Landwerlin]

Hi Paul,

Unfortunate that we've been writing the same patch at the same time :(
I think this we've got pretty much the same fix, but it probably needs 
to be done at the DRM level, because this can impact other drivers too.

Cheers,

-
Lionel

On 08/04/16 17:26, Bob Paauwe wrote:
> 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.
>
> igt-testcase: kms_pipe_color / legacy-gamma-reset-pipeX
> Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++++++++++++++++-
>   1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5155efb6..ff09a18 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13885,8 +13885,44 @@ out:
>   
>   #undef for_each_intel_crtc_masked
>   
> +/*
> + * TODO: Remove this once we have full atomic implmented.
> + */
> +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);
> +
> +	/*
> +	 * Make sure we update the legacy properties so this works when
> +	 * atomic is not enabled.
> +	 */
> +
> +	state = crtc->state;
> +
> +	drm_object_property_set_value(&crtc->base,
> +				      config->degamma_lut_property,
> +				      (state->degamma_lut) ?
> +				      state->degamma_lut->base.id : 0);
> +
> +	drm_object_property_set_value(&crtc->base,
> +				      config->ctm_property,
> +				      (state->ctm) ?
> +				      state->ctm->base.id : 0);
> +
> +	drm_object_property_set_value(&crtc->base,
> +				      config->gamma_lut_property,
> +				      (state->gamma_lut) ?
> +				      state->gamma_lut->base.id : 0);
> +}
> +
>   static const struct drm_crtc_funcs intel_crtc_funcs = {
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
> +	.gamma_set = intel_atomic_legacy_gamma_set,
>   	.set_config = drm_atomic_helper_set_config,
>   	.set_property = drm_atomic_helper_crtc_set_property,
>   	.destroy = intel_crtc_destroy,

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

Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL.

[Bob Paauwe]

On Fri, 8 Apr 2016 18:21:51 +0100
Lionel Landwerlin <llandwerlin@gmail.com> wrote:

> Hi Paul,
> 
> Unfortunate that we've been writing the same patch at the same time :(
> I think this we've got pretty much the same fix, but it probably needs 
> to be done at the DRM level, because this can impact other drivers too.
> 
> Cheers,
> 
> -
> Lionel

In this case, it's happening because the i915 driver is using the
atomic set property paths to implement the set gamma IOCTL. This is
fine if the i915 driver is fully using atomic (nuclear_pageflip = true)
because queries for the properties will go through the atomic path.
But if i915 is not using atomic, then queries for the properties will
not go through the atomic path and will get the incorrect or out of
sync legacy property values.

So this would only effect other drivers if they implemented the set
gamma IOCTL the same way and have two code paths for querying the
properties.

This patch is really a temporary fix until we move to
nuclear_pageflip=true as the default/only mode for the i915 driver.

Bob
> 
> On 08/04/16 17:26, Bob Paauwe wrote:
> > 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.
> >
> > igt-testcase: kms_pipe_color / legacy-gamma-reset-pipeX
> > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++++++++++++++++-
> >   1 file changed, 37 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 5155efb6..ff09a18 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13885,8 +13885,44 @@ out:
> >   
> >   #undef for_each_intel_crtc_masked
> >   
> > +/*
> > + * TODO: Remove this once we have full atomic implmented.
> > + */
> > +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);
> > +
> > +	/*
> > +	 * Make sure we update the legacy properties so this works when
> > +	 * atomic is not enabled.
> > +	 */
> > +
> > +	state = crtc->state;
> > +
> > +	drm_object_property_set_value(&crtc->base,
> > +				      config->degamma_lut_property,
> > +				      (state->degamma_lut) ?
> > +				      state->degamma_lut->base.id : 0);
> > +
> > +	drm_object_property_set_value(&crtc->base,
> > +				      config->ctm_property,
> > +				      (state->ctm) ?
> > +				      state->ctm->base.id : 0);
> > +
> > +	drm_object_property_set_value(&crtc->base,
> > +				      config->gamma_lut_property,
> > +				      (state->gamma_lut) ?
> > +				      state->gamma_lut->base.id : 0);
> > +}
> > +
> >   static const struct drm_crtc_funcs intel_crtc_funcs = {
> > -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
> > +	.gamma_set = intel_atomic_legacy_gamma_set,
> >   	.set_config = drm_atomic_helper_set_config,
> >   	.set_property = drm_atomic_helper_crtc_set_property,
> >   	.destroy = intel_crtc_destroy,  
> 



-- 
--
Bob Paauwe                  
Bob.J.Paauwe@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

Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL.

[Lionel Landwerlin]

Hi Paul,

Unfortunate that we've been writing the same patch at the same time :(
I think this we've got pretty much the same fix, but it probably needs 
to be done at the DRM level, because this can impact other drivers too.

Cheers,

-
Lionel

On 08/04/16 17:26, Bob Paauwe wrote:
> 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.
>
> igt-testcase: kms_pipe_color / legacy-gamma-reset-pipeX
> Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++++++++++++++++-
>   1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5155efb6..ff09a18 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13885,8 +13885,44 @@ out:
>   
>   #undef for_each_intel_crtc_masked
>   
> +/*
> + * TODO: Remove this once we have full atomic implmented.
> + */
> +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);
> +
> +	/*
> +	 * Make sure we update the legacy properties so this works when
> +	 * atomic is not enabled.
> +	 */
> +
> +	state = crtc->state;
> +
> +	drm_object_property_set_value(&crtc->base,
> +				      config->degamma_lut_property,
> +				      (state->degamma_lut) ?
> +				      state->degamma_lut->base.id : 0);
> +
> +	drm_object_property_set_value(&crtc->base,
> +				      config->ctm_property,
> +				      (state->ctm) ?
> +				      state->ctm->base.id : 0);
> +
> +	drm_object_property_set_value(&crtc->base,
> +				      config->gamma_lut_property,
> +				      (state->gamma_lut) ?
> +				      state->gamma_lut->base.id : 0);
> +}
> +
>   static const struct drm_crtc_funcs intel_crtc_funcs = {
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
> +	.gamma_set = intel_atomic_legacy_gamma_set,
>   	.set_config = drm_atomic_helper_set_config,
>   	.set_property = drm_atomic_helper_crtc_set_property,
>   	.destroy = intel_crtc_destroy,

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

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

[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@intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Bob Paauwe <bob.j.paauwe@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);
+
+	/*
+	 * Make sure we update the legacy properties so this works when
+	 * atomic is not enabled.
+	 */
+
+	state = crtc->state;
+
+	drm_object_property_set_value(&crtc->base,
+				      config->degamma_lut_property,
+				      (state->degamma_lut) ?
+				      state->degamma_lut->base.id : 0);
+
+	drm_object_property_set_value(&crtc->base,
+				      config->ctm_property,
+				      (state->ctm) ?
+				      state->ctm->base.id : 0);
+
+	drm_object_property_set_value(&crtc->base,
+				      config->gamma_lut_property,
+				      (state->gamma_lut) ?
+				      state->gamma_lut->base.id : 0);
+}
+
 static const struct drm_crtc_funcs intel_crtc_funcs = {
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
+	.gamma_set = intel_atomic_legacy_gamma_set,
 	.set_config = drm_atomic_helper_set_config,
 	.set_property = drm_atomic_helper_crtc_set_property,
 	.destroy = intel_crtc_destroy,
-- 
2.5.5

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

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

[Maarten Lankhorst]

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)
>
Seems to me that this is fixed by [PATCH] drm: atomic: fix legacy gamma set helper?
Not sure why this patch is required on top.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Lionel Landwerlin]

On 19/04/16 07:02, Maarten Lankhorst 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)
>>
> Seems to me that this is fixed by [PATCH] drm: atomic: fix legacy gamma set helper?
> Not sure why this patch is required on top.
>
Daniel would prefer to have this fix only in the i915 driver as this is 
the last driver still transitioning to atomic.

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

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

[Maarten Lankhorst]

Op 19-04-16 om 12:13 schreef Lionel Landwerlin:
> On 19/04/16 07:02, Maarten Lankhorst 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)
>>>
>> Seems to me that this is fixed by [PATCH] drm: atomic: fix legacy gamma set helper?
>> Not sure why this patch is required on top.
>>
> Daniel would prefer to have this fix only in the i915 driver as this is the last driver still transitioning to atomic.
>
Oh indeed, lack of DRIVER_ATOMIC cap again. :(
Still feels like something that doesn't belong in the driver, but ok..

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

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

[Daniel Vetter]

On Wed, Apr 20, 2016 at 09:00:09AM +0200, Maarten Lankhorst wrote:
> Op 19-04-16 om 12:13 schreef Lionel Landwerlin:
> > On 19/04/16 07:02, Maarten Lankhorst 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)
> >>>
> >> Seems to me that this is fixed by [PATCH] drm: atomic: fix legacy gamma set helper?
> >> Not sure why this patch is required on top.
> >>
> > Daniel would prefer to have this fix only in the i915 driver as this is the last driver still transitioning to atomic.
> >
> Oh indeed, lack of DRIVER_ATOMIC cap again. :(
> Still feels like something that doesn't belong in the driver, but ok..

Agreed, hence why I asked for a FIXME comment so we don't forget to rip
this out once we enale DRIVER_ATOMIC for real.
-Daniel
-- 
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: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)

[Lionel Landwerlin]

On 20/04/16 13:38, Daniel Vetter wrote:
> On Wed, Apr 20, 2016 at 09:00:09AM +0200, Maarten Lankhorst wrote:
>> Op 19-04-16 om 12:13 schreef Lionel Landwerlin:
>>> On 19/04/16 07:02, Maarten Lankhorst 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)
>>>>>
>>>> Seems to me that this is fixed by [PATCH] drm: atomic: fix legacy gamma set helper?
>>>> Not sure why this patch is required on top.
>>>>
>>> Daniel would prefer to have this fix only in the i915 driver as this is the last driver still transitioning to atomic.
>>>
>> Oh indeed, lack of DRIVER_ATOMIC cap again. :(
>> Still feels like something that doesn't belong in the driver, but ok..
> Agreed, hence why I asked for a FIXME comment so we don't forget to rip
> this out once we enale DRIVER_ATOMIC for real.
> -Daniel
Hi Daniel,

Was there anything missing in the last version of this patch?

Thanks,

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

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

[Daniel Vetter]

On Wed, May 04, 2016 at 02:49:50PM +0100, Lionel Landwerlin wrote:
> On 20/04/16 13:38, Daniel Vetter wrote:
> >On Wed, Apr 20, 2016 at 09:00:09AM +0200, Maarten Lankhorst wrote:
> >>Op 19-04-16 om 12:13 schreef Lionel Landwerlin:
> >>>On 19/04/16 07:02, Maarten Lankhorst 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)
> >>>>>
> >>>>Seems to me that this is fixed by [PATCH] drm: atomic: fix legacy gamma set helper?
> >>>>Not sure why this patch is required on top.
> >>>>
> >>>Daniel would prefer to have this fix only in the i915 driver as this is the last driver still transitioning to atomic.
> >>>
> >>Oh indeed, lack of DRIVER_ATOMIC cap again. :(
> >>Still feels like something that doesn't belong in the driver, but ok..
> >Agreed, hence why I asked for a FIXME comment so we don't forget to rip
> >this out once we enale DRIVER_ATOMIC for real.
> >-Daniel
> Hi Daniel,
> 
> Was there anything missing in the last version of this patch?

Pending review comments from Maarten, and no r-b tag.
-Daniel
-- 
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: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)

[Maarten Lankhorst]

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@intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Bob Paauwe <bob.j.paauwe@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.

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

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

[Bob Paauwe]

On Tue, 19 Apr 2016 08:05:26 +0200
Maarten Lankhorst <maarten.lankhorst@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@intel.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Bob Paauwe <bob.j.paauwe@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?

Bob

> 
> ~Maarten



-- 
--
Bob Paauwe                  
Bob.J.Paauwe@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

Re: [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@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@intel.com>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Signed-off-by: Bob Paauwe <bob.j.paauwe@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: Set legacy properties when using legacy gamma set IOCTL. (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Set legacy properties when using legacy gamma set IOCTL. (rev2)
URL   : https://patchwork.freedesktop.org/series/5466/
State : failure

== Summary ==

  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/kconfig/conf.o
  SHIPPED scripts/kconfig/zconf.tab.c
  SHIPPED scripts/kconfig/zconf.lex.c
  SHIPPED scripts/kconfig/zconf.hash.c
Makefile:538: recipe for target 'olddefconfig' failed
make: *** [olddefconfig] Terminated

Full logs at /archive/deploy/logs/CI_Patchwork_build_2092

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