* [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL.
@ 2016-04-08 16:26 Bob Paauwe
2016-04-08 17:10 ` ✗ Fi.CI.BAT: failure for " Patchwork
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Bob Paauwe @ 2016-04-08 16:26 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Set legacy properties when using legacy gamma set IOCTL.
2016-04-08 16:26 [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL Bob Paauwe
@ 2016-04-08 17:10 ` Patchwork
2016-04-08 17:21 ` [PATCH] " Lionel Landwerlin
` (3 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2016-04-08 17:10 UTC (permalink / raw)
To: Bob Paauwe; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL.
2016-04-08 16:26 [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL Bob Paauwe
2016-04-08 17:10 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-04-08 17:21 ` Lionel Landwerlin
2016-04-08 17:58 ` Bob Paauwe
2016-04-08 17:21 ` Lionel Landwerlin
` (2 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Lionel Landwerlin @ 2016-04-08 17:21 UTC (permalink / raw)
To: intel-gfx, bob.j.paauwe
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL.
2016-04-08 16:26 [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL Bob Paauwe
2016-04-08 17:10 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-04-08 17:21 ` [PATCH] " Lionel Landwerlin
@ 2016-04-08 17:21 ` Lionel Landwerlin
2016-04-18 16:47 ` [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2) Bob Paauwe
2016-04-27 15:03 ` ✗ Fi.CI.BAT: failure for drm/i915: Set legacy properties when using legacy gamma set IOCTL. (rev2) Patchwork
4 siblings, 0 replies; 20+ messages in thread
From: Lionel Landwerlin @ 2016-04-08 17:21 UTC (permalink / raw)
To: intel-gfx, bob.j.paauwe
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL.
2016-04-08 17:21 ` [PATCH] " Lionel Landwerlin
@ 2016-04-08 17:58 ` Bob Paauwe
0 siblings, 0 replies; 20+ messages in thread
From: Bob Paauwe @ 2016-04-08 17:58 UTC (permalink / raw)
To: Lionel Landwerlin; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-04-08 16:26 [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL Bob Paauwe
` (2 preceding siblings ...)
2016-04-08 17:21 ` Lionel Landwerlin
@ 2016-04-18 16:47 ` Bob Paauwe
2016-04-19 6:02 ` Maarten Lankhorst
2016-04-19 6:05 ` Maarten Lankhorst
2016-04-27 15:03 ` ✗ Fi.CI.BAT: failure for drm/i915: Set legacy properties when using legacy gamma set IOCTL. (rev2) Patchwork
4 siblings, 2 replies; 20+ messages in thread
From: Bob Paauwe @ 2016-04-18 16:47 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-04-18 16:47 ` [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2) Bob Paauwe
@ 2016-04-19 6:02 ` Maarten Lankhorst
2016-04-19 10:13 ` Lionel Landwerlin
2016-04-19 6:05 ` Maarten Lankhorst
1 sibling, 1 reply; 20+ messages in thread
From: Maarten Lankhorst @ 2016-04-19 6:02 UTC (permalink / raw)
To: Bob Paauwe, intel-gfx
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-04-18 16:47 ` [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2) Bob Paauwe
2016-04-19 6:02 ` Maarten Lankhorst
@ 2016-04-19 6:05 ` Maarten Lankhorst
2016-04-19 16:20 ` Bob Paauwe
1 sibling, 1 reply; 20+ messages in thread
From: Maarten Lankhorst @ 2016-04-19 6:05 UTC (permalink / raw)
To: Bob Paauwe, intel-gfx
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-04-19 6:02 ` Maarten Lankhorst
@ 2016-04-19 10:13 ` Lionel Landwerlin
2016-04-20 7:00 ` Maarten Lankhorst
0 siblings, 1 reply; 20+ messages in thread
From: Lionel Landwerlin @ 2016-04-19 10:13 UTC (permalink / raw)
To: Maarten Lankhorst, Bob Paauwe, intel-gfx
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-04-19 6:05 ` Maarten Lankhorst
@ 2016-04-19 16:20 ` Bob Paauwe
2016-04-20 6:57 ` Maarten Lankhorst
0 siblings, 1 reply; 20+ messages in thread
From: Bob Paauwe @ 2016-04-19 16:20 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-04-19 16:20 ` Bob Paauwe
@ 2016-04-20 6:57 ` Maarten Lankhorst
0 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2016-04-20 6:57 UTC (permalink / raw)
To: Bob Paauwe; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-04-19 10:13 ` Lionel Landwerlin
@ 2016-04-20 7:00 ` Maarten Lankhorst
2016-04-20 12:38 ` Daniel Vetter
0 siblings, 1 reply; 20+ messages in thread
From: Maarten Lankhorst @ 2016-04-20 7:00 UTC (permalink / raw)
To: Lionel Landwerlin, Bob Paauwe, intel-gfx
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-04-20 7:00 ` Maarten Lankhorst
@ 2016-04-20 12:38 ` Daniel Vetter
2016-05-04 13:49 ` Lionel Landwerlin
0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2016-04-20 12:38 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Set legacy properties when using legacy gamma set IOCTL. (rev2)
2016-04-08 16:26 [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL Bob Paauwe
` (3 preceding siblings ...)
2016-04-18 16:47 ` [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2) Bob Paauwe
@ 2016-04-27 15:03 ` Patchwork
4 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2016-04-27 15:03 UTC (permalink / raw)
To: Bob Paauwe; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-04-20 12:38 ` Daniel Vetter
@ 2016-05-04 13:49 ` Lionel Landwerlin
2016-05-09 6:52 ` Daniel Vetter
0 siblings, 1 reply; 20+ messages in thread
From: Lionel Landwerlin @ 2016-05-04 13:49 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-05-04 13:49 ` Lionel Landwerlin
@ 2016-05-09 6:52 ` Daniel Vetter
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2016-05-09 6:52 UTC (permalink / raw)
To: Lionel Landwerlin; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-07-15 16:13 ` Bob Paauwe
@ 2016-07-18 14:27 ` Maarten Lankhorst
0 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2016-07-18 14:27 UTC (permalink / raw)
To: Bob Paauwe, Lionel Landwerlin; +Cc: intel-gfx
Op 15-07-16 om 18:13 schreef Bob Paauwe:
> On Fri, 15 Jul 2016 14:59:02 +0100
> Lionel Landwerlin <lionel.g.landwerlin@intel.com> wrote:
>
>> From: Bob Paauwe <bob.j.paauwe@intel.com>
>>
>> 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)
>>
>> v3: Update arguments of the gamma_set vfunc (Lionel)
>>
>> v4: Fixed vfunc prototype (Lionel)
> Lionel,
>
> The changes looks correct to me, thanks! Is it appropriate for me to
> add a reviewed-by for those changes?
I think the signed-off-by is strong enough, patch applied, thanks.
Pushed.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-07-15 13:59 ` [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2) Lionel Landwerlin
2016-07-15 16:13 ` Bob Paauwe
@ 2016-07-18 7:50 ` Maarten Lankhorst
1 sibling, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2016-07-18 7:50 UTC (permalink / raw)
To: Lionel Landwerlin, intel-gfx
Op 15-07-16 om 15:59 schreef Lionel Landwerlin:
> From: Bob Paauwe <bob.j.paauwe@intel.com>
>
> 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)
>
> v3: Update arguments of the gamma_set vfunc (Lionel)
>
> v4: Fixed vfunc prototype (Lionel)
>
> igt-testcase: kms_pipe_color / legacy-gamma-reset-pipeX
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-07-15 13:59 ` [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2) Lionel Landwerlin
@ 2016-07-15 16:13 ` Bob Paauwe
2016-07-18 14:27 ` Maarten Lankhorst
2016-07-18 7:50 ` Maarten Lankhorst
1 sibling, 1 reply; 20+ messages in thread
From: Bob Paauwe @ 2016-07-15 16:13 UTC (permalink / raw)
To: Lionel Landwerlin; +Cc: intel-gfx
On Fri, 15 Jul 2016 14:59:02 +0100
Lionel Landwerlin <lionel.g.landwerlin@intel.com> wrote:
> From: Bob Paauwe <bob.j.paauwe@intel.com>
>
> 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)
>
> v3: Update arguments of the gamma_set vfunc (Lionel)
>
> v4: Fixed vfunc prototype (Lionel)
Lionel,
The changes looks correct to me, thanks! Is it appropriate for me to
add a reviewed-by for those changes?
Bob
>
> igt-testcase: kms_pipe_color / legacy-gamma-reset-pipeX
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 44 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9337d3a..fb7d8fc5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13924,8 +13924,50 @@ 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 int intel_atomic_legacy_gamma_set(struct drm_crtc *crtc,
> + u16 *red, u16 *green, u16 *blue,
> + uint32_t size)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_crtc_state *state;
> + int ret;
> +
> + ret = drm_atomic_helper_legacy_gamma_set(crtc, red, green, blue, size);
> + if (ret)
> + return ret;
> +
> + /*
> + * 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);
> +
> + return 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2)
2016-07-15 10:59 [PATCH] Nightly color management fixes Lionel Landwerlin
@ 2016-07-15 13:59 ` Lionel Landwerlin
2016-07-15 16:13 ` Bob Paauwe
2016-07-18 7:50 ` Maarten Lankhorst
0 siblings, 2 replies; 20+ messages in thread
From: Lionel Landwerlin @ 2016-07-15 13:59 UTC (permalink / raw)
To: intel-gfx
From: Bob Paauwe <bob.j.paauwe@intel.com>
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)
v3: Update arguments of the gamma_set vfunc (Lionel)
v4: Fixed vfunc prototype (Lionel)
igt-testcase: kms_pipe_color / legacy-gamma-reset-pipeX
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 44 +++++++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9337d3a..fb7d8fc5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13924,8 +13924,50 @@ 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 int intel_atomic_legacy_gamma_set(struct drm_crtc *crtc,
+ u16 *red, u16 *green, u16 *blue,
+ uint32_t size)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_crtc_state *state;
+ int ret;
+
+ ret = drm_atomic_helper_legacy_gamma_set(crtc, red, green, blue, size);
+ if (ret)
+ return ret;
+
+ /*
+ * 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);
+
+ return 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.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-07-18 14:27 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-08 16:26 [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL Bob Paauwe
2016-04-08 17:10 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-04-08 17:21 ` [PATCH] " Lionel Landwerlin
2016-04-08 17:58 ` Bob Paauwe
2016-04-08 17:21 ` Lionel Landwerlin
2016-04-18 16:47 ` [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2) Bob Paauwe
2016-04-19 6:02 ` Maarten Lankhorst
2016-04-19 10:13 ` Lionel Landwerlin
2016-04-20 7:00 ` Maarten Lankhorst
2016-04-20 12:38 ` Daniel Vetter
2016-05-04 13:49 ` Lionel Landwerlin
2016-05-09 6:52 ` Daniel Vetter
2016-04-19 6:05 ` Maarten Lankhorst
2016-04-19 16:20 ` Bob Paauwe
2016-04-20 6:57 ` Maarten Lankhorst
2016-04-27 15:03 ` ✗ Fi.CI.BAT: failure for drm/i915: Set legacy properties when using legacy gamma set IOCTL. (rev2) Patchwork
2016-07-15 10:59 [PATCH] Nightly color management fixes Lionel Landwerlin
2016-07-15 13:59 ` [PATCH] drm/i915: Set legacy properties when using legacy gamma set IOCTL. (v2) Lionel Landwerlin
2016-07-15 16:13 ` Bob Paauwe
2016-07-18 14:27 ` Maarten Lankhorst
2016-07-18 7:50 ` Maarten Lankhorst
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.