[Intel-gfx] [PATCH 1/2] drm/i915/fbdev: print error in case drm_fb_helper_initial_config fails
Andrzej Hajda
andrzej.hajda at intel.com
Thu May 5 18:57:25 UTC 2022
Hi Jani,
On 05.05.2022 20:37, Jani Nikula wrote:
> 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.
Apparently this patch didn't help in catching the bug, so no big
feelings about it.
Anyway I think I have found the issue I was looking for: hpd poll worker
could be run during driver removal after console unregistration causing
re-registration of console, which is not removed later leaving dangling
pointers.
If you could take a look at the patch [1], it would be nice :)
[1]: https://patchwork.freedesktop.org/series/103621/#rev1
Regards
Andrzej
>
>> + intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
>> }
>>
>> void intel_fbdev_initial_config_async(struct drm_device *dev)
More information about the Intel-gfx
mailing list