Message-ID: <CAMpQs4+_5v23z3bkT4ykPE_PkgZYUTGKM82xcGnyj4US4O4TCw@mail.gmail.com>
Date: Thu, 9 Jan 2025 20:56:01 +0800
From: Binbin Zhou <zhoubb.aaron@...il.com>
To: Thomas Zimmermann <tzimmermann@...e.de>
Cc: Binbin Zhou <zhoubinbin@...ngson.cn>, Huacai Chen <chenhuacai@...ngson.cn>, 
	Lee Jones <lee@...nel.org>, Corey Minyard <minyard@....org>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Huacai Chen <chenhuacai@...nel.org>, 
	linux-kernel@...r.kernel.org, openipmi-developer@...ts.sourceforge.net, 
	dri-devel@...ts.freedesktop.org, Xuerui Wang <kernel@...0n.name>, 
	loongarch@...ts.linux.dev, Chong Qiao <qiaochong@...ngson.cn>
Subject: Re: [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display

Hi Thomas:

Sorry for the late reply.

On Mon, Jan 6, 2025 at 10:10 PM Thomas Zimmermann <tzimmermann@...e.de> wrote:
>
> Hi,
>
> Thanks for the info.
>
>
> Am 06.01.25 um 08:03 schrieb Binbin Zhou:
> [...]
> >> Could you point to the exact call that fails within simpledrm?
> > If we use simpledrm directly, the following error occurs:
> >
> > [    8.289823] simple-framebuffer simple-framebuffer.0: [drm] *ERROR*
> > could not acquire memory range [mem 0xe0031200000-0xe00315fffff flags
> > 0x200]: -16
> > [    8.312681] simple-framebuffer simple-framebuffer.0: probe with
> > driver simple-framebuffer failed with error -16
> >
> > The reason for the failure: overlapping resources.
> >
> > https://elixir.bootlin.com/linux/v6.12.6/source/drivers/video/aperture.c#L175
>
> This error means that there's already an instance of simpledrm bound to
> the BMC framebuffer. So you already have a working display and some
> graphics under Linux without the new driver, right?

Yes, I checked again and found that the **efifb** binds to the BMC framebuffer.

>
> If so, why do you need a new driver that does exactly the same as simpledrm?

Regarding the new driver, we implemented the BMC display based on
simpledrm. But first we need to fix the initialization failure above,
and more importantly, we need to implement the BMC reset function [1].

Specifically, when the BMC reset, it will cause the pcie controller to
disconnect and the display will be disconnected with it. After that,
we need to restore the pcie bar data, as well as re-push the display
information (ls2kbmc_push_drm_mode()).

Based on this, I would like to rewrite a new display driver.

[1]: patch(4/4)
https://lore.kernel.org/all/b0ac8b81fbb8955ed8ada27aba423cff45aad9f6.1735550269.git.zhoubinbin@loongson.cn/
>
> Best regards
> Thomas
>
> >>> Because although we register the driver in platform form, its memory
> >>> belongs to pci space and we can see the corresponding pci probe and
> >>> resource allocation in Patch-1.
> >> I don't understand. Graphics memory is often located on the PCI bus.
> >> What is so special about this one?
> >>
> >>> Therefore, we need to use aperture_remove_conflicting_pci_devices().
> >> So there is already a device that represents the graphics card? That's
> >> what you'd remove here? If you only add that MFD device, who owns the
> >> framebuffer? If it's the PCI device from patch 1 ("ls2k-bmc"), why does
> >> aperture_remove_conflicting_pci_devices() not remove that device? I'm
> >> somewhat confused, because the logic in your driver mostly looks like it
> >> binds to a pre-configured framebuffer, but some of the code doesn't.
> > Perhaps the use of aperture_remove_conflicting_pci_devices() is wrong,
> > as there is only one display device for the LS2K BMC and there will be
> > no phase conflict.
> >
> > When I tried to use that API before, it was partly due to the error
> > above, and partly because I referenced that other display drivers via
> > pci_driver.probe() would have it, just in case I used it, which was
> > probably the wrong choice.
> >
> > The resources for pci bar0 are as follows:
> > BAR0: e0030000000/SZ_32M
> >
> > 0x0              0x600000  0xf00001c    16M            32M
> > +----+--------------+--------+-----------+---+-----------------+
> > | 2M | simpldrm |           | IPMI      |     | video env     |
> > +-----------------------------------------------------------------+
> >
> > The mfd driver registers the ls2kbmc-framebuffer and ls2k-ipmi-si
> > devices according to the resource allocation shown above. At the same
> > time, the ls2kbmc drm is bound to the pre-configured “simpldrm”
> > resource in the above figure, which is passed through the
> > ls2kbmc-framebuffer driver. In addition, the resolution is read from
> > “video env” for the time being, and the resolution adaption is planned
> > to be added later.
> >
> >> Best regards Thomas
> >>
> >>> Also, since we are using BMC display, the display will be disconnected
> >>> when BMC reset, at this time we need to push the display data (crtc,
> >>> connector, etc.) manually as shown in Patch-4.
> >>>
> >>> Probably it's not the most suitable way to implement it.
> >>>
> >>>> Best regards
> >>>> Thomas
> >>>>
> >>>>> +     },
> >>>>> +     .probe = ls2kbmc_probe,
> >>>>> +     .remove = ls2kbmc_remove,
> >>>>> +};
> >>>>> +
> >>>>> +module_platform_driver(ls2kbmc_platform_driver);
> >>>>> +
> >>>>> +MODULE_DESCRIPTION("DRM driver for Loongson-2K BMC");
> >>>>> +MODULE_LICENSE("GPL");
> >>>> --
> >>>> --
> >>>> 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)
> >>>>
> >> --
> >> --
> >> 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)
> >>
> >
> > --
> > Thanks.
> > Binbin
>
> --
> --
> 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)
>


-- 
Thanks.
Binbin

Powered by blists - more mailing lists