RE: [PATCH 1/2] drm/amdgpu/mst: Stop ignoring error codes and deadlocking

[Lin, Wayne]

[Public]



> -----Original Message-----
> From: Lyude Paul <lyude@xxxxxxxxxx>
> Sent: Tuesday, November 15, 2022 5:55 AM
> To: Lin, Wayne <Wayne.Lin@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Wentland, Harry <Harry.Wentland@xxxxxxx>; stable@xxxxxxxxxxxxxxx;
> Li, Sun peng (Leo) <Sunpeng.Li@xxxxxxx>; Siqueira, Rodrigo
> <Rodrigo.Siqueira@xxxxxxx>; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Koenig, Christian
> <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>; David
> Airlie <airlied@xxxxxxxxx>; Daniel Vetter <daniel@xxxxxxxx>; Kazlauskas,
> Nicholas <Nicholas.Kazlauskas@xxxxxxx>; Pillai, Aurabindo
> <Aurabindo.Pillai@xxxxxxx>; Li, Roman <Roman.Li@xxxxxxx>; Zuo, Jerry
> <Jerry.Zuo@xxxxxxx>; Wu, Hersen <hersenxs.wu@xxxxxxx>; Thomas
> Zimmermann <tzimmermann@xxxxxxx>; Mahfooz, Hamza
> <Hamza.Mahfooz@xxxxxxx>; Hung, Alex <Alex.Hung@xxxxxxx>; Francis,
> David <David.Francis@xxxxxxx>; Mikita Lipski <mikita.lipski@xxxxxxx>; Liu,
> Wenjing <Wenjing.Liu@xxxxxxx>; open list:DRM DRIVERS <dri-
> devel@xxxxxxxxxxxxxxxxxxxxx>; open list <linux-kernel@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH 1/2] drm/amdgpu/mst: Stop ignoring error codes and
> deadlocking
> 
> On Wed, 2022-11-09 at 09:48 +0000, Lin, Wayne wrote:
> > >   	}
> > > -	if (!drm_dp_mst_atomic_check(state) && !debugfs_overwrite) {
> > > +	ret = drm_dp_mst_atomic_check(state);
> > > +	if (ret == 0 && !debugfs_overwrite) {
> > >   		set_dsc_configs_from_fairness_vars(params, vars, count, k);
> > > -		return true;
> > > +		return 0;
> > > +	} else if (ret == -EDEADLK) {
> > > +		return ret;
> >
> > I think we should return here whenever there is an error. Not just for
> > EDEADLK case.
> 
> Are we sure about this one? I think we may actually want to make this so it
> returns on ret != -ENOSPC, since we want the function to continue if there's
> no space in the atomic state available so it can try recomputing things with
> compression enabled. On ret == 0 it should return early without doing
> compression, and on ret == -ENOSPC it should just continue the function
> from there
> 
Oh, right.. Thanks for saving me from causing disaster : )

> --
> Cheers,
>  Lyude Paul (she/her)
>  Software Engineer at Red Hat
--
Regards,
Wayne