On Tue, Jan 14, 2025 at 01:31:20PM -0300, Gustavo Sousa wrote: > Quoting Jani Nikula (2025-01-14 12:21:50-03:00) > >On Mon, 13 Jan 2025, Gustavo Sousa <gustavo.sousa@xxxxxxxxx> wrote: > >> The CMTG is a timing generator that runs in parallel with transcoders > >> timing generators and can be used as a reference for synchronization. > >> > >> On PTL (display Xe3_LPD), we have observed that we are inheriting from > >> GOP a display configuration with the CMTG enabled. Because our driver > >> doesn't currently implement any CMTG sequences, the CMTG ends up still > >> enabled after our driver takes over. > >> > >> We need to make sure that the CMTG is not enabled if we are not going to > >> use it. For that, let's add a partial implementation in our driver that > >> only cares about disabling the CMTG if it was found enabled during > >> initial hardware readout. In the future, we can also implement sequences > >> for enabling CMTG if that becomes a needed feature. > > > >Doesn't this patch disable the CRTC, not the CMTG? > > It disables the CMTG and that's it for LNL and PTL. > > For platforms prior to LNL, disabling the CMTG requires a modeset; > specifically for those, the CRTC is also disabled during the > sanitization process (not sure if there is a clean way of forcing a > modeset from the sanitization routine). I'm not sure why this whole global state stuff is needed here. It seems to me that this should be handled more or less the same as port sync. Ie: - track the cmtg state in intel_crtc_state - read it out - add it to the state checker - add the necessary bits to the disable sequence (no need for enable right now I guess if we force a disable) - flag mode_changed=true for any crtc that has cmtg enabled in initial commit to force the modeset I guess the one open question is how to deal with cases where the same CMTG is used for two pipes (assuming that's a thing?). We may need to extend the port_sync master/slave handling in the enable/disable sequences to deal with cmtg as well to make sure things are done in the right order. Also it looks like CMTG is more or less a full blow trancoder (ie. has a full set of timing registers). The docs are rather confusing but it looks to me like they're saying that one should still program the normal transcoder registers as well, even when using CMTG. I guess if we ever implement proper support for this we should at least have some kind of sanity check to confirm that. -- Ville Syrjälä Intel
