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?
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?
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.
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
