On Mon, Nov 17, 2025 at 02:23:51PM -0300, Gustavo Sousa wrote: > Quoting Imre Deak (2025-11-17 12:17:48-03:00) > >On Mon, Nov 17, 2025 at 12:02:37PM -0300, Gustavo Sousa wrote: > >> [...] > >> >> > >> + if (iom_dp_resource_lock(tc)) > >> >> > >> + return false; > >> >> > >> + > >> >> > >> + val = intel_de_read(display, IOM_DP_RESOURCE_MNG); > >> >> > >> + > >> >> > >> + consumer = val & IOM_DDI_CONSUMER_MASK(tc_port); > >> >> > >> + consumer >>= IOM_DDI_CONSUMER_SHIFT(tc_port); > >> >> > >> + > >> >> > >> + /* > >> >> > >> + * Bspec instructs to select first available DDI, but our > >> >> > >> driver is not > >> >> > >> + * ready for such dynamic allocation yet. For now, we > >> >> > >> force a "static" > >> >> > >> + * allocation: map the physical port (where HPD happens) > >> >> > >> to the > >> >> > >> + * encoder's DDI (logical TC port, represented by > >> >> > >> tc_port). > >> >> > >> + */ > >> >> > >> + expected_consumer = IOM_DDI_CONSUMER_STATIC_TC(tc_port); > >> >> > >> + expected_consumer >>= IOM_DDI_CONSUMER_SHIFT(tc_port); > >> >> > >> >> One more thing occured to me: why can't this allocate any free DDI? IOW > >> >> does the address of DDI_BUF_CTL (aka DDI_CTL_DE) used for tc_port depend > >> >> on which DDI gets allocated (or is there any other dependency on which > >> >> DDI got allocated)? AFAICS there is no such dependency and the above > >> >> address would be DDI_BUF_CTL(encoder->port) regardless of the allocated > >> >> DDI. In that case any free DDI could be allocated here. > >> > > >> >Ok, checking this again, DDI_BUF_CTL etc. DDI register addresses will > >> >depend on the allocated DDI. So nvm the above, the mapping needs to > >> >stay 1:1 for now until all the DDI reg accesses are converted to index > >> >the registers with the allocated DDI index. > >> > >> As far as I understand this, especially after talking with Windows > >> folks, the allocated DDI will define the port index for the whole > >> programming, including the registers used to program the PHY - and the > >> hardware would take care of routing to the correct PHY. > > > >Correct, that's how I also understood it after "checking this again". > > > >> Thus, it appears we would need to do the allocation at hotplug time, > >> like saying "this PHY will be driven by DDI x". > > > >To clarify, if the mapping is 1:1, as in this patch, the allocation can > >be done statically during driver loading as discussed earlier. This is > >the only way it will work atm, because the DDI allocation cannot fail > >during runtime. > > Two scenarios that come to mind about doing this on probe time: > > 1) The driver could be loaded with nothing yet attached to the legacy > connector. However, I believe the TCSS doesn't require the > connector to be attached for the allocation to work. So, we are > probably fine here. > > 2) If the legacy connector is never used during the driver's lifetime, > we are basically holding a resource that could have been used by a > DP-alt/TBT connection.
Yes, this is also the way how things work at the moment: a legacy connector reserves a DDI whether or not a sink will be connected to it. I think keeping that existing behavior for now is what makes sense. > For the dynamic feature (to be implemented in the future), how to you > see this? > > 1) Should we allocate the DDI at HPD time and fail report the > connector as disconnected on failure? > 2) Should we allocate the DDI as part of the atomic check phase and > fail the modeset if we can't do it? > 3) Should we allocate the DDI as part of the atomic tail (hardware > commit) and raise errors if the allocation fails? I think the only way for enabling the dynamic allocation of DDIs is to do that whenever the PHY is used, i.e. when the PHY is connected via intel_tc_port_lock() or intel_tc_port_get_link(). That's because even AUX transfers for a detection need an allocated DDI. This is what the patch is currently doing, however, that can be done only once the remapping of a connector/encoder -> allocated DDI is in place and the driver can also handle a modeset on a legacy connector for which the DDI allocation fails. > Another question: once we implement the dynamic feature, this "allocate > on probe time" thing will have to go away, right? The allocation would simply have to moved back to happen during connecting the PHY. > In that case, we would basically suffer the same runtime risks that we > would be trying to avoid, no? No, then the driver will be able to handle a modeset for a legacy connector without a DDI allocated to it. > Perhaps I'm missing something, but, if not: I wonder if doing the > "allocate on probe time" thing is really worth implementing, since it > will be a "temporary" thing that would be reverted later. In that case, > we could try implementing the static allocation in the same place where > dynamic allocation would happen. I still think for now it makes sense to implement the allocate on probe time thing, which is simple enough and provides what we have now (legacy connectors keep a DDI reserved). Adding support for dynamic DDI allocations will allow enabling 3 TBT DP tunnels instead of the current 2 DP tunnels for instance, but this requires a lot of changes affecting current platforms as well. I think moving back the allocation to happen during connecting the PHY - when the changes for that are in place - would be also simple. > >> One of the reasons I think we can't allocate a free DDI at the moment is > >> that the driver is expecting a 1:1 static mapping for HPD interations. > >> We will neeed to make the driver aware of the mapping in order to use > >> the correct encoder when handling HPD events. > > > >Again clarifying, that the above is true only for legacy connectors. IOW > >for a TBT/DP-alt connector, where IOM does the DDI allocation, the HPD > >IRQ delivered to the driver will be already according to the allocated > >DDI. That is those connectors are _different_ wrt. to the mapping > >requirement than the dynamic legacy connectors, for those TBT/DP-alt > >connectors the DDI registers will be accessed based on the > >tc_port/encoder->port to which the HPD IRQ is delivered. > > That's my understanding as well. > > -- > Gustavo Sousa
