[PATCH] Handle failures in setting a CRTC to a DRM mode properly

[Michel Dänzer]

On 22.10.2015 03:52, cpaul at redhat.com wrote:
> From: Stephen Chandler Paul <cpaul at redhat.com>
> 
> This fixes a bug where running the card out of PPLL's when hotplugging
> another monitor would result in all of the displays going blank and
> failing to work properly until X was restarted or the user switched to
> another VT.
> 
> Signed-off-by: Stephen Chandler Paul <cpaul at redhat.com>
> ---
>  src/drmmode_display.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index 64e79d4..5d69fbf 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -762,10 +762,12 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
>  		}
>  		ret = drmModeSetCrtc(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
>  				     fb_id, x, y, output_ids, output_count, &kmode);
> -		if (ret)
> +		if (ret) {
>  			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
>  				   "failed to set mode: %s", strerror(-ret));
> -		else
> +			ret = FALSE;
> +			goto done;
> +		} else
>  			ret = TRUE;
>  
>  		if (crtc->scrn->pScreen)
> 

Thanks for the good catch, again.

I think assigning the return value of drmModeSetCrtc to ret (which is
of type Bool) is just a bit confusing. How about this:

		if (drmModeSetCrtc(drmmode->fd,
				   drmmode_crtc->mode_crtc->crtc_id,
				   fb_id, x, y, output_ids,
				   output_count, &kmode) != 0) {
			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
				   "failed to set mode: %s",
				   strerror(errno));
			ret = FALSE;
			goto done;
		} else
			ret = TRUE;


That would get my

Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer