From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5A80AC5B572 for ; Mon, 17 Aug 2026 15:39:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EADD210E878; Mon, 17 Aug 2026 15:39:19 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="Vo8Fz0l0"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.7]) by gabe.freedesktop.org (Postfix) with ESMTPS id 134FE10E878 for ; Mon, 17 Aug 2026 15:39:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1786981158; x=1818517158; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=p9ERbaRdaJyiFnCkirPk/rDUxuyCkqp2mhU9yRLmLrI=; b=Vo8Fz0l0f67uOWseqBg7oWTJQVxSsoYsrb6rLzxqRbamSMYBBakDKaZb 6OM6JYA2w7U3AR0Fui+0wJlb5osMXkPdSp9uFJ/uP5uASqeGi4qSnWINw loBQQ5McYoHBH4x+ilT8QLf4dgAC8tVFX51aJMXgN/ix981JuF6r9BGqb 2wng2So9COdO1Y++Yd67MhcHYdbhiqI5wyy1t0wlLuoArazfcv7OKiORs I4jwCWi/Bt+bDaiKJ+2wJnI1euLKxDK2mTkPbaUggW+7M+i8V+sAss7zM gwaoictLhZCEiVDQfWfsVFV38Wtx+MhE0BIFfnomXn8DZb+xGZxJgf/uW g==; X-CSE-ConnectionGUID: u5ZA+Vz+RmaUsAzSdbMxMw== X-CSE-MsgGUID: WEGpW242Rxe7jg1+0ojnCg== X-IronPort-AV: E=McAfee;i="6800,10657,11877"; a="113000944" X-IronPort-AV: E=Sophos;i="6.25,229,1779174000"; d="scan'208";a="113000944" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Aug 2026 08:39:18 -0700 X-CSE-ConnectionGUID: qfMv+tE9RNmO9+wedlS4kA== X-CSE-MsgGUID: oG77V1dZQ4m0611vkSsy7A== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.25,229,1779174000"; d="scan'208";a="264981596" Received: from abityuts-desk1.ger.corp.intel.com (HELO localhost) ([10.245.245.110]) by orviesa007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Aug 2026 08:39:15 -0700 From: Jani Nikula To: Jonathan Cavitt , intel-gfx@lists.freedesktop.org Cc: alex.zuo@intel.com, jonathan.cavitt@intel.com, ville.syrjala@linux.intel.com Subject: Re: [PATCH v3] drm/i915/display: Check some INVALID_TRANSCODER cases In-Reply-To: <20260817145838.252365-1-jonathan.cavitt@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs Bertel Jungin Aukio 5, 02600 Espoo, Finland References: <20260817145838.252365-1-jonathan.cavitt@intel.com> Date: Mon, 17 Aug 2026 18:39:13 +0300 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Mon, 17 Aug 2026, Jonathan Cavitt wrote: > There are some cases in intel_ddi.c, such as in intel_ddi_is_audio_enabled > and intel_ddi_compute_config_late, where we attempt to perform a BIT > shift using a passed transcoder enum value. This value may be -1, > INVALID_TRANSCODER, which can result in undefined behavior if this > occurs. > > In the former case, we can simply return false if this is the transcoder > passed (as audio is not enabled on an invalid transcoder). > > In the latter case, the likely expected behavior is to set the > crtc_state->sync_mode_slaves_mask to zero, so just do that directly and > avoid a risky bit shift. > > The likelihood of either case occurring during normal execution is > unknown and possibly very low. Regardless, this covers a static analyis > issue. > > v2: Rewrite the latter case to streamline it (Ville) > > v3: Target cpu_transcoder in intel_ddi_compute_config_late change (Jani) > > Signed-off-by: Jonathan Cavitt > Cc: Ville Syrj=C3=A4l=C3=A4 > Cc: Jani Nikula > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i= 915/display/intel_ddi.c > index 9b3b526e5e55..2bfbb6297b5d 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -3893,7 +3893,8 @@ static void intel_ddi_set_idle_link_train(struct in= tel_dp *intel_dp, > static bool intel_ddi_is_audio_enabled(struct intel_display *display, > enum transcoder cpu_transcoder) > { > - if (cpu_transcoder =3D=3D TRANSCODER_EDP) > + if (cpu_transcoder =3D=3D TRANSCODER_EDP || > + cpu_transcoder =3D=3D INVALID_TRANSCODER) > return false; >=20=20 > if (!intel_display_power_is_enabled(display, POWER_DOMAIN_AUDIO_MMIO)) > @@ -4679,6 +4680,10 @@ static int intel_ddi_compute_config_late(struct in= tel_atomic_state *state, >=20=20 > if (crtc_state->master_transcoder =3D=3D crtc_state->cpu_transcoder) { > crtc_state->master_transcoder =3D INVALID_TRANSCODER; > + if (crtc_state->cpu_transcoder =3D=3D INVALID_TRANSCODER) { > + crtc_state->sync_mode_slaves_mask =3D 0; > + return 0; > + } > crtc_state->sync_mode_slaves_mask =3D port_sync_transcoders & > ~REG_BIT(crtc_state->cpu_transcoder); What I dislike about this is the crtc_state->cpu_transcoder being INVALID_TRANSCODER now looks like a normal state of things, which it isn't. Adding checks for it reinforces the idea that you should check for it everywhere. Which you shouldn't. In the code path being checked, crtc_state->master_transcoder also ends up being INVALID_TRANSCODER by coincidence, through a ffs(0) - 1 computation. We should never end up with INVALID_TRANSCODER as a result of a computation like that. It should always be explicit INVALID_TRANSCODER through other checks, i.e. port_sync_transcoders being 0 in this case. BR, Jani. > } --=20 Jani Nikula, Intel