On Thu, 30 Jan 2020, Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx> wrote:
> On some platforms such as Elkhart Lake, although we may use DDI D
> to drive a connector, we have to use PHY A (Combo Phy PORT A) to
> detect the hotplug interrupts as per the spec because there is no
> one-to-one mapping between DDIs and PHYs. Therefore, use the
> function intel_port_to_phy() which contains the logic for such
> mapping(s) to find the correct hpd_pin.
>
> This change should not affect other platforms as there is always
> a one-to-one mapping between DDIs and PHYs.
>
> v2:
> - Convert the case statements to use PHYs instead of PORTs (Jani)
>
> v3:
> - Refactor the function to reduce the number of return statements by
> lumping all the case statements together except PHY_F which needs
> special handling (Jose)
>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_hotplug.c | 37 ++++++++------------
> 1 file changed, 15 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 042d98bae1ea..27e3033278a0 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -89,29 +89,22 @@
> enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
> enum port port)
> {
> - switch (port) {
> - case PORT_A:
> - return HPD_PORT_A;
> - case PORT_B:
> - return HPD_PORT_B;
> - case PORT_C:
> - return HPD_PORT_C;
> - case PORT_D:
> - return HPD_PORT_D;
> - case PORT_E:
> - return HPD_PORT_E;
> - case PORT_F:
> - if (IS_CNL_WITH_PORT_F(dev_priv))
> - return HPD_PORT_E;
> - return HPD_PORT_F;
> - case PORT_G:
> - return HPD_PORT_G;
> - case PORT_H:
> - return HPD_PORT_H;
> - case PORT_I:
> - return HPD_PORT_I;
> + enum phy phy = intel_port_to_phy(dev_priv, port);
> +
> + switch (phy) {
> + case PHY_F:
> + return IS_CNL_WITH_PORT_F(dev_priv) ? HPD_PORT_E : HPD_PORT_F;
> + case PHY_A:
> + case PHY_B:
> + case PHY_C:
> + case PHY_D:
> + case PHY_E:
> + case PHY_G:
> + case PHY_H:
> + case PHY_I:
> + return HPD_PORT_A + phy;
I know José asked you to do this, but now you've tied two enum sequences
together without explaining it anywhere. Before this, AFAICT, enum
hpd_pin was just an abstract enumeration where the actual values of the
enums didn't mean a thing, apart from 0 for HPD_NONE.
Maybe this is what we want to do, but we should never be so casual about
it.
BR,
Jani.
> default:
> - MISSING_CASE(port);
> + MISSING_CASE(phy);
> return HPD_NONE;
> }
> }
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
