Re: [Intel-gfx] [PATCH v2 4/6] drm/i915/display: add phy, vbt and ddi indexes

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Jul 01, 2020 at 10:24:07AM -0700, Lucas De Marchi wrote:
> On Wed, Jul 01, 2020 at 08:04:30PM +0300, Ville Syrjälä wrote:
> >On Wed, Jun 24, 2020 at 05:11:18PM -0700, Lucas De Marchi wrote:
> >> Identify 3 possible cases in which the index numbers can be different
> >> from the "port" and add them to the description-based ddi initialization
> >> table.  This can be used in place of additional functions mapping from
> >> one to the other.  Right now we already cover part of this by creating kind of
> >> virtual phy numbering, but that comes with downsides:
> >>
> >> a) there's not really a "phy numbering" in the spec, this is purely a
> >> software thing; hardware uses whatever they want thinking mapping from
> >> one to the other arbitrarily is easy in software.
> >>
> >> b) currently the mapping occurs on "leaf" functions, making the decision
> >> based on the platform for each of those functions
> >>
> >> With this new table the approach will be: the port, as defined by the
> >> enum port, is merely a driver convention and won't be used anymore to
> >> define the register offset or register bits. For that we have the other
> >> 3 indexes, identified as being possibly different from the current usage
> >> of register bits: ddi, vbt and phy. The phy type is also added here,
> >> meant to replace the checks for combo vs tc.
> >>
> >> v2: Rebase and add RKL
> >>
> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_display.c  | 64 ++++++++++---------
> >>  drivers/gpu/drm/i915/display/intel_display.h  |  8 +++
> >>  .../drm/i915/display/intel_display_types.h    |  4 ++
> >>  3 files changed, 45 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >> index c234b50212b0..d591063502c5 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> @@ -16806,57 +16806,59 @@ static void intel_pps_init(struct drm_i915_private *dev_priv)
> >>  }
> >>
> >>  static const struct intel_ddi_port_info rkl_ports[] = {
> >> -	{ .name = "DDI A",   .port = PORT_A },
> >> -	{ .name = "DDI B",   .port = PORT_B },
> >> -	{ .name = "DDI TC1", .port = PORT_D },
> >> -	{ .name = "DDI TC2", .port = PORT_E },
> >> +	{ .name = "DDI A", .port = PORT_A, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x0, .phy_idx = 0x0, .vbt_idx = 0x0, },
> >
> >I'm thinking we won't need ddi_idx and instead 'port' should suffice.
> >We can add the aliases with the TC names for tgl+ to unconfuse the
> >current mess. In fact I already tried that in a local branch (while
> >doing the hpd_pin cleanup) and it looks mostly fine to me. There are
> >a few annoying parts, like power domains, where we still end up with
> >port G-I names that don't exist anywhere in bspec (excetp in VBT).
> 
> I think we should stop trying that because it leads to the current mess
> we put ourselves into.  Hence my idea of "port should be just a software
> thing, don't try to make it match the hardware".  HW indexes (for register
> address, bitfields and whatnot) are provided by the correspondent _idx.
> Which index you use depends on what part of the hw you are talking to.
> 
> See the TODO below of one case this would be true. Once the conversions
> are finished we change them and then for every ddi+ platform, port is
> just a number we can use to identify the entry in the table.

Seems contrary to pretty much everything else in the driver so
not great IMO.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx