Re: [Intel-gfx] [PATCH v2] drm/i915/display: Don't use port enum as register offset

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

On Wed, Sep 21, 2022 at 10:52:59PM +0530, Balasubramani Vivekanandan wrote:
> Display DDI ports are enumerated as PORT_A,PORT_B... . The enums are
> also used as an index to access the DDI_BUF_CTL register for the port.
> 
> With the introduction of TypeC ports, new enums PORT_TC1,PORT_TC2.. were
> added starting from enum value 4 to match the index position of the
> DDI_BUF_CTL register of those ports. Because those early platforms had
> only 3 non-TypeC ports PORT_A,PORT_B, PORT_C followed by TypeC ports.
> So the enums PORT_D,PORT_E.. and PORT_TC1,PORT_TC2.. used the same enum
> values.
> 
> Driver also used the condition `if (port > PORT_TC1)` to identify if a
> port is a TypeC port or non-TypeC.

No one should really be doing that, apart from a few exceptions
during initialization. Apart from that I don't think enum port
should really be doing anything else these days than being the
register block offset we pass to the port registers.

Well, the VBT code does screw over that idea kinda. I've been
occasionally pondering some kind of separate namespace for ports
for the VBT code but haven't really it throught it through in
any detail.

> 
> >From XELPD, additional non-TypeC ports were added in the platform
> calling them as PORT D, PORT E and the DDI registers for those ports
> were positioned after TypeC ports.  So the enums PORT_D and PORT_E can't
> be used as their values do not match with register position. It led to
> creating new enums PORT_D_XELPD, PORT_E_XELPD for ports D and E.
> 
> The condition `if (port > PORT_TC1)` was no more valid for XELPD to
> identify a TypeC port. Also it led to many additional special checks for
> ports PORT_D_XELPD/PORT_E_XELPD.
> 
> With new platforms indicating that the DDI register positions of ports
> can vary across platforms it makes no more feasible to maintain the port
> enum values to match the DDI register position.

Do we know that it's going to get even more messy?

Anyways, we have the exact same thing with AUX CH, so trying to
change one but not the other isn't really going to help.

And on top of that we have the horrorshow in intel_port_to_phy()
& co. I think the phy stuff is probably what we should try to sort
out next, since IMO it's the bigger mess.

-- 
Ville Syrjälä
Intel