[PATCH v5 7/7] drm/i915: Implement fbdev emulation as in-kernel client
Thomas Zimmermann
tzimmermann at suse.de
Wed Nov 1 09:35:32 UTC 2023
Hi
Am 01.11.23 um 10:25 schrieb Ville Syrjälä:
> On Wed, Nov 01, 2023 at 09:33:41AM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 25.10.23 um 13:36 schrieb Hogander, Jouni:
>> [...]
>>>> +
>>>> + if (!drm_drv_uses_atomic_modeset(dev))
>>>> + drm_helper_disable_unused_functions(dev);
>>>
>>> Can you please explain why this is needed here?
>>
>> This disables some parts of the mode-setting pipeline and is required
>> for drivers with non-atomic commits. AFAICT atomic mode setting is not
>> supported on some very old Intel chips. [1] I'll leave a short comment
>> on the code.
>
> We don't expose the atomic uapi because the watermark code is
> kinda sketchy for the old chips, but internally i915 is 100% atomic.
Great, thanks. I'll drop that code then.
Best regards
Thomas
>
>>
>> [1]
>> https://elixir.bootlin.com/linux/v6.6/source/drivers/gpu/drm/i915/intel_device_info.c#L399
>>
>> Best regard
>> Thomas
>>
>>>
>>>> +
>>>> + ret = drm_fb_helper_initial_config(fb_helper);
>>>> + if (ret)
>>>> + goto err_drm_fb_helper_fini;
>>>> +
>>>> + vga_switcheroo_client_fb_set(pdev, fb_helper->info);
>>>>
>>>> return 0;
>>>> +
>>>> +err_drm_fb_helper_fini:
>>>> + drm_fb_helper_fini(fb_helper);
>>>> +err_drm_err:
>>>> + drm_err(dev, "Failed to setup i915 fbdev emulation
>>>> (ret=%d)\n", ret);
>>>> + return ret;
>>>> }
>>>>
>>>> static const struct drm_client_funcs intel_fbdev_client_funcs = {
>>>> @@ -703,22 +729,23 @@ static const struct drm_client_funcs
>>>> intel_fbdev_client_funcs = {
>>>> .hotplug = intel_fbdev_client_hotplug,
>>>> };
>>>>
>>>> -int intel_fbdev_init(struct drm_device *dev)
>>>> +void intel_fbdev_setup(struct drm_i915_private *dev_priv)
>>>
>>> Use i915 rather than dev_priv.
>>>
>>> BR,
>>>
>>> Jouni Högander
>>>
>>>> {
>>>> - struct drm_i915_private *dev_priv = to_i915(dev);
>>>> + struct drm_device *dev = &dev_priv->drm;
>>>> struct intel_fbdev *ifbdev;
>>>> int ret;
>>>>
>>>> - if (drm_WARN_ON(dev, !HAS_DISPLAY(dev_priv)))
>>>> - return -ENODEV;
>>>> + if (!HAS_DISPLAY(dev_priv))
>>>> + return;
>>>>
>>>> ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
>>>> if (!ifbdev)
>>>> - return -ENOMEM;
>>>> -
>>>> - mutex_init(&ifbdev->hpd_lock);
>>>> + return;
>>>> drm_fb_helper_prepare(dev, &ifbdev->helper, 32,
>>>> &intel_fb_helper_funcs);
>>>>
>>>> + dev_priv->display.fbdev.fbdev = ifbdev;
>>>> + INIT_WORK(&dev_priv->display.fbdev.suspend_work,
>>>> intel_fbdev_suspend_worker);
>>>> + mutex_init(&ifbdev->hpd_lock);
>>>> if (intel_fbdev_init_bios(dev, ifbdev))
>>>> ifbdev->helper.preferred_bpp = ifbdev->preferred_bpp;
>>>> else
>>>> @@ -726,68 +753,19 @@ int intel_fbdev_init(struct drm_device *dev)
>>>>
>>>> ret = drm_client_init(dev, &ifbdev->helper.client, "i915-
>>>> fbdev",
>>>> &intel_fbdev_client_funcs);
>>>> - if (ret)
>>>> + if (ret) {
>>>> + drm_err(dev, "Failed to register client: %d\n", ret);
>>>> goto err_drm_fb_helper_unprepare;
>>>> + }
>>>>
>>>> - ret = drm_fb_helper_init(dev, &ifbdev->helper);
>>>> - if (ret)
>>>> - goto err_drm_client_release;
>>>> -
>>>> - dev_priv->display.fbdev.fbdev = ifbdev;
>>>> - INIT_WORK(&dev_priv->display.fbdev.suspend_work,
>>>> intel_fbdev_suspend_worker);
>>>> + drm_client_register(&ifbdev->helper.client);
>>>>
>>>> - return 0;
>>>> + return;
>>>>
>>>> -err_drm_client_release:
>>>> - drm_client_release(&ifbdev->helper.client);
>>>> err_drm_fb_helper_unprepare:
>>>> drm_fb_helper_unprepare(&ifbdev->helper);
>>>> + mutex_destroy(&ifbdev->hpd_lock);
>>>> kfree(ifbdev);
>>>> - return ret;
>>>> -}
>>>> -
>>>> -static void intel_fbdev_initial_config(void *data, async_cookie_t
>>>> cookie)
>>>> -{
>>>> - struct intel_fbdev *ifbdev = data;
>>>> -
>>>> - /* Due to peculiar init order wrt to hpd handling this is
>>>> separate. */
>>>> - if (drm_fb_helper_initial_config(&ifbdev->helper))
>>>> - intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
>>>> -}
>>>> -
>>>> -void intel_fbdev_initial_config_async(struct drm_i915_private
>>>> *dev_priv)
>>>> -{
>>>> - struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
>>>> -
>>>> - if (!ifbdev)
>>>> - return;
>>>> -
>>>> - ifbdev->cookie = async_schedule(intel_fbdev_initial_config,
>>>> ifbdev);
>>>> -}
>>>> -
>>>> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
>>>> -{
>>>> - struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
>>>> -
>>>> - if (!ifbdev)
>>>> - return;
>>>> -
>>>> - intel_fbdev_set_suspend(&dev_priv->drm,
>>>> FBINFO_STATE_SUSPENDED, true);
>>>> -
>>>> - if (!current_is_async())
>>>> - intel_fbdev_sync(ifbdev);
>>>> -
>>>> - drm_fb_helper_unregister_info(&ifbdev->helper);
>>>> -}
>>>> -
>>>> -void intel_fbdev_fini(struct drm_i915_private *dev_priv)
>>>> -{
>>>> - struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv-
>>>>> display.fbdev.fbdev);
>>>> -
>>>> - if (!ifbdev)
>>>> - return;
>>>> -
>>>> - intel_fbdev_destroy(ifbdev);
>>>> }
>>>>
>>>> struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev
>>>> *fbdev)
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>> b/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>> index 8c953f102ba22..08de2d5b34338 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
>>>> @@ -14,27 +14,11 @@ struct intel_fbdev;
>>>> struct intel_framebuffer;
>>>>
>>>> #ifdef CONFIG_DRM_FBDEV_EMULATION
>>>> -int intel_fbdev_init(struct drm_device *dev);
>>>> -void intel_fbdev_initial_config_async(struct drm_i915_private
>>>> *dev_priv);
>>>> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
>>>> -void intel_fbdev_fini(struct drm_i915_private *dev_priv);
>>>> +void intel_fbdev_setup(struct drm_i915_private *dev_priv);
>>>> void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool
>>>> synchronous);
>>>> struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev
>>>> *fbdev);
>>>> #else
>>>> -static inline int intel_fbdev_init(struct drm_device *dev)
>>>> -{
>>>> - return 0;
>>>> -}
>>>> -
>>>> -static inline void intel_fbdev_initial_config_async(struct
>>>> drm_i915_private *dev_priv)
>>>> -{
>>>> -}
>>>> -
>>>> -static inline void intel_fbdev_unregister(struct drm_i915_private
>>>> *dev_priv)
>>>> -{
>>>> -}
>>>> -
>>>> -static inline void intel_fbdev_fini(struct drm_i915_private
>>>> *dev_priv)
>>>> +static inline void intel_fbdev_setup(struct drm_i915_private
>>>> *dev_priv)
>>>> {
>>>> }
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_driver.c
>>>> b/drivers/gpu/drm/i915/i915_driver.c
>>>> index 86460cd8167d1..53663c0cc3be4 100644
>>>> --- a/drivers/gpu/drm/i915/i915_driver.c
>>>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>>>> @@ -817,6 +817,8 @@ int i915_driver_probe(struct pci_dev *pdev, const
>>>> struct pci_device_id *ent)
>>>>
>>>> i915->do_release = true;
>>>>
>>>> + intel_fbdev_setup(i915);
>>>> +
>>>> return 0;
>>>>
>>>> out_cleanup_gem:
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>
>
>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20231101/e80a5cb1/attachment-0001.sig>
More information about the dri-devel
mailing list