| Message ID | 1468579917-6727-3-git-send-email-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hey, Op 15-07-16 om 12:51 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) > > 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 | 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 3074c56..3269af4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13806,8 +13806,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, Does this compile with -nightly? The gamma_set atomic function should now return a int, so the return value from drm_atomic_helper_legacy_gamma_set should be returned here too. I've fixed the return value because of this patch, otherwise -ERESTARTSYS wasn't handled correctly. ~Maarten
On Mon, 18 Jul 2016 09:14:56 +0200 Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote: > Hey, > > Op 15-07-16 om 12:51 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) > > > > 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 | 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 3074c56..3269af4 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13806,8 +13806,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, > > Does this compile with -nightly? The gamma_set atomic function should now return a int, > so the return value from drm_atomic_helper_legacy_gamma_set should be returned here too. > I've fixed the return value because of this patch, otherwise -ERESTARTSYS wasn't handled > correctly. > > ~Maarten > Maarten, Not any more, you need to use Lionel's updated version. I think he missed updating the title as he made a couple of changes and it's up to version 4 now. Looks like you just reviewed his version. Bob
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3074c56..3269af4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13806,8 +13806,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,