Re: [PATCH 27/32] drm/i915/vbt: Add fields dedicated_external and dyn_port_over_tc

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 17 Oct 2025, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> Quoting Jani Nikula (2025-10-15 12:24:12-03:00)
>>On Wed, 15 Oct 2025, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>>> VBT version 264 adds new fields associated to Xe3p_LPD's new ways of
>>> configuring SoC for TC ports and PHYs.  Update the code to match the
>>> updates in VBT.
>>>
>>> The new field dedicated_external is used to represent TC ports that are
>>> connected to PHYs outside of the Type-C subsystem, meaning that they
>>> behave like dedicated ports and don't require the extra Type-C
>>> programming.  In an upcoming change, we will update the driver to take
>>> this field into consideration when detecting the type of port.
>>>
>>> The new field dyn_port_over_tc is used to inform that the TC port can be
>>> dynamically allocated for a legacy connector in the Type-C subsystem,
>>> which is a new feature in Xe3p_LPD.  In upcoming changes, we will use
>>> that field in order to handle the IOM resource management programming
>>> required for that.
>>>
>>> Note that, when dedicated_external is set, the fields dp_usb_type_c and
>>> tbt are tagged as "don't care" in the spec, so they should be ignored in
>>> that case, so also make sure to update the accessor functions to take
>>> that into consideration.
>>>
>>> Bspec: 20124, 68954, 74304
>>> Cc: Shekhar Chauhan <shekhar.chauhan@intel.com>
>>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_bios.c     | 20 +++++++++++++++++++-
>>>  drivers/gpu/drm/i915/display/intel_bios.h     |  2 ++
>>>  drivers/gpu/drm/i915/display/intel_vbt_defs.h |  7 ++++++-
>>>  3 files changed, 27 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>>> index 3596dce84c28..e466728ced0f 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>>> @@ -2777,7 +2777,7 @@ static int child_device_expected_size(u16 version)
>>>  {
>>>          BUILD_BUG_ON(sizeof(struct child_device_config) < 40);
>>>  
>>> -        if (version > 263)
>>> +        if (version > 264)
>>>                  return -ENOENT;
>>>          else if (version >= 263)
>>>                  return 44;
>>> @@ -3714,14 +3714,32 @@ int intel_bios_hdmi_ddc_pin(const struct intel_bios_encoder_data *devdata)
>>>  
>>>  bool intel_bios_encoder_supports_typec_usb(const struct intel_bios_encoder_data *devdata)
>>>  {
>>> +        if (intel_bios_encoder_is_dedicated_external(devdata))
>>> +                return false;
>>> +
>>
>>We already have mechanisms for this. Please don't pollute the functions.
>>
>>dp_usb_type_c should just be set to 0 in a new sanitize_something()
>>function at the end of parse_ddi_port()
>>
>>>          return devdata->display->vbt.version >= 195 && devdata->child.dp_usb_type_c;
>>>  }
>>>  
>>>  bool intel_bios_encoder_supports_tbt(const struct intel_bios_encoder_data *devdata)
>>>  {
>>> +        if (intel_bios_encoder_is_dedicated_external(devdata))
>>> +                return false;
>>> +
>>
>>Ditto.
>>
>>tbt should just be set to 0 in a new sanitize_something() function at
>>the end of parse_ddi_port()
>
> Aren't sanitize_*() functions, at least in the context of intel_bios.c,
> for actually fixing bogus information reported by the VBT?

Yes.

> Arguably, that wouldn't be the case for dedicated_external and the
> related fields, since it is actually about a new way to interpret bits
> for the new version of the VBT.

Well, if the spec says you shouln't have some bits set in combination
with something else, then having those set is bogus, no?

> One of my concerns with the sanitize approach would be gotchas with
> anything that tries to use the fields before they are sanitized (e.g.
> another sanitization function gets added in the future that would use
> one of the sanitized fields).  Perhaps that's never gonna happen?

The sanitization part should be careful about that, obviously.

BR,
Jani.



-- 
Jani Nikula, Intel