[PATCH v2 0/4] TC phy check cleanup

[Sripada, Radhakrishna]

Hi Jani,

> -----Original Message-----
> From: Jani Nikula <jani.nikula at linux.intel.com>
> Sent: Thursday, December 21, 2023 2:04 AM
> To: Sripada, Radhakrishna <radhakrishna.sripada at intel.com>; intel-
> gfx at lists.freedesktop.org
> Subject: Re: [PATCH v2 0/4] TC phy check cleanup
> 
> On Wed, 20 Dec 2023, Radhakrishna Sripada <radhakrishna.sripada at 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.
Common case making a tc check out of encoder sure makes lot of sense
And agreed to you point. The question that arises in the special cases.
For ex. during vbt_defaults init in intel_bios.c, I can only handle for legacy tc ports.

In other cases where only phy info is available, I may have to iterate over all the
drm_encoders to obtain port info and compare it with phy info adding lot of lookup
overhead. Or we may have to use encoder based lookups in all associated lookups like
icl_port_to_ddc_pin etc.

Thoughts?

Radhakrishna(RK) Sripada
> 
> 
> 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