Re: [PATCH v4 07/11] drm/i915/xe3p_lpd: Extend Type-C flow for static DDI allocation

[Imre Deak <imre.deak@intel.com>]

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