Hi

Am 25.10.23 um 11:36 schrieb Hogander, Jouni:
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 ?

Yes, of course. After i915 has been converted to use drm_client, we can turn all this into consistent error handling across all the various drivers' fbdev emulation.


  {
         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 ?

Same here.

Best regards
Thomas


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),


--
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)

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature

Reply via email to