[PATCH] drm/i915/display: Check some INVALID_TRANSCODER cases

[PATCH] drm/i915/display: Check some INVALID_TRANSCODER cases

[Jonathan Cavitt]

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.

Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 178074316a2c..7abeb60779e5 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3875,7 +3875,8 @@ static void intel_ddi_set_idle_link_train(struct intel_dp *intel_dp,
 static bool intel_ddi_is_audio_enabled(struct intel_display *display,
 				       enum transcoder cpu_transcoder)
 {
-	if (cpu_transcoder == TRANSCODER_EDP)
+	if (cpu_transcoder == TRANSCODER_EDP ||
+	    cpu_transcoder == INVALID_TRANSCODER)
 		return false;
 
 	if (!intel_display_power_is_enabled(display, POWER_DOMAIN_AUDIO_MMIO))
@@ -4637,8 +4638,13 @@ static int intel_ddi_compute_config_late(struct intel_encoder *encoder,
 
 	if (crtc_state->master_transcoder == crtc_state->cpu_transcoder) {
 		crtc_state->master_transcoder = INVALID_TRANSCODER;
-		crtc_state->sync_mode_slaves_mask =
-			port_sync_transcoders & ~BIT(crtc_state->cpu_transcoder);
+		if (crtc_state->cpu_transcoder == INVALID_TRANSCODER ||
+		    !port_sync_transcoders)
+			crtc_state->sync_mode_slaves_mask = 0;
+		else
+
+			crtc_state->sync_mode_slaves_mask =
+				port_sync_transcoders & ~BIT(crtc_state->cpu_transcoder);
 	}
 
 	return 0;
-- 
2.43.0

Re: [PATCH] drm/i915/display: Check some INVALID_TRANSCODER cases

[Ville Syrjälä]

On Thu, Apr 09, 2026 at 09:00:02PM +0000, 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.
> 
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 178074316a2c..7abeb60779e5 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3875,7 +3875,8 @@ static void intel_ddi_set_idle_link_train(struct intel_dp *intel_dp,
>  static bool intel_ddi_is_audio_enabled(struct intel_display *display,
>  				       enum transcoder cpu_transcoder)
>  {
> -	if (cpu_transcoder == TRANSCODER_EDP)
> +	if (cpu_transcoder == TRANSCODER_EDP ||
> +	    cpu_transcoder == INVALID_TRANSCODER)
>  		return false;
>  
>  	if (!intel_display_power_is_enabled(display, POWER_DOMAIN_AUDIO_MMIO))
> @@ -4637,8 +4638,13 @@ static int intel_ddi_compute_config_late(struct intel_encoder *encoder,
>  
>  	if (crtc_state->master_transcoder == crtc_state->cpu_transcoder) {
>  		crtc_state->master_transcoder = INVALID_TRANSCODER;
> -		crtc_state->sync_mode_slaves_mask =
> -			port_sync_transcoders & ~BIT(crtc_state->cpu_transcoder);
> +		if (crtc_state->cpu_transcoder == INVALID_TRANSCODER ||
> +		    !port_sync_transcoders)
> +			crtc_state->sync_mode_slaves_mask = 0;
> +		else
> +
> +			crtc_state->sync_mode_slaves_mask =
> +				port_sync_transcoders & ~BIT(crtc_state->cpu_transcoder);

Making the code convoluted just to satisfy some dumb static
analysis tool is not great.

If we really need to care about this crap then I think it'd be better
to introduce some kind of intel_transcoder_mask() function that
explicitly deals with INVALID_TRANSCODER.

>  	}
>  
>  	return 0;
> -- 
> 2.43.0

-- 
Ville Syrjälä
Intel

RE: [PATCH] drm/i915/display: Check some INVALID_TRANSCODER cases

[Cavitt, Jonathan]

-----Original Message-----
From: Ville Syrjälä <ville.syrjala@linux.intel.com> 
Sent: Thursday, April 9, 2026 2:12 PM
To: Cavitt, Jonathan <jonathan.cavitt@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Gupta, Saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>
Subject: Re: [PATCH] drm/i915/display: Check some INVALID_TRANSCODER cases
> 
> On Thu, Apr 09, 2026 at 09:00:02PM +0000, 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.
> > 
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 178074316a2c..7abeb60779e5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3875,7 +3875,8 @@ static void intel_ddi_set_idle_link_train(struct intel_dp *intel_dp,
> >  static bool intel_ddi_is_audio_enabled(struct intel_display *display,
> >  				       enum transcoder cpu_transcoder)
> >  {
> > -	if (cpu_transcoder == TRANSCODER_EDP)
> > +	if (cpu_transcoder == TRANSCODER_EDP ||
> > +	    cpu_transcoder == INVALID_TRANSCODER)
> >  		return false;
> >  
> >  	if (!intel_display_power_is_enabled(display, POWER_DOMAIN_AUDIO_MMIO))
> > @@ -4637,8 +4638,13 @@ static int intel_ddi_compute_config_late(struct intel_encoder *encoder,
> >  
> >  	if (crtc_state->master_transcoder == crtc_state->cpu_transcoder) {
> >  		crtc_state->master_transcoder = INVALID_TRANSCODER;
> > -		crtc_state->sync_mode_slaves_mask =
> > -			port_sync_transcoders & ~BIT(crtc_state->cpu_transcoder);
> > +		if (crtc_state->cpu_transcoder == INVALID_TRANSCODER ||
> > +		    !port_sync_transcoders)
> > +			crtc_state->sync_mode_slaves_mask = 0;
> > +		else
> > +
> > +			crtc_state->sync_mode_slaves_mask =
> > +				port_sync_transcoders & ~BIT(crtc_state->cpu_transcoder);
> 
> Making the code convoluted just to satisfy some dumb static
> analysis tool is not great.
> 
> If we really need to care about this crap then I think it'd be better
> to introduce some kind of intel_transcoder_mask() function that
> explicitly deals with INVALID_TRANSCODER.

If the issue is with this specific segment, then would this be better?
"""
        if (crtc_state->master_transcoder == crtc_state->cpu_transcoder) {
                if (crtc_state->master_transcoder == INVALID_TRANSCODER) {
                        crtc_state->sync_mode_slaves_mask = 0;
                        return 0;
                }
                crtc_state->master_transcoder = INVALID_TRANSCODER;
                crtc_state->sync_mode_slaves_mask =
                        port_sync_transcoders & ~BIT(crtc_state->cpu_transcoder);
        }
"""

Otherwise, I could just mark the issue as a false positive on my end?
-Jonathan Cavitt

> 
> >  	}
> >  
> >  	return 0;
> > -- 
> > 2.43.0
> 
> -- 
> Ville Syrjälä
> Intel
> 

✗ i915.CI.BAT: failure for drm/i915/display: Check some INVALID_TRANSCODER cases

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 3528 bytes --]

== Series Details ==

Series: drm/i915/display: Check some INVALID_TRANSCODER cases
URL   : https://patchwork.freedesktop.org/series/164655/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_18306 -> Patchwork_164655v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_164655v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_164655v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164655v1/index.html

Participating hosts (42 -> 40)
------------------------------

  Missing    (2): bat-dg2-13 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_164655v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@uncore:
    - bat-arls-6:         [PASS][1] -> [INCOMPLETE][2] +1 other test incomplete
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18306/bat-arls-6/igt@i915_selftest@live@uncore.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164655v1/bat-arls-6/igt@i915_selftest@live@uncore.html

  
Known issues
------------

  Here are the changes found in Patchwork_164655v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@core_debugfs@read-all-entries:
    - bat-adlp-9:         [PASS][3] -> [DMESG-WARN][4] ([i915#15673])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18306/bat-adlp-9/igt@core_debugfs@read-all-entries.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164655v1/bat-adlp-9/igt@core_debugfs@read-all-entries.html

  * igt@i915_selftest@live@workarounds:
    - bat-arls-5:         [PASS][5] -> [DMESG-FAIL][6] ([i915#12061]) +1 other test dmesg-fail
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18306/bat-arls-5/igt@i915_selftest@live@workarounds.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164655v1/bat-arls-5/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@core_auth@basic-auth:
    - bat-adlp-9:         [DMESG-WARN][7] ([i915#15673]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18306/bat-adlp-9/igt@core_auth@basic-auth.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164655v1/bat-adlp-9/igt@core_auth@basic-auth.html

  * igt@i915_selftest@live:
    - bat-mtlp-8:         [DMESG-FAIL][9] ([i915#12061]) -> [PASS][10] +1 other test pass
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18306/bat-mtlp-8/igt@i915_selftest@live.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164655v1/bat-mtlp-8/igt@i915_selftest@live.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#15673]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15673


Build changes
-------------

  * Linux: CI_DRM_18306 -> Patchwork_164655v1

  CI-20190529: 20190529
  CI_DRM_18306: 97d8833ffba6bd3d6aaa51169069620ac17a2e37 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8852: 8852
  Patchwork_164655v1: 97d8833ffba6bd3d6aaa51169069620ac17a2e37 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164655v1/index.html

[-- Attachment #2: Type: text/html, Size: 4407 bytes --]