[Intel-gfx] [PATCH 1/2] drm/i915/fbdev: print error in case drm_fb_helper_initial_config fails

[Jani Nikula]

On Wed, 04 May 2022, Andrzej Hajda <andrzej.hajda at intel.com> wrote:
> On some configurations drm_fb_helper_initial_config sometimes fails.
> Logging error value should help debugging such issues.
>
> Signed-off-by: Andrzej Hajda <andrzej.hajda at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbdev.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 221336178991f0..557c7f15ac22a9 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -539,11 +539,16 @@ int intel_fbdev_init(struct drm_device *dev)
>  static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
>  {
>  	struct intel_fbdev *ifbdev = data;
> +	int ret;
>  
>  	/* Due to peculiar init order wrt to hpd handling this is separate. */
> -	if (drm_fb_helper_initial_config(&ifbdev->helper,
> -					 ifbdev->preferred_bpp))
> -		intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
> +	ret = drm_fb_helper_initial_config(&ifbdev->helper,
> +					   ifbdev->preferred_bpp);
> +	if (!ret)
> +		return;

If this patch is intended for merging, I'd prefer keeping the failure
path indented within if (ret).

> +	drm_err(ifbdev->helper.dev, "failed to set initial configuration: %pe\n",
> +		ERR_PTR(ret));

I'm leaning towards preferring "%s", errname(ret) over "%pe",
ERR_PTR(ret) because everyone knows what %s means and using ERR_PTR()
seems odd when it's really a plain int.

BR,
Jani.

> +	intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
>  }
>  
>  void intel_fbdev_initial_config_async(struct drm_device *dev)

-- 
Jani Nikula, Intel Open Source Graphics Center