Re: [PATCH i-g-t v2 05/23] tests/amdgpu/amd_abm: Don't use uninitialized 'pipe'

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

On Tue, Feb 24, 2026 at 04:08:16PM +0200, Jani Nikula wrote:
> On Sat, 21 Feb 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The for_each_valid_output_on_pipe() loops in amd_abm use an
> > uninitialized 'pipe' variable. Not entirely sure what the intention
> > was with the change to for_each_valid_output_on_pipe(), but eg.
> > set_abm_level() gets called from those loops and it operates on
> > data->pipe_id instead of whatever stack garbage pipe the loop
> > happens to use. So let's assume the intention was to use the same
> > fixed pipe for everything and just pass data->pipe_id to
> > for_each_valid_output_on_pipe().
> 
> Would be great to get input from the authors, but in any case
> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>

Ta. I pushed the series as is, mainly to avoid conflicts creeping
in, and I want to finish the treewide refactoring (hopefully) this
week. We can revisit this test later if the change turns out to be
wrong.

> 
> >
> >  #include "scripts/iterators.cocci"
> >
> > @@
> > iterator name for_each_valid_output_on_pipe;
> > expression DISPLAY, OUTPUT;
> > identifier PIPE;
> > type T;
> > @@
> > - T PIPE;
> > <+... when != PIPE = ...
> >   for_each_valid_output_on_pipe(DISPLAY,
> > -	PIPE,
> > +	data->pipe_id,
> > 	OUTPUT)
> > 	{...}
> > ...+>
> >
> > Cc: George Zhang <george.zhang@amd.com>
> > Cc: Tom Chung <chiahsuan.chung@amd.com>
> > Fixes: 1235576931b5 ("tests/amdgpu/amd_abm: Fixed tests to only run on one output per pipe")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  tests/amdgpu/amd_abm.c | 18 ++++++------------
> >  1 file changed, 6 insertions(+), 12 deletions(-)
> >
> > diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
> > index 110dd89e3ac2..159f6c7c69e9 100644
> > --- a/tests/amdgpu/amd_abm.c
> > +++ b/tests/amdgpu/amd_abm.c
> > @@ -170,10 +170,9 @@ static void test_fini(data_t *data)
> >  {
> >  	igt_display_t *display = &data->display;
> >  	igt_output_t *output;
> > -	enum pipe pipe;
> >  
> >  	/* Disable ABM before exit test */
> > -	for_each_valid_output_on_pipe(&data->display, pipe, output) {
> > +	for_each_valid_output_on_pipe(&data->display, data->pipe_id, output) {
> >  		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> >  			continue;
> >  		set_abm_level(data, output, 0);
> > @@ -328,9 +327,8 @@ static void backlight_dpms_cycle(data_t *data)
> >  	int max_brightness;
> >  	int pwm_1, pwm_2;
> >  	igt_output_t *output;
> > -	enum pipe pipe;
> >  
> > -	for_each_valid_output_on_pipe(&data->display, pipe, output) {
> > +	for_each_valid_output_on_pipe(&data->display, data->pipe_id, output) {
> >  		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> >  			continue;
> >  
> > @@ -359,10 +357,9 @@ static void backlight_monotonic_basic(data_t *data)
> >  	int prev_pwm, pwm;
> >  	int brightness_step;
> >  	int brightness;
> > -	enum pipe pipe;
> >  	igt_output_t *output;
> >  
> > -	for_each_valid_output_on_pipe(&data->display, pipe, output) {
> > +	for_each_valid_output_on_pipe(&data->display, data->pipe_id, output) {
> >  		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> >  			continue;
> >  		ret = backlight_read_max_brightness(&max_brightness);
> > @@ -393,10 +390,9 @@ static void backlight_monotonic_abm(data_t *data)
> >  	int prev_pwm, pwm;
> >  	int brightness_step;
> >  	int brightness;
> > -	enum pipe pipe;
> >  	igt_output_t *output;
> >  
> > -	for_each_valid_output_on_pipe(&data->display, pipe, output) {
> > +	for_each_valid_output_on_pipe(&data->display, data->pipe_id, output) {
> >  		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> >  			continue;
> >  		ret = backlight_read_max_brightness(&max_brightness);
> > @@ -426,10 +422,9 @@ static void abm_enabled(data_t *data)
> >  	int ret, i;
> >  	int max_brightness;
> >  	int pwm, prev_pwm, pwm_without_abm;
> > -	enum pipe pipe;
> >  	igt_output_t *output;
> >  
> > -	for_each_valid_output_on_pipe(&data->display, pipe, output) {
> > +	for_each_valid_output_on_pipe(&data->display, data->pipe_id, output) {
> >  		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> >  			continue;
> >  
> > @@ -460,10 +455,9 @@ static void abm_gradual(data_t *data)
> >  	int convergence_delay = 10;
> >  	int prev_pwm, pwm, curr;
> >  	int max_brightness;
> > -	enum pipe pipe;
> >  	igt_output_t *output;
> >  
> > -	for_each_valid_output_on_pipe(&data->display, pipe, output) {
> > +	for_each_valid_output_on_pipe(&data->display, data->pipe_id, output) {
> >  		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> >  			continue;
> 
> -- 
> Jani Nikula, Intel

-- 
Ville Syrjälä
Intel