Re: [PATCH i-g-t v2] tests/kms_color: Always use atomic_commit for setting color properties

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Feb 12, 2026 at 10:15:50AM +0000, Jason-JH Lin (林睿祥) wrote:
> > > > [snip]
> > > > 
> > > > Okay, I checked that drm_mode_obj_set_property_ioctl() called
> > > > from
> > > > drmModeObjectSetProperty() will synchronize the CRTC properties,
> > > > such
> > > > as, gamm_lut into the CRTC state of atomic_state.
> > > > 
> > > > Our drm driver has implemented mode_config.funcs-
> > > > >atomic_commit(), 
> > > > so maybe we have some bug in our drivers that cause the issue I
> > > > encountered, the original IGT code should work fine.
> > > > 
> > > > I'll check out this later and send the wrapper API if needed.
> > > > Thanks for the correction and guidance.
> > > > 
> > > 
> > > Finally, I found the issue is that color_mgmt_changed is not set
> > > when
> > > updating gamma_lut, causing gamma changes to not be applied to
> > > hardware.
> > > 
> > > Root cause analysis:
> > > - When updating GAMMA_LUT, color_mgmt_changed is set correctly.
> > > - But when a subsequent unrelated property (e.g. VRR_ENABLED) is
> > > committed, __drm_atomic_helper_crtc_duplicate_state() resets
> > > color_mgmt_changed=0.
> > > - This results in crtc->state having gamma_lut but
> > > color_mgmt_changed=0, so gamma is never applied to hardware.
> > > 
> > > DRM framework design:
> > > - color_mgmt_changed is a per-commit flag, reset on each new
> > > commit.
> > > - Only set if color management properties change.
> > > - In COMMIT_LEGACY mode (used by IGT), each property is committed
> > > separately.
> > > - Unrelated property commits overwrite color_mgmt_changed=1 from
> > > previous gamma_lut update.
> > > 
> > > Solution in MTK DRM driver:
> > > - Retain color_mgmt_changed if the old state has the flag and color
> > > management blobs have not changed, preventing it from being
> > > overwritten
> > > by unrelated property commits.
> > 
> > That shouldn't be needed because, as you noted, each property
> > is commited separately. So the commit resulting from the GAMMA_LUT
> > setproperty should have already updated the hardware before the
> > next setproperty commit happens.
> > 
> 
> But what I notice in the DRM driver is that GAMMA_LUT setproperty won't
> update the hardware immediately until other properties are set in the
> same atomic_flush().

Then you must have a bug somewhere. Each ioctl should
result in an atomic commit.

> 
> Here is the log from my notice:
> 
> [ 2021.157476] [Jason] set_property_atomic START: prop=GAMMA_LUT,
> val=125
> [ 2021.157478] mediatek-drm mediatek-drm.7.auto: [Jason]
> duplicate_state: OLD gamma_lut=0000000000000000 (id=0),
> ctm=0000000000000000
> [ 2021.157480] mediatek-drm mediatek-drm.7.auto: [Jason]
> duplicate_state: NEW gamma_lut=0000000000000000 (id=0),
> ctm=0000000000000000
> [ 2021.157481] [Jason] crtc_set_property: CRTC 0, prop=GAMMA_LUT,
> val=125
> [ 2021.157482] BEFORE set: state->gamma_lut=0000000000000000 (id=0)
> 
> [ 2021.157484] BEFORE set: state->ctm=0000000000000000 (id=0)
> [ 2021.157485] BEFORE set: state->color_mgmt_changed=0
> [ 2021.157486] [Jason] AFTER gamma_lut set: replaced=1,
> color_mgmt_changed=1
> [ 2021.157487] AFTER set: state->gamma_lut=00000000602291e1 (id=125)
> [ 2021.157489] [Jason] swap_state: CRTC 0
> [ 2021.157490] BEFORE swap: crtc->state->gamma_lut=0000000000000000
> (id=0)
> [ 2021.157491] BEFORE swap: crtc->state->ctm=0000000000000000 (id=0)
> [ 2021.157492] BEFORE swap: crtc->state->color_mgmt_changed=1
> [ 2021.157493] ATOMIC STATE: new_crtc_state->gamma_lut=00000000602291e1
> (id=125)
> [ 2021.157495] ATOMIC STATE: new_crtc_state->ctm=0000000000000000
> (id=0)
> [ 2021.157496] ATOMIC STATE: new_crtc_state->color_mgmt_changed=1
> [ 2021.157497] [Jason] set_property_atomic COMMIT SUCCESS:
> prop=GAMMA_LUT
> 
> [ 2021.157499] [Jason] set_property_atomic START: prop=VRR_ENABLED,
> val=0
> [ 2021.157501] mediatek-drm mediatek-drm.7.auto: [Jason]
> duplicate_state: OLD gamma_lut=00000000602291e1 (id=125),
> ctm=0000000000000000
> [ 2021.157503] mediatek-drm mediatek-drm.7.auto: [Jason]
> duplicate_state: NEW gamma_lut=00000000602291e1 (id=125),
> ctm=0000000000000000
> [ 2021.157504] [Jason] crtc_set_property: CRTC 0, prop=VRR_ENABLED,
> val=0
> [ 2021.157505] BEFORE set: state->gamma_lut=00000000602291e1 (id=125)
> [ 2021.157506] BEFORE set: state->ctm=0000000000000000 (id=0)
> [ 2021.157508] BEFORE set: state->color_mgmt_changed=0
> [ 2021.157509] [Jason] swap_state: CRTC 0
> [ 2021.157510] BEFORE swap: crtc->state->gamma_lut=00000000602291e1
> (id=125)
> [ 2021.157512] BEFORE swap: crtc->state->ctm=0000000000000000 (id=0)
> [ 2021.157513] BEFORE swap: crtc->state->color_mgmt_changed=1
> [ 2021.157514] ATOMIC STATE: new_crtc_state->gamma_lut=00000000602291e1
> (id=125)
> [ 2021.157515] ATOMIC STATE: new_crtc_state->ctm=0000000000000000
> (id=0)
> [ 2021.157517] ATOMIC STATE: new_crtc_state->color_mgmt_changed=0
> [ 2021.157518] [Jason] set_property_atomic COMMIT SUCCESS:
> prop=VRR_ENABLED
> 
> [ 2021.157530] [Jason] set_property_atomic START: prop=rotation, val=1
> [ 2021.157535] [Jason] set_property_atomic COMMIT SUCCESS:
> prop=rotation
> [ 2021.157587] mediatek-drm mediatek-drm.7.auto: [Jason]
> duplicate_state: OLD gamma_lut=00000000602291e1 (id=125),
> ctm=0000000000000000
> [ 2021.157589] mediatek-drm mediatek-drm.7.auto: [Jason]
> duplicate_state: NEW gamma_lut=00000000602291e1 (id=125),
> ctm=0000000000000000
> [ 2021.157605] [Jason] swap_state: CRTC 0
> 
> [ 2021.157606] BEFORE swap: crtc->state->gamma_lut=00000000602291e1
> (id=125)
> [ 2021.157607] BEFORE swap: crtc->state->ctm=0000000000000000 (id=0)
> [ 2021.157608] BEFORE swap: crtc->state->color_mgmt_changed=0
> [ 2021.157609] ATOMIC STATE: new_crtc_state->gamma_lut=00000000602291e1
> (id=125)
> [ 2021.157611] ATOMIC STATE: new_crtc_state->ctm=0000000000000000
> (id=0)
> [ 2021.157612] ATOMIC STATE: new_crtc_state->color_mgmt_changed=0
> [ 2021.157970] mediatek-disp-pmqos mediatek-disp-pmqos.20.auto:
> mtk_disp_pmqos_set_mmclk[196] final_level(freq=3, 624000000)
> [ 2021.361663] mediatek-drm mediatek-drm.7.auto: [Jason] atomic_flush:
> color_mgmt_changed=0, gamma_lut=00000000602291e1, ctm=0000000000000000
> 
> 
> From the log above, color_mgmt_changed will be reset to 0 after
> VRR_ENABLED setproperty and swap crtc_state.
> 
> But if we use atomic_commit from IGT, then this won't happen.
> 
> Regards,
> Jason-JH Lin

-- 
Ville Syrjälä
Intel