Hi Daniel, On Wednesday, October 16th, 2024 at 2:27 PM, Daniel Stone <daniel@xxxxxxxxxxxxx> wrote: > Hi all, > > On Wed, 16 Oct 2024 at 02:11, Andy Yan andyshrk@xxxxxxx wrote: > > > At 2024-10-16 04:13:40, "Piotr Zalewski" pZ010001011111@xxxxxxxxx wrote: > > > > > Ok I get it now. Is such rework correct? - when gamma LUT for rk356x is > > > being set, instead of disabling the LUT before the gamma LUT write for the > > > current CRTC's video port, active video port is selected. Selection is > > > based on if DSP LUT EN bit is set for particular video port. eg: > > > > If the userspace want to set gamma for CRTCx, then that is indeed where they want to set the > > gamma on。The driver silently sets the gamma on another CRTC, which is not what the user wants. > > > > I think there are two options： > > （1）return a error if gamma is enable on other CRTC， this is what we done in our BSP code[1] > > (2) disable the dsp_lut on privious CRTC, then switch to the current CRTC which userspace wants. > > > 1 is the only solution that can work. Silently changing the colour > properties of a separate CRTC is not OK, since this can lead to > displaying incorrect content. Ok right kernel keeps track of the state and sees gamma as enabled even if dsp lut en bit was cleared. Would it be better to check if gamma is already enabled on another CRTC in atomic_check rather than atomic_begin/atomic_flush (and silently fail) like in[1]? [1] https://github.com/armbian/linux-rockchip/blob/rk3576-6.1-dev-2024_04_19/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c#L3666 Best regards, Piotr Zalewski
