On Wed, Jan 15, 2025 at 04:41:05PM -0300, Gustavo Sousa wrote: > Quoting Gustavo Sousa (2025-01-15 13:18:48-03:00) > >Quoting Ville Syrjälä (2025-01-15 12:07:39-03:00) > >>On Wed, Jan 15, 2025 at 09:44:14AM -0300, Gustavo Sousa wrote: > >>> Quoting Ville Syrjälä (2025-01-14 14:32:45-03:00) > >>> >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 > >>> > >>> The main reasons I implemented CMTG state as a global state were that > >>> CMTG is not a exactly per-pipe thing and it could affect multiple pipes > >>> (A and B), at least not on pre-LNL platforms. > >> > >>I suppose. But it doesn't seem to be fully really independent > >>thing either especially given the dependency to the port PLL > >>and such, and that's all handled per-pipe. > > > >To make matters worse, it is possible for CMTG A being driven by PHY B > >and vice-versa. > > So... I'm trying to come up with a way to handle CMTG state as part of > the intel_crtc_state. I have some questions that I was hoping you could > help me with... > > 1) For those pre-LNL platforms that have a single CMTG, what would be > your suggestion? > > I was thinking about keeping the state on pipe A's intel_crtc_state, but > then how to handle when pipe B's eDP TG is sync'ing with the CMTG? > Should we just pull in pipe A's into the atomic state and deal with it? I was thinking we could just have a bitmask of pipes just like with port sync. If one needs a modeset we could then suck all of them in. Althought for just the initial disable thing we'd not really need even that I guess since we'd any flag all of them. I suppose the one whose port PLL is providing the clock should be considered the primary for the purposes of the modeset sequence. > > If it is just transcoder B's eDP that is hooked up wit the CMTG, pulling > pipe A into the atomic state only to handle the CMTG seems rather > unnecessary to me. Just accept it and live on? > > 2) As of LNL, eDP A would sync only with CMTG A and eDP B, with CMTG B. > So, I guess having each state in the respective intel_crtc_state > seems okay here. > > If we were to encounter a CMTG dual sync mode (is it fair to > consider that a possibility from the GOP?), since only care about > disabling of CMTGs for now, I guess we do not need to worry about > turning sure the secondary CMTG (which will also be disabled) into > primary, right? Yeah, just making sure the thing gets disabled more or less properly should suffice for now. > > 3) There is also the case that we could have a CMTG (the single one in > pre-LNL; A or B for as of LNL) being clocked by a PHY that is not > being used to drive any transcoder. Not sure we could expect that > from GOP, but it is nevertheless a valid configuration. Is there even a way to turn on a port PLL without turning on the whole port in the current hw with its per-port PLLs? > > We probably wouldn't be able to disable the CMTG during the initial > modeset commit in this case, because we need the PHY up before > accessing CMTG registers, and such PHY would be already off because > of our sanitization routine after hardware state readout. > > Since our driver doesn't even model the PHY being active and not > driving a transcoder (to the best of my knowledge), should we keep > this case to be dealt with in the future? > > -- > Gustavo Sousa -- Ville Syrjälä Intel
