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

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- 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 --]