On Wed, 03 Sep 2025, Ankit Nautiyal <ankit.k.nauti...@intel.com> wrote:
> The helper intel_encoder_to_tc() can potentially return TC_PORT_NONE
> (-1) and cause overflow while computing ddc pins in
> icl_encoder_to_ddc_pin().
>
> Check for TC_PORT_NONE before deriving the ddc pins for TC port.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nauti...@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_hdmi.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index cbee628eb26b..85f70cedc40c 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2791,10 +2791,16 @@ static u8 icl_encoder_to_ddc_pin(struct intel_encoder
> *encoder)
> struct intel_display *display = to_intel_display(encoder);
> enum port port = encoder->port;
>
> - if (intel_encoder_is_combo(encoder))
> + if (intel_encoder_is_combo(encoder)) {
> return GMBUS_PIN_1_BXT + port;
> - else if (intel_encoder_is_tc(encoder))
> - return GMBUS_PIN_9_TC1_ICP + intel_encoder_to_tc(encoder);
> + } else if (intel_encoder_is_tc(encoder)) {
> + enum tc_port tc_port = intel_encoder_to_tc(encoder);
intel_encoder_to_tc() can only return TC_PORT_NONE if intel_phy_is_tc()
== false. But intel_encoder_is_tc() just above means intel_phy_is_tc()
== true.
This case can't happen, it's a static analyzer being overzealous.
Adding checks like this to please a static analyzer leads to misery,
because it adds unnecessary code to maintain, and it will never be run.
I think it would be more interesting to make intel_port_to_tc() WARN on
!intel_phy_is_tc(), return a bogus >= 0 port, and ensure all callers
only call it on TC ports.
This obviously leads to issues if it happens, but hey, it shouldn't
happen, and intel_encoder_to_tc() returning TC_PORT_NONE is *already*
such a case. Just move it to lower levels.
If we start checking for every impossible situation, and propagating
errors for them, our codebase will be 90% error handling.
BR,
Jani.
> +
> + if (tc_port != TC_PORT_NONE)
> + return GMBUS_PIN_9_TC1_ICP + tc_port;
> +
> + drm_WARN(display->drm, 1, "Invalid TC port\n");
> + }
>
> drm_WARN(display->drm, 1, "Unknown port:%c\n", port_name(port));
> return GMBUS_PIN_2_BXT;
--
Jani Nikula, Intel
