Re: [PATCH v4 2/2] drm/amd/display: add vendor specific reset

[Hamza Mahfooz <someguy@effective-light.com>]

On Mon, Feb 23, 2026 at 10:34:17AM +0100, Christian König wrote:
> > @@ -11085,8 +11087,26 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
> >  	/* Signal HW programming completion */
> >  	drm_atomic_helper_commit_hw_done(state);
> >  
> > -	if (wait_for_vblank)
> > -		drm_atomic_helper_wait_for_flip_done(dev, state);
> > +	if (wait_for_vblank &&
> > +	    drm_atomic_helper_wait_for_flip_done(dev, state)) {
> > +		mutex_lock(&dm->dc_lock);
> > +		if (dm_dmub_hw_init(adev))
> 
> From Michel's explanation that is pretty much a no-go because it potentially causes other atomic commits to react in unforeseen ways.
> 

This code would only run if the forced modeset fails, which is to say we
are already in a hung state, so I don't expect any other atomic commits
to be firing off. Also, evidently the timeout isn't a one off, so it's
almost certainly caused by hung firmware and not by a bug in the
driver's vblanking code.

> > +			drm_dev_wedged_event(dev, DRM_WEDGE_RECOVERY_REBIND |
> > +					     DRM_WEDGE_RECOVERY_BUS_RESET,
> > +					     NULL);
> 
> Please completely drop that. This here is a sledge hammer and not going to fly anywhere.
> 

I don't feel too strongly about it, though isn't the point to inform
userspace that we were unable to recover? AFAIK the prescribed methods
are mere suggestions and users can choose to ignore them if they feel
that they are too hard hitting.

> > +		mutex_unlock(&dm->dc_lock);
> > +
> > +		spin_lock_irqsave(&dev->event_lock, flags);
> > +		drm_for_each_crtc(crtc, dev) {
> 
> This should go over only the CRTCs in the atomic commit currently handled.
> 
> Have you tried sending a hotplug event for the connectors in question as Michel suggested?
> 

Sure, I can look into that. However, we would still need the firmware
reload. Otherwise, we would just be forcing a modeset twice in
succession.