On Tue, Aug 28, 2018 at 05:08:51PM +0100, Alexandru-Cosmin Gheorghe wrote:
On Tue, Aug 28, 2018 at 04:48:45PM +0100, Brian Starkey wrote:Hi Alex, On Tue, Aug 28, 2018 at 04:33:20PM +0100, Alexandru Gheorghe wrote: >When doing suspend/resume drivers usually use the >drm_atomic_helper_suspend/drm_atomic_helper_resume pair for saving the >state and then re-comitting it. > >The problem is that drm_crtc_state has a bool field called >color_mgmt_changed, which mali-dp and other drivers uses it to detect >if coefficients need to be reprogrammed, but that never happens >because the saved state has color_mgmt_changed set to 0. > >Fix that by setting color_mgmt_changed to true in >drm_atomic_helper_check_modeset when at least one of gamma_lut, >degamma_lut, ctm is different between the new and the old crtc state. > >Also, this makes unnecessary the setting of color_mgmt_changed in >places where gamma_lut/degamma_lut/ctm are set in the new crtc_state. > >Changes since v2: > - Instead of setting color_mgmt_changed in commit_duplicated_set > just set it in check_modeset and clean up other place where it was > set, suggested by Maarten Lankhorst. > >Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@xxxxxxx> >--- >drivers/gpu/drm/drm_atomic.c | 12 +++--------- >drivers/gpu/drm/drm_atomic_helper.c | 8 +++++++- >drivers/gpu/drm/drm_fb_helper.c | 1 - >3 files changed, 10 insertions(+), 11 deletions(-) > >diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c >index d0478abc01bd..9bcada3c299e 100644 >--- a/drivers/gpu/drm/drm_atomic.c >+++ b/drivers/gpu/drm/drm_atomic.c >@@ -554,29 +554,23 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, > drm_property_blob_put(mode); > return ret; > } else if (property == config->degamma_lut_property) { >- ret = drm_atomic_replace_property_blob_from_id(dev, >+ return drm_atomic_replace_property_blob_from_id(dev, > &state->degamma_lut, > val, > -1, sizeof(struct drm_color_lut), > &replaced); >- state->color_mgmt_changed |= replaced; >- return ret; > } else if (property == config->ctm_property) { >- ret = drm_atomic_replace_property_blob_from_id(dev, >+ return drm_atomic_replace_property_blob_from_id(dev, > &state->ctm, > val, > sizeof(struct drm_color_ctm), -1, > &replaced); >- state->color_mgmt_changed |= replaced; >- return ret; > } else if (property == config->gamma_lut_property) { >- ret = drm_atomic_replace_property_blob_from_id(dev, >+ return drm_atomic_replace_property_blob_from_id(dev, > &state->gamma_lut, > val, > -1, sizeof(struct drm_color_lut), > &replaced); >- state->color_mgmt_changed |= replaced; >- return ret; Looks like 'replaced' is now unused, so you can remove it.It's needed as an argument for drm_atomic_replace_property_blob_from_id, and the prototype of the function makes sense to me.
Silly me, I thought drm_atomic_replace_property_blob_from_id would check it against NULL and skip, but I see it doesn't, and that'll teach me to not trust my memory :-) Sorry for the noise, -Brian
Cheers, -Brian > } else if (property == config->prop_out_fence_ptr) { > s32 __user *fence_ptr = u64_to_user_ptr(val); > >diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c >index 2c23a48482da..fe22e1ad468a 100644 >--- a/drivers/gpu/drm/drm_atomic_helper.c >+++ b/drivers/gpu/drm/drm_atomic_helper.c >@@ -611,6 +611,13 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, > > return -EINVAL; > } >+ if (new_crtc_state->degamma_lut != old_crtc_state->degamma_lut || >+ new_crtc_state->ctm != old_crtc_state->ctm || >+ new_crtc_state->gamma_lut != old_crtc_state->gamma_lut) { >+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] color management changed\n", >+ crtc->base.id, crtc->name); >+ new_crtc_state->color_mgmt_changed = true; >+ } > } > > ret = handle_conflicting_encoders(state, false); >@@ -3947,7 +3954,6 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, > replaced = drm_property_replace_blob(&crtc_state->degamma_lut, NULL); > replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL); > replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, blob); >- crtc_state->color_mgmt_changed |= replaced; > > ret = drm_atomic_commit(state); > >diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >index 4b0dd20bccb8..8541e95a5410 100644 >--- a/drivers/gpu/drm/drm_fb_helper.c >+++ b/drivers/gpu/drm/drm_fb_helper.c >@@ -1442,7 +1442,6 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info) > replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL); > replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, > gamma_lut); >- crtc_state->color_mgmt_changed |= replaced; > } > > ret = drm_atomic_commit(state); >-- >2.18.0 >-- Cheers, Alex G
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
