Re: [PATCH v2 0/4] TC phy check cleanup

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

On Wed, 20 Dec 2023, Radhakrishna Sripada <radhakrishna.sripada@intel.com> wrote:
> We are relying on end-less if-else ladders for a type-c phy
> capabilities check. Though it made sense when platforms supported
> legacy type-c support, modern platforms rely on the information
> passed by vbt. This cleanup restricts the if-else ladder to the
> platforms supporting legacy type-c phys and relies on vbt info
> for modern client and discrete platforms.

This series is moving the vbt handling in the wrong direction.

We want to *avoid* new lookups. The idea is that you do the lookup
*once* when initializing the encoder, and after that you use
encoder->devdata.

If you look at the intel_phy_is_tc() call sites, you'll observe that
almost all of the places have the encoder (or devdata) already
available. They get the port from encoder->port, and the phy from
intel_port_to_phy().

So this throws away the information that's already available, and then
goes to look it up again in the worst possible way.

You should convert intel_phy_is_tc() to something like
intel_encoder_is_tc(), and pass encoder to it instead of phy. Similarly,
intel_port_to_tc() should be converted to intel_encoder_to_tc().

There are a couple of special cases that only have devdata or phy. But
we can handle the special cases after making the common case
straightforward.


BR,
Jani.


>
> v2: Move cleanup vbt later to handle safe encoder removal
>
> Radhakrishna Sripada (4):
>   drm/i915: Move intel_bios_driver_remove later
>   drm/i915: Rename intel_bios_encoder_data_lookup as a port variant
>   drm/i915: Introduce intel_encoder_phy_data_lookup
>   drm/i915: Separate tc check for legacy and non legacy tc phys
>
>  drivers/gpu/drm/i915/display/g4x_dp.c         |  2 +-
>  drivers/gpu/drm/i915/display/g4x_hdmi.c       |  2 +-
>  drivers/gpu/drm/i915/display/intel_bios.c     | 15 +++++++++-
>  drivers/gpu/drm/i915/display/intel_bios.h     |  5 +++-
>  drivers/gpu/drm/i915/display/intel_ddi.c      |  2 +-
>  drivers/gpu/drm/i915/display/intel_display.c  | 29 ++++++++++++-------
>  .../drm/i915/display/intel_display_device.h   |  1 +
>  .../drm/i915/display/intel_display_driver.c   |  4 +--
>  drivers/gpu/drm/i915/display/intel_dp.c       |  2 +-
>  9 files changed, 44 insertions(+), 18 deletions(-)

-- 
Jani Nikula, Intel