[Intel-gfx] [PATCH 2/4] drm/i915/fbdev: Use i915/drm_i915_private consistently
Rodrigo Vivi
rodrigo.vivi at intel.com
Thu Sep 13 21:11:11 UTC 2018
On Thu, Sep 13, 2018 at 02:16:27PM +0100, Chris Wilson wrote:
> Convert the interface (except for the drm core callback) to accept
> drm_i915_private as that is our native type of the majority of
> operations.
this message just tells about s/dev/dev_priv...
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 10 +--
> drivers/gpu/drm/i915/intel_drv.h | 28 +++----
> drivers/gpu/drm/i915/intel_fbdev.c | 117 ++++++++++++++---------------
> 3 files changed, 77 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 5dd7fc582e6f..a74de4428c79 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -692,7 +692,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
> if (INTEL_INFO(dev_priv)->num_pipes == 0)
> return 0;
>
> - ret = intel_fbdev_init(dev);
> + ret = intel_fbdev_init(dev_priv);
> if (ret)
> goto cleanup_gem;
>
> @@ -1260,7 +1260,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
> * irqs are fully enabled. We do it last so that the async config
> * cannot run before the connectors are registered.
> */
> - intel_fbdev_initial_config_async(dev);
> + intel_fbdev_initial_config_async(dev_priv);
>
> /*
> * We need to coordinate the hotplugs with the asynchronous fbdev
> @@ -1527,7 +1527,7 @@ static int i915_driver_open(struct drm_device *dev, struct drm_file *file)
> */
> static void i915_driver_lastclose(struct drm_device *dev)
> {
> - intel_fbdev_restore_mode(dev);
> + intel_fbdev_restore_mode(to_i915(dev));
> vga_switcheroo_process_delayed_switch();
> }
>
> @@ -1623,7 +1623,7 @@ static int i915_drm_suspend(struct drm_device *dev)
>
> intel_opregion_unregister(dev_priv);
>
> - intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
> + intel_fbdev_set_suspend(dev_priv, FBINFO_STATE_SUSPENDED, true);
>
> dev_priv->suspend_count++;
>
> @@ -1784,7 +1784,7 @@ static int i915_drm_resume(struct drm_device *dev)
>
> intel_opregion_register(dev_priv);
>
> - intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
> + intel_fbdev_set_suspend(dev_priv, FBINFO_STATE_RUNNING, false);
>
> intel_opregion_notify_adapter(dev_priv, PCI_D0);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bf1c38728a59..daa2fc007d02 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1779,32 +1779,34 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder,
>
> /* legacy fbdev emulation in intel_fbdev.c */
> #ifdef CONFIG_DRM_FBDEV_EMULATION
> -extern int intel_fbdev_init(struct drm_device *dev);
> -extern void intel_fbdev_initial_config_async(struct drm_device *dev);
> -extern void intel_fbdev_unregister(struct drm_i915_private *dev_priv);
> -extern void intel_fbdev_fini(struct drm_i915_private *dev_priv);
> -extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
> -extern void intel_fbdev_output_poll_changed(struct drm_device *dev);
> -extern void intel_fbdev_restore_mode(struct drm_device *dev);
> +int intel_fbdev_init(struct drm_i915_private *i915);
> +void intel_fbdev_initial_config_async(struct drm_i915_private *i915);
> +void intel_fbdev_unregister(struct drm_i915_private *i915);
> +void intel_fbdev_fini(struct drm_i915_private *i915);
> +void intel_fbdev_set_suspend(struct drm_i915_private *i915,
> + int state, bool synchronous);
> +void intel_fbdev_restore_mode(struct drm_i915_private *i915);
> +void intel_fbdev_output_poll_changed(struct drm_device *dev);
> #else
> -static inline int intel_fbdev_init(struct drm_device *dev)
> +static inline int intel_fbdev_init(struct drm_i915_private *i915)
> {
> return 0;
> }
>
> -static inline void intel_fbdev_initial_config_async(struct drm_device *dev)
> +static inline void intel_fbdev_initial_config_async(struct drm_i915_private *i915)
> {
> }
>
> -static inline void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
> +static inline void intel_fbdev_unregister(struct drm_i915_private *i915)
> {
> }
>
> -static inline void intel_fbdev_fini(struct drm_i915_private *dev_priv)
> +static inline void intel_fbdev_fini(struct drm_i915_private *i915)
> {
> }
>
> -static inline void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous)
> +static inline void intel_fbdev_set_suspend(struct drm_i915_private *i915,
> + int state, bool synchronous)
> {
> }
>
> @@ -1812,7 +1814,7 @@ static inline void intel_fbdev_output_poll_changed(struct drm_device *dev)
> {
> }
>
> -static inline void intel_fbdev_restore_mode(struct drm_device *dev)
> +static inline void intel_fbdev_restore_mode(struct drm_i915_private *i915)
> {
> }
> #endif
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index f99332972b7a..84ebd8102215 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -54,11 +54,14 @@ static void intel_fbdev_invalidate(struct intel_fbdev *ifbdev)
> intel_fb_obj_invalidate(obj, origin);
> }
>
> +static struct intel_fbdev *info_to_fbdev(struct fb_info *info)
> +{
> + return container_of(info->par, struct intel_fbdev, helper);
> +}
> +
So, could we split this info_to_fbdev helper in a separated patch?
> static int intel_fbdev_set_par(struct fb_info *info)
> {
> - struct drm_fb_helper *fb_helper = info->par;
> - struct intel_fbdev *ifbdev =
> - container_of(fb_helper, struct intel_fbdev, helper);
> + struct intel_fbdev *ifbdev = info_to_fbdev(info);
> int ret;
>
> ret = drm_fb_helper_set_par(info);
> @@ -70,9 +73,7 @@ static int intel_fbdev_set_par(struct fb_info *info)
>
> static int intel_fbdev_blank(int blank, struct fb_info *info)
> {
> - struct drm_fb_helper *fb_helper = info->par;
> - struct intel_fbdev *ifbdev =
> - container_of(fb_helper, struct intel_fbdev, helper);
> + struct intel_fbdev *ifbdev = info_to_fbdev(info);
> int ret;
>
> ret = drm_fb_helper_blank(blank, info);
> @@ -85,9 +86,7 @@ static int intel_fbdev_blank(int blank, struct fb_info *info)
> static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
> struct fb_info *info)
> {
> - struct drm_fb_helper *fb_helper = info->par;
> - struct intel_fbdev *ifbdev =
> - container_of(fb_helper, struct intel_fbdev, helper);
> + struct intel_fbdev *ifbdev = info_to_fbdev(info);
> int ret;
>
> ret = drm_fb_helper_pan_display(var, info);
> @@ -113,11 +112,10 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> {
> struct intel_fbdev *ifbdev =
> container_of(helper, struct intel_fbdev, helper);
> - struct drm_framebuffer *fb;
> - struct drm_device *dev = helper->dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_i915_private *i915 = to_i915(helper->dev);
> struct drm_mode_fb_cmd2 mode_cmd = {};
> struct drm_i915_gem_object *obj;
> + struct drm_framebuffer *fb;
> int size, ret;
>
> /* we don't do packed 24bpp */
> @@ -139,10 +137,10 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> * important and we should probably use that space with FBC or other
> * features. */
> obj = NULL;
> - if (size * 2 < dev_priv->stolen_usable_size)
> - obj = i915_gem_object_create_stolen(dev_priv, size);
> + if (size * 2 < i915->stolen_usable_size)
> + obj = i915_gem_object_create_stolen(i915, size);
> if (obj == NULL)
> - obj = i915_gem_object_create(dev_priv, size);
> + obj = i915_gem_object_create(i915, size);
> if (IS_ERR(obj)) {
> DRM_ERROR("failed to allocate framebuffer\n");
> ret = PTR_ERR(obj);
> @@ -170,16 +168,14 @@ static int intelfb_create(struct drm_fb_helper *helper,
> {
> struct intel_fbdev *ifbdev =
> container_of(helper, struct intel_fbdev, helper);
> + struct drm_i915_private *i915 = to_i915(helper->dev);
> struct intel_framebuffer *intel_fb = ifbdev->fb;
> - struct drm_device *dev = helper->dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct pci_dev *pdev = dev_priv->drm.pdev;
> - struct i915_ggtt *ggtt = &dev_priv->ggtt;
> + struct i915_ggtt *ggtt = &i915->ggtt;
> const struct i915_ggtt_view view = {
> .type = I915_GGTT_VIEW_NORMAL,
> };
> - struct fb_info *info;
> struct drm_framebuffer *fb;
> + struct fb_info *info;
> struct i915_vma *vma;
> unsigned long flags = 0;
> bool prealloc = false;
> @@ -209,8 +205,8 @@ static int intelfb_create(struct drm_fb_helper *helper,
> sizes->fb_height = intel_fb->base.height;
> }
>
> - mutex_lock(&dev->struct_mutex);
> - intel_runtime_pm_get(dev_priv);
> + mutex_lock(&i915->drm.struct_mutex);
> + intel_runtime_pm_get(i915);
>
> /* Pin the GGTT vma for our access via info->screen_base.
> * This also validates that any existing fb inherited from the
> @@ -242,10 +238,11 @@ static int intelfb_create(struct drm_fb_helper *helper,
> info->fbops = &intelfb_ops;
>
> /* setup aperture base/size for vesafb takeover */
> - info->apertures->ranges[0].base = dev->mode_config.fb_base;
> + info->apertures->ranges[0].base = i915->drm.mode_config.fb_base;
> info->apertures->ranges[0].size = ggtt->mappable_end;
>
> - info->fix.smem_start = dev->mode_config.fb_base + i915_ggtt_offset(vma);
> + info->fix.smem_start =
> + i915->drm.mode_config.fb_base + i915_ggtt_offset(vma);
> info->fix.smem_len = vma->node.size;
>
> vaddr = i915_vma_pin_iomap(vma);
> @@ -277,16 +274,16 @@ static int intelfb_create(struct drm_fb_helper *helper,
> ifbdev->vma = vma;
> ifbdev->vma_flags = flags;
>
> - intel_runtime_pm_put(dev_priv);
> - mutex_unlock(&dev->struct_mutex);
> - vga_switcheroo_client_fb_set(pdev, info);
> + intel_runtime_pm_put(i915);
> + mutex_unlock(&i915->drm.struct_mutex);
> + vga_switcheroo_client_fb_set(i915->drm.pdev, info);
> return 0;
>
> out_unpin:
> intel_unpin_fb_vma(vma, flags);
> out_unlock:
> - intel_runtime_pm_put(dev_priv);
> - mutex_unlock(&dev->struct_mutex);
> + intel_runtime_pm_put(i915);
> + mutex_unlock(&i915->drm.struct_mutex);
> return ret;
> }
>
> @@ -335,7 +332,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> struct drm_fb_offset *offsets,
> bool *enabled, int width, int height)
> {
> - struct drm_i915_private *dev_priv = to_i915(fb_helper->dev);
> + struct drm_i915_private *i915 = to_i915(fb_helper->dev);
> unsigned long conn_configured, conn_seq, mask;
> unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG);
> int i, j;
> @@ -484,7 +481,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> * fbdev helper library.
> */
> if (num_connectors_enabled != num_connectors_detected &&
> - num_connectors_enabled < INTEL_INFO(dev_priv)->num_pipes) {
> + num_connectors_enabled < INTEL_INFO(i915)->num_pipes) {
> DRM_DEBUG_KMS("fallback: Not all outputs enabled\n");
> DRM_DEBUG_KMS("Enabled: %i, detected: %i\n", num_connectors_enabled,
> num_connectors_detected);
> @@ -540,7 +537,7 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
> * Note we only support a single fb shared across pipes for boot (mostly for
> * fbcon), so we just find the biggest and use that.
> */
> -static bool intel_fbdev_init_bios(struct drm_device *dev,
> +static bool intel_fbdev_init_bios(struct drm_i915_private *i915,
> struct intel_fbdev *ifbdev)
> {
> struct intel_framebuffer *fb = NULL;
> @@ -549,7 +546,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
> unsigned int max_size = 0;
>
> /* Find the largest fb */
> - for_each_crtc(dev, crtc) {
> + for_each_crtc(&i915->drm, crtc) {
> struct drm_i915_gem_object *obj =
> intel_fb_obj(crtc->primary->state->fb);
> intel_crtc = to_intel_crtc(crtc);
> @@ -574,7 +571,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
> }
>
> /* Now make sure all the pipes will fit into it */
> - for_each_crtc(dev, crtc) {
> + for_each_crtc(&i915->drm, crtc) {
> unsigned int cur_size;
>
> intel_crtc = to_intel_crtc(crtc);
> @@ -637,7 +634,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
> drm_framebuffer_get(&ifbdev->fb->base);
>
> /* Final pass to check if any active pipes don't have fbs */
> - for_each_crtc(dev, crtc) {
> + for_each_crtc(&i915->drm, crtc) {
> intel_crtc = to_intel_crtc(crtc);
>
> if (!crtc->state->active)
> @@ -659,39 +656,39 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
>
> static void intel_fbdev_suspend_worker(struct work_struct *work)
> {
> - intel_fbdev_set_suspend(&container_of(work,
> - struct drm_i915_private,
> - fbdev_suspend_work)->drm,
> + intel_fbdev_set_suspend(container_of(work,
> + struct drm_i915_private,
> + fbdev_suspend_work),
> FBINFO_STATE_RUNNING,
> true);
> }
>
> -int intel_fbdev_init(struct drm_device *dev)
> +int intel_fbdev_init(struct drm_i915_private *i915)
> {
> - struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_fbdev *ifbdev;
> int ret;
>
> - if (WARN_ON(INTEL_INFO(dev_priv)->num_pipes == 0))
> + if (WARN_ON(INTEL_INFO(i915)->num_pipes == 0))
Also this patch highlights the brave new world of mixed dev_priv and i915
that we live....
Why not to just use dev_priv here and start mixing this i915 case here?
on the same file and on same patch?
I believe or we should convert at once all dev_priv to i915 or move
all back to dev_priv. But the mixed world confuses me a lot...
> return -ENODEV;
>
> ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
> if (ifbdev == NULL)
> return -ENOMEM;
>
> - drm_fb_helper_prepare(dev, &ifbdev->helper, &intel_fb_helper_funcs);
> + drm_fb_helper_prepare(&i915->drm,
> + &ifbdev->helper, &intel_fb_helper_funcs);
>
> - if (!intel_fbdev_init_bios(dev, ifbdev))
> + if (!intel_fbdev_init_bios(i915, ifbdev))
> ifbdev->preferred_bpp = 32;
>
> - ret = drm_fb_helper_init(dev, &ifbdev->helper, 4);
> + ret = drm_fb_helper_init(&i915->drm, &ifbdev->helper, 4);
> if (ret) {
> kfree(ifbdev);
> return ret;
> }
>
> - dev_priv->fbdev = ifbdev;
> - INIT_WORK(&dev_priv->fbdev_suspend_work, intel_fbdev_suspend_worker);
> + i915->fbdev = ifbdev;
> + INIT_WORK(&i915->fbdev_suspend_work, intel_fbdev_suspend_worker);
>
> drm_fb_helper_single_add_all_connectors(&ifbdev->helper);
>
> @@ -708,9 +705,9 @@ static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
> }
>
> -void intel_fbdev_initial_config_async(struct drm_device *dev)
> +void intel_fbdev_initial_config_async(struct drm_i915_private *i915)
> {
> - struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
> + struct intel_fbdev *ifbdev = i915->fbdev;
>
> if (!ifbdev)
> return;
> @@ -728,23 +725,23 @@ static void intel_fbdev_sync(struct intel_fbdev *ifbdev)
> ifbdev->cookie = 0;
> }
>
> -void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
> +void intel_fbdev_unregister(struct drm_i915_private *i915)
> {
> - struct intel_fbdev *ifbdev = dev_priv->fbdev;
> + struct intel_fbdev *ifbdev = i915->fbdev;
>
> if (!ifbdev)
> return;
>
> - cancel_work_sync(&dev_priv->fbdev_suspend_work);
> + cancel_work_sync(&i915->fbdev_suspend_work);
> if (!current_is_async())
> intel_fbdev_sync(ifbdev);
>
> drm_fb_helper_unregister_fbi(&ifbdev->helper);
> }
>
> -void intel_fbdev_fini(struct drm_i915_private *dev_priv)
> +void intel_fbdev_fini(struct drm_i915_private *i915)
> {
> - struct intel_fbdev *ifbdev = fetch_and_zero(&dev_priv->fbdev);
> + struct intel_fbdev *ifbdev = fetch_and_zero(&i915->fbdev);
>
> if (!ifbdev)
> return;
> @@ -752,10 +749,10 @@ void intel_fbdev_fini(struct drm_i915_private *dev_priv)
> intel_fbdev_destroy(ifbdev);
> }
>
> -void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous)
> +void intel_fbdev_set_suspend(struct drm_i915_private *i915,
> + int state, bool synchronous)
> {
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_fbdev *ifbdev = dev_priv->fbdev;
> + struct intel_fbdev *ifbdev = i915->fbdev;
> struct fb_info *info;
>
> if (!ifbdev || !ifbdev->vma)
> @@ -772,7 +769,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
> * ourselves, so only flush outstanding work upon suspend!
> */
> if (state != FBINFO_STATE_RUNNING)
> - flush_work(&dev_priv->fbdev_suspend_work);
> + flush_work(&i915->fbdev_suspend_work);
> console_lock();
> } else {
> /*
> @@ -785,7 +782,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
> /* Don't block our own workqueue as this can
> * be run in parallel with other i915.ko tasks.
> */
> - schedule_work(&dev_priv->fbdev_suspend_work);
> + schedule_work(&i915->fbdev_suspend_work);
> return;
> }
> }
> @@ -814,9 +811,9 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev)
> drm_fb_helper_hotplug_event(&ifbdev->helper);
> }
>
> -void intel_fbdev_restore_mode(struct drm_device *dev)
> +void intel_fbdev_restore_mode(struct drm_i915_private *i915)
> {
> - struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
> + struct intel_fbdev *ifbdev = i915->fbdev;
>
> if (!ifbdev)
> return;
> --
> 2.19.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list