Hi Thomas, couple of inline commments/suggestions below.
On Wed, 2023-09-27 at 12:26 +0200, Thomas Zimmermann wrote:
> Move code from ad-hoc fbdev callbacks into DRM client functions
> and remove the old callbacks. The functions instruct the client
> to poll for changed output or restore the display.
>
> The DRM core calls both, the old callbacks and the new client
> helpers, from the same places. The new functions perform the same
> operation as before, so there's no change in functionality.
>
> Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
> ---
> .../drm/i915/display/intel_display_driver.c | 1 -
> drivers/gpu/drm/i915/display/intel_fbdev.c | 11 ++++++++--
> drivers/gpu/drm/i915/display/intel_fbdev.h | 9 --------
> drivers/gpu/drm/i915/i915_driver.c | 22 -----------------
> --
> 4 files changed, 9 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
> b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 44b59ac301e69..ffdcddd1943e0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -96,7 +96,6 @@ void intel_display_driver_init_hw(struct
> drm_i915_private *i915)
> static const struct drm_mode_config_funcs intel_mode_funcs = {
> .fb_create = intel_user_framebuffer_create,
> .get_format_info = intel_fb_get_format_info,
> - .output_poll_changed = intel_fbdev_output_poll_changed,
> .mode_valid = intel_mode_valid,
> .atomic_check = intel_atomic_check,
> .atomic_commit = intel_atomic_commit,
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
> b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index d9e69471a782a..39de61d4e7906 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -638,7 +638,7 @@ void intel_fbdev_set_suspend(struct drm_device
> *dev, int state, bool synchronous
> intel_fbdev_hpd_set_suspend(dev_priv, state);
> }
>
> -void intel_fbdev_output_poll_changed(struct drm_device *dev)
> +static void intel_fbdev_output_poll_changed(struct drm_device *dev)
Now as this isn't drm_mode_config_funcs callback anymore: Maybe you
could return error value/0 ?
> {
> struct intel_fbdev *ifbdev = to_i915(dev)-
> >display.fbdev.fbdev;
> bool send_hpd;
> @@ -657,7 +657,7 @@ void intel_fbdev_output_poll_changed(struct
> drm_device *dev)
> drm_fb_helper_hotplug_event(&ifbdev->helper);
> }
>
> -void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv)
> +static void intel_fbdev_restore_mode(struct drm_i915_private
Similar comment as above. I.e. return error value/0 ?
BR,
Jouni Högander
> *dev_priv)
> {
> struct intel_fbdev *ifbdev = dev_priv->display.fbdev.fbdev;
>
> @@ -681,11 +681,18 @@ static void
> intel_fbdev_client_unregister(struct drm_client_dev *client)
>
> static int intel_fbdev_client_restore(struct drm_client_dev *client)
> {
> + struct drm_i915_private *dev_priv = to_i915(client->dev);
> +
> + intel_fbdev_restore_mode(dev_priv);
> + vga_switcheroo_process_delayed_switch();
> +
> return 0;
> }
>
> static int intel_fbdev_client_hotplug(struct drm_client_dev *client)
> {
> + intel_fbdev_output_poll_changed(client->dev);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h
> b/drivers/gpu/drm/i915/display/intel_fbdev.h
> index 04fd523a50232..8c953f102ba22 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.h
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
> @@ -19,8 +19,6 @@ 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_set_suspend(struct drm_device *dev, int state, bool
> synchronous);
> -void intel_fbdev_output_poll_changed(struct drm_device *dev);
> -void intel_fbdev_restore_mode(struct drm_i915_private *dev_priv);
> struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev
> *fbdev);
> #else
> static inline int intel_fbdev_init(struct drm_device *dev)
> @@ -44,13 +42,6 @@ static inline void intel_fbdev_set_suspend(struct
> drm_device *dev, int state, bo
> {
> }
>
> -static inline void intel_fbdev_output_poll_changed(struct drm_device
> *dev)
> -{
> -}
> -
> -static inline void intel_fbdev_restore_mode(struct drm_i915_private
> *i915)
> -{
> -}
> static inline struct intel_framebuffer
> *intel_fbdev_framebuffer(struct intel_fbdev *fbdev)
> {
> return NULL;
> diff --git a/drivers/gpu/drm/i915/i915_driver.c
> b/drivers/gpu/drm/i915/i915_driver.c
> index de19197d2e052..86460cd8167d1 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -924,27 +924,6 @@ static int i915_driver_open(struct drm_device
> *dev, struct drm_file *file)
> return 0;
> }
>
> -/**
> - * i915_driver_lastclose - clean up after all DRM clients have
> exited
> - * @dev: DRM device
> - *
> - * Take care of cleaning up after all DRM clients have exited. In
> the
> - * mode setting case, we want to restore the kernel's initial mode
> (just
> - * in case the last client left us in a bad state).
> - *
> - * Additionally, in the non-mode setting case, we'll tear down the
> GTT
> - * and DMA structures, since the kernel won't be using them, and
> clea
> - * up any GEM state.
> - */
> -static void i915_driver_lastclose(struct drm_device *dev)
> -{
> - struct drm_i915_private *i915 = to_i915(dev);
> -
> - intel_fbdev_restore_mode(i915);
> -
> - vga_switcheroo_process_delayed_switch();
> -}
> -
> static void i915_driver_postclose(struct drm_device *dev, struct
> drm_file *file)
> {
> struct drm_i915_file_private *file_priv = file->driver_priv;
> @@ -1822,7 +1801,6 @@ static const struct drm_driver i915_drm_driver
> = {
> DRIVER_SYNCOBJ_TIMELINE,
> .release = i915_driver_release,
> .open = i915_driver_open,
> - .lastclose = i915_driver_lastclose,
> .postclose = i915_driver_postclose,
> .show_fdinfo = PTR_IF(IS_ENABLED(CONFIG_PROC_FS),
> i915_drm_client_fdinfo),
>
