[Intel-gfx] [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/intel-gfx/attachments/20231101/e80a5cb1/attachment-0001.sig>


More information about the Intel-gfx mailing list