[PATCH 0/6] drm/msm: Convert fbdev to DRM client

[PATCH 0/6] drm/msm: Convert fbdev to DRM client

[Thomas Zimmermann]

Convert msm' fbdev code to struct drm_client. Replaces the current
ad-hoc integration. The conversion includes a number of cleanups. As
with most other drivers' fbdev emulation, fbdev in msm is now just
another DRM client that runs after the DRM device has been registered.

Once all drivers' fbdev emulation has been converted to struct drm_client,
we can attempt to add additional in-kernel clients. A DRM-based dmesg
log or a bootsplash are commonly mentioned. DRM can then switch easily
among the existing clients if/when required.

I did the conversion from similar experience with other drivers. But I
don't have the hardware to test this. Any testing is welcome.

Thomas Zimmermann (6):
  drm/msm: Clear aperture ownership outside of fbdev code
  drm/msm: Remove fb from struct msm_fbdev
  drm/msm: Remove struct msm_fbdev
  drm/msm: Remove fbdev from struct msm_drm_private
  drm/msm: Initialize fbdev DRM client
  drm/msm: Implement fbdev emulation as in-kernel client

 drivers/gpu/drm/msm/msm_debugfs.c |   6 +-
 drivers/gpu/drm/msm/msm_drv.c     |  21 ++--
 drivers/gpu/drm/msm/msm_drv.h     |  12 ++-
 drivers/gpu/drm/msm/msm_fbdev.c   | 168 ++++++++++++++++++------------
 4 files changed, 118 insertions(+), 89 deletions(-)

-- 
2.40.0

[PATCH 1/6] drm/msm: Clear aperture ownership outside of fbdev code

[Thomas Zimmermann]

Move aperture management out of the fbdev code. It is unrelated
and needs to run even if fbdev support has been disabled. Call
the helper at the top of msm_drm_init() to take over hardware
from other drivers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/msm/msm_drv.c   | 6 ++++++
 drivers/gpu/drm/msm/msm_fbdev.c | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index aca48c868c14..5211140ec50b 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -12,6 +12,7 @@
 #include <linux/uaccess.h>
 #include <uapi/linux/sched/types.h>
 
+#include <drm/drm_aperture.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_file.h>
@@ -411,6 +412,11 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
 	if (drm_firmware_drivers_only())
 		return -ENODEV;
 
+	/* the fw fb could be anywhere in memory */
+	ret = drm_aperture_remove_framebuffers(false, drv);
+	if (ret)
+		return ret;
+
 	ddev = drm_dev_alloc(drv, dev);
 	if (IS_ERR(ddev)) {
 		DRM_DEV_ERROR(dev, "failed to allocate drm_device\n");
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index d26aa52217ce..fc7d0406a9f9 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -4,7 +4,6 @@
  * Author: Rob Clark <robdclark@gmail.com>
  */
 
-#include <drm/drm_aperture.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
@@ -154,11 +153,6 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 		goto fail;
 	}
 
-	/* the fw fb could be anywhere in memory */
-	ret = drm_aperture_remove_framebuffers(false, dev->driver);
-	if (ret)
-		goto fini;
-
 	ret = drm_fb_helper_initial_config(helper);
 	if (ret)
 		goto fini;
-- 
2.40.0

Re: [PATCH 1/6] drm/msm: Clear aperture ownership outside of fbdev code

[Dmitry Baryshkov]

On Thu, 30 Mar 2023 at 10:41, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Move aperture management out of the fbdev code. It is unrelated
> and needs to run even if fbdev support has been disabled. Call
> the helper at the top of msm_drm_init() to take over hardware
> from other drivers.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/msm/msm_drv.c   | 6 ++++++
>  drivers/gpu/drm/msm/msm_fbdev.c | 6 ------
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index aca48c868c14..5211140ec50b 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -12,6 +12,7 @@
>  #include <linux/uaccess.h>
>  #include <uapi/linux/sched/types.h>
>
> +#include <drm/drm_aperture.h>
>  #include <drm/drm_bridge.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_file.h>
> @@ -411,6 +412,11 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
>         if (drm_firmware_drivers_only())
>                 return -ENODEV;
>
> +       /* the fw fb could be anywhere in memory */
> +       ret = drm_aperture_remove_framebuffers(false, drv);
> +       if (ret)
> +               return ret;
> +

I think it is not a good place to remove framebuffers. EFIFB might be
still alive and if we kick it out, it might be very hard to debug what
went wrong.
Could you please move it after component bind? Then we can be sure at
least that all subdevices are bound. I see that armada and sun4i call
it as late as possible, when no calls can fail.

>         ddev = drm_dev_alloc(drv, dev);
>         if (IS_ERR(ddev)) {
>                 DRM_DEV_ERROR(dev, "failed to allocate drm_device\n");
> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
> index d26aa52217ce..fc7d0406a9f9 100644
> --- a/drivers/gpu/drm/msm/msm_fbdev.c
> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
> @@ -4,7 +4,6 @@
>   * Author: Rob Clark <robdclark@gmail.com>
>   */
>
> -#include <drm/drm_aperture.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_fourcc.h>
> @@ -154,11 +153,6 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
>                 goto fail;
>         }
>
> -       /* the fw fb could be anywhere in memory */
> -       ret = drm_aperture_remove_framebuffers(false, dev->driver);
> -       if (ret)
> -               goto fini;
> -
>         ret = drm_fb_helper_initial_config(helper);
>         if (ret)
>                 goto fini;
> --
> 2.40.0
>


-- 
With best wishes
Dmitry

Re: [PATCH 1/6] drm/msm: Clear aperture ownership outside of fbdev code

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3142 bytes --]

Hi

Am 30.03.23 um 11:51 schrieb Dmitry Baryshkov:
> On Thu, 30 Mar 2023 at 10:41, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Move aperture management out of the fbdev code. It is unrelated
>> and needs to run even if fbdev support has been disabled. Call
>> the helper at the top of msm_drm_init() to take over hardware
>> from other drivers.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/msm/msm_drv.c   | 6 ++++++
>>   drivers/gpu/drm/msm/msm_fbdev.c | 6 ------
>>   2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
>> index aca48c868c14..5211140ec50b 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.c
>> +++ b/drivers/gpu/drm/msm/msm_drv.c
>> @@ -12,6 +12,7 @@
>>   #include <linux/uaccess.h>
>>   #include <uapi/linux/sched/types.h>
>>
>> +#include <drm/drm_aperture.h>
>>   #include <drm/drm_bridge.h>
>>   #include <drm/drm_drv.h>
>>   #include <drm/drm_file.h>
>> @@ -411,6 +412,11 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
>>          if (drm_firmware_drivers_only())
>>                  return -ENODEV;
>>
>> +       /* the fw fb could be anywhere in memory */
>> +       ret = drm_aperture_remove_framebuffers(false, drv);
>> +       if (ret)
>> +               return ret;
>> +
> 
> I think it is not a good place to remove framebuffers. EFIFB might be
> still alive and if we kick it out, it might be very hard to debug what
> went wrong.
> Could you please move it after component bind? Then we can be sure at
> least that all subdevices are bound. I see that armada and sun4i call
> it as late as possible, when no calls can fail.

Ok. I briefly looked over the code to make sure that no code touches HW 
settings before msm acquires the device.

Best regards
Thomas

> 
>>          ddev = drm_dev_alloc(drv, dev);
>>          if (IS_ERR(ddev)) {
>>                  DRM_DEV_ERROR(dev, "failed to allocate drm_device\n");
>> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
>> index d26aa52217ce..fc7d0406a9f9 100644
>> --- a/drivers/gpu/drm/msm/msm_fbdev.c
>> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
>> @@ -4,7 +4,6 @@
>>    * Author: Rob Clark <robdclark@gmail.com>
>>    */
>>
>> -#include <drm/drm_aperture.h>
>>   #include <drm/drm_crtc.h>
>>   #include <drm/drm_fb_helper.h>
>>   #include <drm/drm_fourcc.h>
>> @@ -154,11 +153,6 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
>>                  goto fail;
>>          }
>>
>> -       /* the fw fb could be anywhere in memory */
>> -       ret = drm_aperture_remove_framebuffers(false, dev->driver);
>> -       if (ret)
>> -               goto fini;
>> -
>>          ret = drm_fb_helper_initial_config(helper);
>>          if (ret)
>>                  goto fini;
>> --
>> 2.40.0
>>
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[PATCH 2/6] drm/msm: Remove fb from struct msm_fbdev

[Thomas Zimmermann]

Fbdev's struct fb_helper stores a pointer to the framebuffer. Remove
struct msm_fbdev.fb, which contains thre same value. No functional
changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/msm/msm_fbdev.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index fc7d0406a9f9..323a79d9ef83 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -14,8 +14,6 @@
 #include "msm_gem.h"
 #include "msm_kms.h"
 
-static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma);
-
 /*
  * fbdev funcs, to implement legacy fbdev interface on top of drm driver
  */
@@ -24,9 +22,16 @@ static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma);
 
 struct msm_fbdev {
 	struct drm_fb_helper base;
-	struct drm_framebuffer *fb;
 };
 
+static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+	struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
+	struct drm_gem_object *bo = msm_framebuffer_bo(helper->fb, 0);
+
+	return drm_gem_prime_mmap(bo, vma);
+}
+
 static const struct fb_ops msm_fb_ops = {
 	.owner = THIS_MODULE,
 	DRM_FB_HELPER_DEFAULT_OPS,
@@ -42,19 +47,9 @@ static const struct fb_ops msm_fb_ops = {
 	.fb_mmap = msm_fbdev_mmap,
 };
 
-static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
-{
-	struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
-	struct msm_fbdev *fbdev = to_msm_fbdev(helper);
-	struct drm_gem_object *bo = msm_framebuffer_bo(fbdev->fb, 0);
-
-	return drm_gem_prime_mmap(bo, vma);
-}
-
 static int msm_fbdev_create(struct drm_fb_helper *helper,
 		struct drm_fb_helper_surface_size *sizes)
 {
-	struct msm_fbdev *fbdev = to_msm_fbdev(helper);
 	struct drm_device *dev = helper->dev;
 	struct msm_drm_private *priv = dev->dev_private;
 	struct drm_framebuffer *fb = NULL;
@@ -101,7 +96,6 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
 
 	DBG("fbi=%p, dev=%p", fbi, dev);
 
-	fbdev->fb = fb;
 	helper->fb = fb;
 
 	fbi->fbops = &msm_fb_ops;
@@ -118,7 +112,7 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
 	fbi->fix.smem_len = bo->size;
 
 	DBG("par=%p, %dx%d", fbi->par, fbi->var.xres, fbi->var.yres);
-	DBG("allocated %dx%d fb", fbdev->fb->width, fbdev->fb->height);
+	DBG("allocated %dx%d fb", fb->width, fb->height);
 
 	return 0;
 
@@ -173,6 +167,7 @@ void msm_fbdev_free(struct drm_device *dev)
 {
 	struct msm_drm_private *priv = dev->dev_private;
 	struct drm_fb_helper *helper = priv->fbdev;
+	struct drm_framebuffer *fb = helper->fb;
 	struct msm_fbdev *fbdev;
 
 	DBG();
@@ -184,11 +179,10 @@ void msm_fbdev_free(struct drm_device *dev)
 	fbdev = to_msm_fbdev(priv->fbdev);
 
 	/* this will free the backing object */
-	if (fbdev->fb) {
-		struct drm_gem_object *bo =
-			msm_framebuffer_bo(fbdev->fb, 0);
+	if (fb) {
+		struct drm_gem_object *bo = msm_framebuffer_bo(fb, 0);
 		msm_gem_put_vaddr(bo);
-		drm_framebuffer_remove(fbdev->fb);
+		drm_framebuffer_remove(fb);
 	}
 
 	drm_fb_helper_unprepare(helper);
-- 
2.40.0

Re: [PATCH 2/6] drm/msm: Remove fb from struct msm_fbdev

[Dmitry Baryshkov]

On Thu, 30 Mar 2023 at 10:41, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Fbdev's struct fb_helper stores a pointer to the framebuffer. Remove
> struct msm_fbdev.fb, which contains thre same value. No functional
> changes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/msm/msm_fbdev.c | 32 +++++++++++++-------------------
>  1 file changed, 13 insertions(+), 19 deletions(-)


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry

[PATCH 3/6] drm/msm: Remove struct msm_fbdev

[Thomas Zimmermann]

Remove struct msm_fbdev, which is an empty wrapper around struct
drm_fb_helper. Use the latter directly. No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/msm/msm_fbdev.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index 323a79d9ef83..0985486d194b 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -18,12 +18,6 @@
  * fbdev funcs, to implement legacy fbdev interface on top of drm driver
  */
 
-#define to_msm_fbdev(x) container_of(x, struct msm_fbdev, base)
-
-struct msm_fbdev {
-	struct drm_fb_helper base;
-};
-
 static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
@@ -129,16 +123,13 @@ static const struct drm_fb_helper_funcs msm_fb_helper_funcs = {
 struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 {
 	struct msm_drm_private *priv = dev->dev_private;
-	struct msm_fbdev *fbdev;
 	struct drm_fb_helper *helper;
 	int ret;
 
-	fbdev = kzalloc(sizeof(*fbdev), GFP_KERNEL);
-	if (!fbdev)
+	helper = kzalloc(sizeof(*helper), GFP_KERNEL);
+	if (!helper)
 		return NULL;
 
-	helper = &fbdev->base;
-
 	drm_fb_helper_prepare(dev, helper, 32, &msm_fb_helper_funcs);
 
 	ret = drm_fb_helper_init(dev, helper);
@@ -159,7 +150,6 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 	drm_fb_helper_fini(helper);
 fail:
 	drm_fb_helper_unprepare(helper);
-	kfree(fbdev);
 	return NULL;
 }
 
@@ -168,7 +158,6 @@ void msm_fbdev_free(struct drm_device *dev)
 	struct msm_drm_private *priv = dev->dev_private;
 	struct drm_fb_helper *helper = priv->fbdev;
 	struct drm_framebuffer *fb = helper->fb;
-	struct msm_fbdev *fbdev;
 
 	DBG();
 
@@ -176,8 +165,6 @@ void msm_fbdev_free(struct drm_device *dev)
 
 	drm_fb_helper_fini(helper);
 
-	fbdev = to_msm_fbdev(priv->fbdev);
-
 	/* this will free the backing object */
 	if (fb) {
 		struct drm_gem_object *bo = msm_framebuffer_bo(fb, 0);
@@ -186,7 +173,7 @@ void msm_fbdev_free(struct drm_device *dev)
 	}
 
 	drm_fb_helper_unprepare(helper);
-	kfree(fbdev);
+	kfree(helper);
 
 	priv->fbdev = NULL;
 }
-- 
2.40.0

Re: [PATCH 3/6] drm/msm: Remove struct msm_fbdev

[Dmitry Baryshkov]

On Thu, 30 Mar 2023 at 10:41, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Remove struct msm_fbdev, which is an empty wrapper around struct
> drm_fb_helper. Use the latter directly. No functional changes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/msm/msm_fbdev.c | 19 +++----------------
>  1 file changed, 3 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
> index 323a79d9ef83..0985486d194b 100644
> --- a/drivers/gpu/drm/msm/msm_fbdev.c
> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
> @@ -18,12 +18,6 @@
>   * fbdev funcs, to implement legacy fbdev interface on top of drm driver
>   */
>
> -#define to_msm_fbdev(x) container_of(x, struct msm_fbdev, base)
> -
> -struct msm_fbdev {
> -       struct drm_fb_helper base;
> -};
> -
>  static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
>  {
>         struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
> @@ -129,16 +123,13 @@ static const struct drm_fb_helper_funcs msm_fb_helper_funcs = {
>  struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
>  {
>         struct msm_drm_private *priv = dev->dev_private;
> -       struct msm_fbdev *fbdev;
>         struct drm_fb_helper *helper;
>         int ret;
>
> -       fbdev = kzalloc(sizeof(*fbdev), GFP_KERNEL);
> -       if (!fbdev)
> +       helper = kzalloc(sizeof(*helper), GFP_KERNEL);
> +       if (!helper)
>                 return NULL;
>
> -       helper = &fbdev->base;
> -
>         drm_fb_helper_prepare(dev, helper, 32, &msm_fb_helper_funcs);
>
>         ret = drm_fb_helper_init(dev, helper);
> @@ -159,7 +150,6 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
>         drm_fb_helper_fini(helper);
>  fail:
>         drm_fb_helper_unprepare(helper);
> -       kfree(fbdev);

I think this will leak the newly created helper instance.

>         return NULL;
>  }
>
> @@ -168,7 +158,6 @@ void msm_fbdev_free(struct drm_device *dev)
>         struct msm_drm_private *priv = dev->dev_private;
>         struct drm_fb_helper *helper = priv->fbdev;
>         struct drm_framebuffer *fb = helper->fb;
> -       struct msm_fbdev *fbdev;
>
>         DBG();
>
> @@ -176,8 +165,6 @@ void msm_fbdev_free(struct drm_device *dev)
>
>         drm_fb_helper_fini(helper);
>
> -       fbdev = to_msm_fbdev(priv->fbdev);
> -
>         /* this will free the backing object */
>         if (fb) {
>                 struct drm_gem_object *bo = msm_framebuffer_bo(fb, 0);
> @@ -186,7 +173,7 @@ void msm_fbdev_free(struct drm_device *dev)
>         }
>
>         drm_fb_helper_unprepare(helper);
> -       kfree(fbdev);
> +       kfree(helper);
>
>         priv->fbdev = NULL;
>  }
> --
> 2.40.0
>


-- 
With best wishes
Dmitry

[PATCH 4/6] drm/msm: Remove fbdev from struct msm_drm_private

[Thomas Zimmermann]

The DRM device stores a pointer to the fbdev helper. Remove struct
msm_drm_private.fbdev, which contains the same value. No functional
changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/msm/msm_debugfs.c | 5 ++---
 drivers/gpu/drm/msm/msm_drv.c     | 4 ++--
 drivers/gpu/drm/msm/msm_drv.h     | 2 --
 drivers/gpu/drm/msm/msm_fbdev.c   | 8 ++------
 4 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
index d6ecff0ab618..d4b31165708f 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -241,12 +241,11 @@ static int msm_fb_show(struct seq_file *m, void *arg)
 {
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
 	struct drm_device *dev = node->minor->dev;
-	struct msm_drm_private *priv = dev->dev_private;
 	struct drm_framebuffer *fb, *fbdev_fb = NULL;
 
-	if (priv->fbdev) {
+	if (dev->fb_helper) {
 		seq_printf(m, "fbcon ");
-		fbdev_fb = priv->fbdev->fb;
+		fbdev_fb = dev->fb_helper->fb;
 		msm_framebuffer_describe(fbdev_fb, m);
 	}
 
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 5211140ec50b..9f076b9ab19b 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -242,7 +242,7 @@ static int msm_drm_uninit(struct device *dev)
 	msm_rd_debugfs_cleanup(priv);
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
-	if (fbdev && priv->fbdev)
+	if (ddev->fb_helper)
 		msm_fbdev_free(ddev);
 #endif
 
@@ -537,7 +537,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
 	if (kms && fbdev)
-		priv->fbdev = msm_fbdev_init(ddev);
+		msm_fbdev_init(ddev);
 #endif
 
 	ret = msm_debugfs_late_init(ddev);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 9f0c184b02a0..852f88c36621 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -129,8 +129,6 @@ struct msm_drm_private {
 	bool is_a2xx;
 	bool has_cached_coherent;
 
-	struct drm_fb_helper *fbdev;
-
 	struct msm_rd_state *rd;       /* debugfs to dump all submits */
 	struct msm_rd_state *hangrd;   /* debugfs to dump hanging submits */
 	struct msm_perf_state *perf;
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index 0985486d194b..95b193a5e0d5 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -122,7 +122,6 @@ static const struct drm_fb_helper_funcs msm_fb_helper_funcs = {
 /* initialize fbdev helper */
 struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 {
-	struct msm_drm_private *priv = dev->dev_private;
 	struct drm_fb_helper *helper;
 	int ret;
 
@@ -142,8 +141,6 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 	if (ret)
 		goto fini;
 
-	priv->fbdev = helper;
-
 	return helper;
 
 fini:
@@ -155,8 +152,7 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 
 void msm_fbdev_free(struct drm_device *dev)
 {
-	struct msm_drm_private *priv = dev->dev_private;
-	struct drm_fb_helper *helper = priv->fbdev;
+	struct drm_fb_helper *helper = dev->fb_helper;
 	struct drm_framebuffer *fb = helper->fb;
 
 	DBG();
@@ -175,5 +171,5 @@ void msm_fbdev_free(struct drm_device *dev)
 	drm_fb_helper_unprepare(helper);
 	kfree(helper);
 
-	priv->fbdev = NULL;
+	dev->fb_helper = NULL;
 }
-- 
2.40.0

Re: [PATCH 4/6] drm/msm: Remove fbdev from struct msm_drm_private

[Dmitry Baryshkov]

On Thu, 30 Mar 2023 at 10:41, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> The DRM device stores a pointer to the fbdev helper. Remove struct
> msm_drm_private.fbdev, which contains the same value. No functional
> changes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/msm/msm_debugfs.c | 5 ++---
>  drivers/gpu/drm/msm/msm_drv.c     | 4 ++--
>  drivers/gpu/drm/msm/msm_drv.h     | 2 --
>  drivers/gpu/drm/msm/msm_fbdev.c   | 8 ++------
>  4 files changed, 6 insertions(+), 13 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

[PATCH 5/6] drm/msm: Initialize fbdev DRM client

[Thomas Zimmermann]

Initialize the fbdev client in the fbdev code with empty helper
functions. Also clean up the client. The helpers will later
implement various functionality of the DRM client. No functional
changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/msm/msm_fbdev.c | 38 +++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index 95b193a5e0d5..6c3665c5f4f6 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -119,6 +119,30 @@ static const struct drm_fb_helper_funcs msm_fb_helper_funcs = {
 	.fb_probe = msm_fbdev_create,
 };
 
+/*
+ * struct drm_client
+ */
+
+static void msm_fbdev_client_unregister(struct drm_client_dev *client)
+{ }
+
+static int msm_fbdev_client_restore(struct drm_client_dev *client)
+{
+	return 0;
+}
+
+static int msm_fbdev_client_hotplug(struct drm_client_dev *client)
+{
+	return 0;
+}
+
+static const struct drm_client_funcs msm_fbdev_client_funcs = {
+	.owner		= THIS_MODULE,
+	.unregister	= msm_fbdev_client_unregister,
+	.restore	= msm_fbdev_client_restore,
+	.hotplug	= msm_fbdev_client_hotplug,
+};
+
 /* initialize fbdev helper */
 struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 {
@@ -131,10 +155,16 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 
 	drm_fb_helper_prepare(dev, helper, 32, &msm_fb_helper_funcs);
 
+	ret = drm_client_init(dev, &helper->client, "fbdev", &msm_fbdev_client_funcs);
+	if (ret) {
+		drm_err(dev, "Failed to register client: %d\n", ret);
+		goto err_drm_fb_helper_unprepare;
+	}
+
 	ret = drm_fb_helper_init(dev, helper);
 	if (ret) {
 		DRM_DEV_ERROR(dev->dev, "could not init fbdev: ret=%d\n", ret);
-		goto fail;
+		goto err_drm_client_release;
 	}
 
 	ret = drm_fb_helper_initial_config(helper);
@@ -145,8 +175,11 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 
 fini:
 	drm_fb_helper_fini(helper);
-fail:
+err_drm_client_release:
+	drm_client_release(&helper->client);
+err_drm_fb_helper_unprepare:
 	drm_fb_helper_unprepare(helper);
+	kfree(helper);
 	return NULL;
 }
 
@@ -168,6 +201,7 @@ void msm_fbdev_free(struct drm_device *dev)
 		drm_framebuffer_remove(fb);
 	}
 
+	drm_client_release(&helper->client);
 	drm_fb_helper_unprepare(helper);
 	kfree(helper);
 
-- 
2.40.0

Re: [PATCH 5/6] drm/msm: Initialize fbdev DRM client

[Dmitry Baryshkov]

On Thu, 30 Mar 2023 at 10:41, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Initialize the fbdev client in the fbdev code with empty helper
> functions. Also clean up the client. The helpers will later
> implement various functionality of the DRM client. No functional
> changes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/msm/msm_fbdev.c | 38 +++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)

With the nit below fixed:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>
> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
> index 95b193a5e0d5..6c3665c5f4f6 100644
> --- a/drivers/gpu/drm/msm/msm_fbdev.c
> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
> @@ -119,6 +119,30 @@ static const struct drm_fb_helper_funcs msm_fb_helper_funcs = {
>         .fb_probe = msm_fbdev_create,
>  };
>
> +/*
> + * struct drm_client
> + */
> +
> +static void msm_fbdev_client_unregister(struct drm_client_dev *client)
> +{ }
> +
> +static int msm_fbdev_client_restore(struct drm_client_dev *client)
> +{
> +       return 0;
> +}
> +
> +static int msm_fbdev_client_hotplug(struct drm_client_dev *client)
> +{
> +       return 0;
> +}
> +
> +static const struct drm_client_funcs msm_fbdev_client_funcs = {
> +       .owner          = THIS_MODULE,
> +       .unregister     = msm_fbdev_client_unregister,
> +       .restore        = msm_fbdev_client_restore,
> +       .hotplug        = msm_fbdev_client_hotplug,
> +};
> +
>  /* initialize fbdev helper */
>  struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
>  {
> @@ -131,10 +155,16 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
>
>         drm_fb_helper_prepare(dev, helper, 32, &msm_fb_helper_funcs);
>
> +       ret = drm_client_init(dev, &helper->client, "fbdev", &msm_fbdev_client_funcs);
> +       if (ret) {
> +               drm_err(dev, "Failed to register client: %d\n", ret);
> +               goto err_drm_fb_helper_unprepare;
> +       }
> +
>         ret = drm_fb_helper_init(dev, helper);
>         if (ret) {
>                 DRM_DEV_ERROR(dev->dev, "could not init fbdev: ret=%d\n", ret);
> -               goto fail;
> +               goto err_drm_client_release;
>         }
>
>         ret = drm_fb_helper_initial_config(helper);
> @@ -145,8 +175,11 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
>
>  fini:
>         drm_fb_helper_fini(helper);
> -fail:
> +err_drm_client_release:
> +       drm_client_release(&helper->client);
> +err_drm_fb_helper_unprepare:
>         drm_fb_helper_unprepare(helper);
> +       kfree(helper);

This one should go to the patch 3

>         return NULL;
>  }
>
> @@ -168,6 +201,7 @@ void msm_fbdev_free(struct drm_device *dev)
>                 drm_framebuffer_remove(fb);
>         }
>
> +       drm_client_release(&helper->client);
>         drm_fb_helper_unprepare(helper);
>         kfree(helper);
>
> --
> 2.40.0
>


-- 
With best wishes
Dmitry

[PATCH 6/6] drm/msm: Implement fbdev emulation as in-kernel client

[Thomas Zimmermann]

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.

Replace all code that initializes or releases fbdev emulation
throughout the driver. Instead initialize the fbdev client by a
single call to msm_fbdev_setup() after msm has registered its
DRM device. As in most drivers, msm's fbdev emulation now acts
like a regular DRM client.

The fbdev client setup consists of the initial preparation and the
hot-plugging of the display. The latter creates the fbdev device
and sets up the fbdev framebuffer. The setup performs display
hot-plugging once. If no display can be detected, DRM probe helpers
re-run the detection on each hotplug event.

A call to drm_dev_unregister() releases the client automatically.
No further action is required within msm. If the fbdev framebuffer
has been fully set up, struct fb_ops.fb_destroy implements the
release. For partially initialized emulation, the fbdev client
reverts the initial setup.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/msm/msm_debugfs.c |   1 +
 drivers/gpu/drm/msm/msm_drv.c     |  15 +---
 drivers/gpu/drm/msm/msm_drv.h     |  10 ++-
 drivers/gpu/drm/msm/msm_fbdev.c   | 113 ++++++++++++++++++------------
 4 files changed, 80 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
index d4b31165708f..54f5c91a3e19 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -10,6 +10,7 @@
 #include <linux/fault-inject.h>
 
 #include <drm/drm_debugfs.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_file.h>
 #include <drm/drm_framebuffer.h>
 
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 9f076b9ab19b..339e3523ccb7 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -54,7 +54,6 @@
 
 static const struct drm_mode_config_funcs mode_config_funcs = {
 	.fb_create = msm_framebuffer_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -241,11 +240,6 @@ static int msm_drm_uninit(struct device *dev)
 	msm_perf_debugfs_cleanup(priv);
 	msm_rd_debugfs_cleanup(priv);
 
-#ifdef CONFIG_DRM_FBDEV_EMULATION
-	if (ddev->fb_helper)
-		msm_fbdev_free(ddev);
-#endif
-
 	msm_disp_snapshot_destroy(ddev);
 
 	drm_mode_config_cleanup(ddev);
@@ -535,17 +529,15 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
 	}
 	drm_mode_config_reset(ddev);
 
-#ifdef CONFIG_DRM_FBDEV_EMULATION
-	if (kms && fbdev)
-		msm_fbdev_init(ddev);
-#endif
-
 	ret = msm_debugfs_late_init(ddev);
 	if (ret)
 		goto err_msm_uninit;
 
 	drm_kms_helper_poll_init(ddev);
 
+	if (kms && fbdev)
+		msm_fbdev_setup(ddev);
+
 	return 0;
 
 err_msm_uninit:
@@ -1074,7 +1066,6 @@ static const struct drm_driver msm_driver = {
 				DRIVER_SYNCOBJ,
 	.open               = msm_open,
 	.postclose           = msm_postclose,
-	.lastclose          = drm_fb_helper_lastclose,
 	.dumb_create        = msm_gem_dumb_create,
 	.dumb_map_offset    = msm_gem_dumb_map_offset,
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 852f88c36621..11a9d2ce07a1 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -29,7 +29,6 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_fb_helper.h>
 #include <drm/display/drm_dsc.h>
 #include <drm/msm_drm.h>
 #include <drm/drm_gem.h>
@@ -304,8 +303,13 @@ struct drm_framebuffer *msm_framebuffer_create(struct drm_device *dev,
 struct drm_framebuffer * msm_alloc_stolen_fb(struct drm_device *dev,
 		int w, int h, int p, uint32_t format);
 
-struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev);
-void msm_fbdev_free(struct drm_device *dev);
+#ifdef CONFIG_DRM_FBDEV_EMULATION
+void msm_fbdev_setup(struct drm_device *dev);
+#else
+static inline void msm_fbdev_setup(struct drm_device *dev)
+{
+}
+#endif
 
 struct hdmi;
 #ifdef CONFIG_DRM_MSM_HDMI
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index 6c3665c5f4f6..33570ef504bf 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -4,7 +4,8 @@
  * Author: Rob Clark <robdclark@gmail.com>
  */
 
-#include <drm/drm_crtc.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
@@ -26,6 +27,25 @@ static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
 	return drm_gem_prime_mmap(bo, vma);
 }
 
+static void msm_fbdev_fb_destroy(struct fb_info *info)
+{
+	struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par;
+	struct drm_framebuffer *fb = helper->fb;
+	struct drm_gem_object *bo = msm_framebuffer_bo(fb, 0);
+
+	DBG();
+
+	drm_fb_helper_fini(helper);
+
+	/* this will free the backing object */
+	msm_gem_put_vaddr(bo);
+	drm_framebuffer_remove(fb);
+
+	drm_client_release(&helper->client);
+	drm_fb_helper_unprepare(helper);
+	kfree(helper);
+}
+
 static const struct fb_ops msm_fb_ops = {
 	.owner = THIS_MODULE,
 	DRM_FB_HELPER_DEFAULT_OPS,
@@ -39,6 +59,7 @@ static const struct fb_ops msm_fb_ops = {
 	.fb_copyarea = drm_fb_helper_sys_copyarea,
 	.fb_imageblit = drm_fb_helper_sys_imageblit,
 	.fb_mmap = msm_fbdev_mmap,
+	.fb_destroy = msm_fbdev_fb_destroy,
 };
 
 static int msm_fbdev_create(struct drm_fb_helper *helper,
@@ -124,16 +145,52 @@ static const struct drm_fb_helper_funcs msm_fb_helper_funcs = {
  */
 
 static void msm_fbdev_client_unregister(struct drm_client_dev *client)
-{ }
+{
+	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+
+	if (fb_helper->info) {
+		drm_fb_helper_unregister_info(fb_helper);
+	} else {
+		drm_client_release(&fb_helper->client);
+		drm_fb_helper_unprepare(fb_helper);
+		kfree(fb_helper);
+	}
+}
 
 static int msm_fbdev_client_restore(struct drm_client_dev *client)
 {
+	drm_fb_helper_lastclose(client->dev);
+
 	return 0;
 }
 
 static int msm_fbdev_client_hotplug(struct drm_client_dev *client)
 {
+	struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+	struct drm_device *dev = client->dev;
+	int ret;
+
+	if (dev->fb_helper)
+		return drm_fb_helper_hotplug_event(dev->fb_helper);
+
+	ret = drm_fb_helper_init(dev, fb_helper);
+	if (ret)
+		goto err_drm_err;
+
+	if (!drm_drv_uses_atomic_modeset(dev))
+		drm_helper_disable_unused_functions(dev);
+
+	ret = drm_fb_helper_initial_config(fb_helper);
+	if (ret)
+		goto err_drm_fb_helper_fini;
+
 	return 0;
+
+err_drm_fb_helper_fini:
+	drm_fb_helper_fini(fb_helper);
+err_drm_err:
+	drm_err(dev, "Failed to setup fbdev emulation (ret=%d)\n", ret);
+	return ret;
 }
 
 static const struct drm_client_funcs msm_fbdev_client_funcs = {
@@ -144,15 +201,17 @@ static const struct drm_client_funcs msm_fbdev_client_funcs = {
 };
 
 /* initialize fbdev helper */
-struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
+void msm_fbdev_setup(struct drm_device *dev)
 {
 	struct drm_fb_helper *helper;
 	int ret;
 
+	drm_WARN(dev, !dev->registered, "Device has not been registered.\n");
+	drm_WARN(dev, dev->fb_helper, "fb_helper is already set!\n");
+
 	helper = kzalloc(sizeof(*helper), GFP_KERNEL);
 	if (!helper)
-		return NULL;
-
+		return;
 	drm_fb_helper_prepare(dev, helper, 32, &msm_fb_helper_funcs);
 
 	ret = drm_client_init(dev, &helper->client, "fbdev", &msm_fbdev_client_funcs);
@@ -161,49 +220,15 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 		goto err_drm_fb_helper_unprepare;
 	}
 
-	ret = drm_fb_helper_init(dev, helper);
-	if (ret) {
-		DRM_DEV_ERROR(dev->dev, "could not init fbdev: ret=%d\n", ret);
-		goto err_drm_client_release;
-	}
-
-	ret = drm_fb_helper_initial_config(helper);
+	ret = msm_fbdev_client_hotplug(&helper->client);
 	if (ret)
-		goto fini;
+		drm_dbg_kms(dev, "client hotplug ret=%d\n", ret);
 
-	return helper;
+	drm_client_register(&helper->client);
 
-fini:
-	drm_fb_helper_fini(helper);
-err_drm_client_release:
-	drm_client_release(&helper->client);
-err_drm_fb_helper_unprepare:
-	drm_fb_helper_unprepare(helper);
-	kfree(helper);
-	return NULL;
-}
+	return;
 
-void msm_fbdev_free(struct drm_device *dev)
-{
-	struct drm_fb_helper *helper = dev->fb_helper;
-	struct drm_framebuffer *fb = helper->fb;
-
-	DBG();
-
-	drm_fb_helper_unregister_info(helper);
-
-	drm_fb_helper_fini(helper);
-
-	/* this will free the backing object */
-	if (fb) {
-		struct drm_gem_object *bo = msm_framebuffer_bo(fb, 0);
-		msm_gem_put_vaddr(bo);
-		drm_framebuffer_remove(fb);
-	}
-
-	drm_client_release(&helper->client);
+err_drm_fb_helper_unprepare:
 	drm_fb_helper_unprepare(helper);
 	kfree(helper);
-
-	dev->fb_helper = NULL;
 }
-- 
2.40.0

Re: [PATCH 6/6] drm/msm: Implement fbdev emulation as in-kernel client

[kernel test robot]

Hi Thomas,

I love your patch! Yet something to improve:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.3-rc4]
[cannot apply to next-20230330]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/drm-msm-Clear-aperture-ownership-outside-of-fbdev-code/20230330-154729
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20230330074150.7637-7-tzimmermann%40suse.de
patch subject: [PATCH 6/6] drm/msm: Implement fbdev emulation as in-kernel client
config: csky-randconfig-r011-20230329 (https://download.01.org/0day-ci/archive/20230330/202303301849.HjMnKXNi-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ec39cb11cf72fb01ada6fe51c7c572a31dcc805d
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Thomas-Zimmermann/drm-msm-Clear-aperture-ownership-outside-of-fbdev-code/20230330-154729
        git checkout ec39cb11cf72fb01ada6fe51c7c572a31dcc805d
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=csky olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=csky SHELL=/bin/bash drivers/gpu/drm/msm/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303301849.HjMnKXNi-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpu/drm/msm/msm_drv.c: In function 'msm_drm_init':
>> drivers/gpu/drm/msm/msm_drv.c:538:20: error: 'fbdev' undeclared (first use in this function)
     538 |         if (kms && fbdev)
         |                    ^~~~~
   drivers/gpu/drm/msm/msm_drv.c:538:20: note: each undeclared identifier is reported only once for each function it appears in


vim +/fbdev +538 drivers/gpu/drm/msm/msm_drv.c

   398	
   399	static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
   400	{
   401		struct msm_drm_private *priv = dev_get_drvdata(dev);
   402		struct drm_device *ddev;
   403		struct msm_kms *kms;
   404		int ret, i;
   405	
   406		if (drm_firmware_drivers_only())
   407			return -ENODEV;
   408	
   409		/* the fw fb could be anywhere in memory */
   410		ret = drm_aperture_remove_framebuffers(false, drv);
   411		if (ret)
   412			return ret;
   413	
   414		ddev = drm_dev_alloc(drv, dev);
   415		if (IS_ERR(ddev)) {
   416			DRM_DEV_ERROR(dev, "failed to allocate drm_device\n");
   417			return PTR_ERR(ddev);
   418		}
   419		ddev->dev_private = priv;
   420		priv->dev = ddev;
   421	
   422		priv->wq = alloc_ordered_workqueue("msm", 0);
   423		if (!priv->wq)
   424			return -ENOMEM;
   425	
   426		INIT_LIST_HEAD(&priv->objects);
   427		mutex_init(&priv->obj_lock);
   428	
   429		/*
   430		 * Initialize the LRUs:
   431		 */
   432		mutex_init(&priv->lru.lock);
   433		drm_gem_lru_init(&priv->lru.unbacked, &priv->lru.lock);
   434		drm_gem_lru_init(&priv->lru.pinned,   &priv->lru.lock);
   435		drm_gem_lru_init(&priv->lru.willneed, &priv->lru.lock);
   436		drm_gem_lru_init(&priv->lru.dontneed, &priv->lru.lock);
   437	
   438		/* Teach lockdep about lock ordering wrt. shrinker: */
   439		fs_reclaim_acquire(GFP_KERNEL);
   440		might_lock(&priv->lru.lock);
   441		fs_reclaim_release(GFP_KERNEL);
   442	
   443		drm_mode_config_init(ddev);
   444	
   445		ret = msm_init_vram(ddev);
   446		if (ret)
   447			goto err_drm_dev_put;
   448	
   449		/* Bind all our sub-components: */
   450		ret = component_bind_all(dev, ddev);
   451		if (ret)
   452			goto err_drm_dev_put;
   453	
   454		dma_set_max_seg_size(dev, UINT_MAX);
   455	
   456		msm_gem_shrinker_init(ddev);
   457	
   458		if (priv->kms_init) {
   459			ret = priv->kms_init(ddev);
   460			if (ret) {
   461				DRM_DEV_ERROR(dev, "failed to load kms\n");
   462				priv->kms = NULL;
   463				goto err_msm_uninit;
   464			}
   465			kms = priv->kms;
   466		} else {
   467			/* valid only for the dummy headless case, where of_node=NULL */
   468			WARN_ON(dev->of_node);
   469			kms = NULL;
   470		}
   471	
   472		/* Enable normalization of plane zpos */
   473		ddev->mode_config.normalize_zpos = true;
   474	
   475		if (kms) {
   476			kms->dev = ddev;
   477			ret = kms->funcs->hw_init(kms);
   478			if (ret) {
   479				DRM_DEV_ERROR(dev, "kms hw init failed: %d\n", ret);
   480				goto err_msm_uninit;
   481			}
   482		}
   483	
   484		drm_helper_move_panel_connectors_to_head(ddev);
   485	
   486		ddev->mode_config.funcs = &mode_config_funcs;
   487		ddev->mode_config.helper_private = &mode_config_helper_funcs;
   488	
   489		for (i = 0; i < priv->num_crtcs; i++) {
   490			/* initialize event thread */
   491			priv->event_thread[i].crtc_id = priv->crtcs[i]->base.id;
   492			priv->event_thread[i].dev = ddev;
   493			priv->event_thread[i].worker = kthread_create_worker(0,
   494				"crtc_event:%d", priv->event_thread[i].crtc_id);
   495			if (IS_ERR(priv->event_thread[i].worker)) {
   496				ret = PTR_ERR(priv->event_thread[i].worker);
   497				DRM_DEV_ERROR(dev, "failed to create crtc_event kthread\n");
   498				priv->event_thread[i].worker = NULL;
   499				goto err_msm_uninit;
   500			}
   501	
   502			sched_set_fifo(priv->event_thread[i].worker->task);
   503		}
   504	
   505		ret = drm_vblank_init(ddev, priv->num_crtcs);
   506		if (ret < 0) {
   507			DRM_DEV_ERROR(dev, "failed to initialize vblank\n");
   508			goto err_msm_uninit;
   509		}
   510	
   511		if (kms) {
   512			pm_runtime_get_sync(dev);
   513			ret = msm_irq_install(ddev, kms->irq);
   514			pm_runtime_put_sync(dev);
   515			if (ret < 0) {
   516				DRM_DEV_ERROR(dev, "failed to install IRQ handler\n");
   517				goto err_msm_uninit;
   518			}
   519		}
   520	
   521		ret = drm_dev_register(ddev, 0);
   522		if (ret)
   523			goto err_msm_uninit;
   524	
   525		if (kms) {
   526			ret = msm_disp_snapshot_init(ddev);
   527			if (ret)
   528				DRM_DEV_ERROR(dev, "msm_disp_snapshot_init failed ret = %d\n", ret);
   529		}
   530		drm_mode_config_reset(ddev);
   531	
   532		ret = msm_debugfs_late_init(ddev);
   533		if (ret)
   534			goto err_msm_uninit;
   535	
   536		drm_kms_helper_poll_init(ddev);
   537	
 > 538		if (kms && fbdev)
   539			msm_fbdev_setup(ddev);
   540	
   541		return 0;
   542	
   543	err_msm_uninit:
   544		msm_drm_uninit(dev);
   545	err_drm_dev_put:
   546		drm_dev_put(ddev);
   547		return ret;
   548	}
   549	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 6/6] drm/msm: Implement fbdev emulation as in-kernel client

[kernel test robot]

Hi Thomas,

I love your patch! Yet something to improve:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.3-rc4]
[cannot apply to next-20230330]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/drm-msm-Clear-aperture-ownership-outside-of-fbdev-code/20230330-154729
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20230330074150.7637-7-tzimmermann%40suse.de
patch subject: [PATCH 6/6] drm/msm: Implement fbdev emulation as in-kernel client
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230330/202303301856.zSmpwZjj-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ec39cb11cf72fb01ada6fe51c7c572a31dcc805d
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Thomas-Zimmermann/drm-msm-Clear-aperture-ownership-outside-of-fbdev-code/20230330-154729
        git checkout ec39cb11cf72fb01ada6fe51c7c572a31dcc805d
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/gpu/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303301856.zSmpwZjj-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   drivers/gpu/drm/msm/msm_io_utils.c: In function '_msm_ioremap':
>> drivers/gpu/drm/msm/msm_io_utils.c:72:15: error: implicit declaration of function 'devm_ioremap'; did you mean '_msm_ioremap'? [-Werror=implicit-function-declaration]
      72 |         ptr = devm_ioremap(&pdev->dev, res->start, size);
         |               ^~~~~~~~~~~~
         |               _msm_ioremap
>> drivers/gpu/drm/msm/msm_io_utils.c:72:13: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
      72 |         ptr = devm_ioremap(&pdev->dev, res->start, size);
         |             ^
   cc1: some warnings being treated as errors


vim +72 drivers/gpu/drm/msm/msm_io_utils.c

d89e5028346bd80 Dmitry Baryshkov 2022-01-20  51  
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  52  static void __iomem *_msm_ioremap(struct platform_device *pdev, const char *name,
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  53  				  bool quiet, phys_addr_t *psize)
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  54  {
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  55  	struct resource *res;
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  56  	unsigned long size;
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  57  	void __iomem *ptr;
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  58  
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  59  	if (name)
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  60  		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  61  	else
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  62  		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  63  
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  64  	if (!res) {
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  65  		if (!quiet)
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  66  			DRM_DEV_ERROR(&pdev->dev, "failed to get memory resource: %s\n", name);
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  67  		return ERR_PTR(-EINVAL);
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  68  	}
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  69  
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  70  	size = resource_size(res);
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  71  
d89e5028346bd80 Dmitry Baryshkov 2022-01-20 @72  	ptr = devm_ioremap(&pdev->dev, res->start, size);
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  73  	if (!ptr) {
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  74  		if (!quiet)
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  75  			DRM_DEV_ERROR(&pdev->dev, "failed to ioremap: %s\n", name);
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  76  		return ERR_PTR(-ENOMEM);
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  77  	}
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  78  
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  79  	if (psize)
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  80  		*psize = size;
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  81  
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  82  	return ptr;
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  83  }
d89e5028346bd80 Dmitry Baryshkov 2022-01-20  84  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 0/6] drm/msm: Convert fbdev to DRM client

[Dmitry Baryshkov]

On Thu, 30 Mar 2023 at 10:41, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Convert msm' fbdev code to struct drm_client. Replaces the current
> ad-hoc integration. The conversion includes a number of cleanups. As
> with most other drivers' fbdev emulation, fbdev in msm is now just
> another DRM client that runs after the DRM device has been registered.
>
> Once all drivers' fbdev emulation has been converted to struct drm_client,
> we can attempt to add additional in-kernel clients. A DRM-based dmesg
> log or a bootsplash are commonly mentioned. DRM can then switch easily
> among the existing clients if/when required.
>
> I did the conversion from similar experience with other drivers. But I
> don't have the hardware to test this. Any testing is welcome.

Thank you for your patches! It was on my to do list for quite a while,
but nobody had time to work on it.

>
> Thomas Zimmermann (6):
>   drm/msm: Clear aperture ownership outside of fbdev code
>   drm/msm: Remove fb from struct msm_fbdev
>   drm/msm: Remove struct msm_fbdev
>   drm/msm: Remove fbdev from struct msm_drm_private
>   drm/msm: Initialize fbdev DRM client
>   drm/msm: Implement fbdev emulation as in-kernel client
>
>  drivers/gpu/drm/msm/msm_debugfs.c |   6 +-
>  drivers/gpu/drm/msm/msm_drv.c     |  21 ++--
>  drivers/gpu/drm/msm/msm_drv.h     |  12 ++-
>  drivers/gpu/drm/msm/msm_fbdev.c   | 168 ++++++++++++++++++------------
>  4 files changed, 118 insertions(+), 89 deletions(-)
>
> --
> 2.40.0
>


-- 
With best wishes
Dmitry