| Message ID | 20200415074034.175360-54-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | devm_drm_dev_alloc, v2 | expand |
Hi Am 15.04.20 um 09:40 schrieb Daniel Vetter: > Because it is. Yes. OTOH, as much as I appreciate the simplification, I think it should be in a separate series. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Alexey Brodkin <abrodkin@synopsys.com> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > MAINTAINERS | 2 +- > drivers/gpu/drm/Kconfig | 2 -- > drivers/gpu/drm/Makefile | 1 - > drivers/gpu/drm/arc/Kconfig | 10 ---------- > drivers/gpu/drm/arc/Makefile | 3 --- > drivers/gpu/drm/tiny/Kconfig | 10 ++++++++++ > drivers/gpu/drm/tiny/Makefile | 1 + > drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} | 0 > 8 files changed, 12 insertions(+), 17 deletions(-) > delete mode 100644 drivers/gpu/drm/arc/Kconfig > delete mode 100644 drivers/gpu/drm/arc/Makefile > rename drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} (100%) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 0a5cf105ee37..748244b1625b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1295,7 +1295,7 @@ ARC PGU DRM DRIVER > M: Alexey Brodkin <abrodkin@synopsys.com> > S: Supported > F: Documentation/devicetree/bindings/display/snps,arcpgu.txt > -F: drivers/gpu/drm/arc/ > +F: drivers/gpu/drm/tiny/arcpgu.c > > ARCNET NETWORK LAYER > M: Michael Grzeschik <m.grzeschik@pengutronix.de> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 4f4e7fa001c1..a0a89025d6fa 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -354,8 +354,6 @@ source "drivers/gpu/drm/vc4/Kconfig" > > source "drivers/gpu/drm/etnaviv/Kconfig" > > -source "drivers/gpu/drm/arc/Kconfig" > - > source "drivers/gpu/drm/hisilicon/Kconfig" > > source "drivers/gpu/drm/mediatek/Kconfig" > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 2c0e5a7e5953..e69eafbf9e39 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -109,7 +109,6 @@ obj-y += panel/ > obj-y += bridge/ > obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/ > obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/ > -obj-$(CONFIG_DRM_ARCPGU)+= arc/ > obj-y += hisilicon/ > obj-$(CONFIG_DRM_ZTE) += zte/ > obj-$(CONFIG_DRM_MXSFB) += mxsfb/ > diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig > deleted file mode 100644 > index e8f3d63e0b91..000000000000 > --- a/drivers/gpu/drm/arc/Kconfig > +++ /dev/null > @@ -1,10 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0-only > -config DRM_ARCPGU > - tristate "ARC PGU" > - depends on DRM && OF > - select DRM_KMS_CMA_HELPER > - select DRM_KMS_HELPER > - help > - Choose this option if you have an ARC PGU controller. > - > - If M is selected the module will be called arcpgu. > diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile > deleted file mode 100644 > index b26f2495c532..000000000000 > --- a/drivers/gpu/drm/arc/Makefile > +++ /dev/null > @@ -1,3 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0-only > -arcpgu-y := arcpgu_drv.o > -obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o > diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig > index 2b6414f0fa75..9bbaa1a69050 100644 > --- a/drivers/gpu/drm/tiny/Kconfig > +++ b/drivers/gpu/drm/tiny/Kconfig > @@ -1,5 +1,15 @@ > # SPDX-License-Identifier: GPL-2.0-only > > +config DRM_ARCPGU > + tristate "ARC PGU" > + depends on DRM && OF > + select DRM_KMS_CMA_HELPER > + select DRM_KMS_HELPER > + help > + Choose this option if you have an ARC PGU controller. > + > + If M is selected the module will be called arcpgu. > + > config DRM_CIRRUS_QEMU > tristate "Cirrus driver for QEMU emulated device" > depends on DRM && PCI && MMU > diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile > index 6ae4e9e5a35f..bef6780bdd6f 100644 > --- a/drivers/gpu/drm/tiny/Makefile > +++ b/drivers/gpu/drm/tiny/Makefile > @@ -1,5 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0-only > > +obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o > obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o > obj-$(CONFIG_DRM_GM12U320) += gm12u320.o > obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o > diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/tiny/arcpgu.c > similarity index 100% > rename from drivers/gpu/drm/arc/arcpgu_drv.c > rename to drivers/gpu/drm/tiny/arcpgu.c >
On Wed, Apr 15, 2020 at 10:04 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Hi > > Am 15.04.20 um 09:40 schrieb Daniel Vetter: > > Because it is. > > Yes. > > OTOH, as much as I appreciate the simplification, I think it should be > in a separate series. Right now they all still need to be here because of the devm_drm_dev_alloc patch 1 adds. After that I guess I can split up, but it's kinda more work. The series is really "clean up driver load/unload code around drm_device", and for some of the older drivers there's a _lot_ that can be done. -Daniel > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Alexey Brodkin <abrodkin@synopsys.com> > > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > > > --- > > MAINTAINERS | 2 +- > > drivers/gpu/drm/Kconfig | 2 -- > > drivers/gpu/drm/Makefile | 1 - > > drivers/gpu/drm/arc/Kconfig | 10 ---------- > > drivers/gpu/drm/arc/Makefile | 3 --- > > drivers/gpu/drm/tiny/Kconfig | 10 ++++++++++ > > drivers/gpu/drm/tiny/Makefile | 1 + > > drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} | 0 > > 8 files changed, 12 insertions(+), 17 deletions(-) > > delete mode 100644 drivers/gpu/drm/arc/Kconfig > > delete mode 100644 drivers/gpu/drm/arc/Makefile > > rename drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} (100%) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 0a5cf105ee37..748244b1625b 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -1295,7 +1295,7 @@ ARC PGU DRM DRIVER > > M: Alexey Brodkin <abrodkin@synopsys.com> > > S: Supported > > F: Documentation/devicetree/bindings/display/snps,arcpgu.txt > > -F: drivers/gpu/drm/arc/ > > +F: drivers/gpu/drm/tiny/arcpgu.c > > > > ARCNET NETWORK LAYER > > M: Michael Grzeschik <m.grzeschik@pengutronix.de> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > > index 4f4e7fa001c1..a0a89025d6fa 100644 > > --- a/drivers/gpu/drm/Kconfig > > +++ b/drivers/gpu/drm/Kconfig > > @@ -354,8 +354,6 @@ source "drivers/gpu/drm/vc4/Kconfig" > > > > source "drivers/gpu/drm/etnaviv/Kconfig" > > > > -source "drivers/gpu/drm/arc/Kconfig" > > - > > source "drivers/gpu/drm/hisilicon/Kconfig" > > > > source "drivers/gpu/drm/mediatek/Kconfig" > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > > index 2c0e5a7e5953..e69eafbf9e39 100644 > > --- a/drivers/gpu/drm/Makefile > > +++ b/drivers/gpu/drm/Makefile > > @@ -109,7 +109,6 @@ obj-y += panel/ > > obj-y += bridge/ > > obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/ > > obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/ > > -obj-$(CONFIG_DRM_ARCPGU)+= arc/ > > obj-y += hisilicon/ > > obj-$(CONFIG_DRM_ZTE) += zte/ > > obj-$(CONFIG_DRM_MXSFB) += mxsfb/ > > diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig > > deleted file mode 100644 > > index e8f3d63e0b91..000000000000 > > --- a/drivers/gpu/drm/arc/Kconfig > > +++ /dev/null > > @@ -1,10 +0,0 @@ > > -# SPDX-License-Identifier: GPL-2.0-only > > -config DRM_ARCPGU > > - tristate "ARC PGU" > > - depends on DRM && OF > > - select DRM_KMS_CMA_HELPER > > - select DRM_KMS_HELPER > > - help > > - Choose this option if you have an ARC PGU controller. > > - > > - If M is selected the module will be called arcpgu. > > diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile > > deleted file mode 100644 > > index b26f2495c532..000000000000 > > --- a/drivers/gpu/drm/arc/Makefile > > +++ /dev/null > > @@ -1,3 +0,0 @@ > > -# SPDX-License-Identifier: GPL-2.0-only > > -arcpgu-y := arcpgu_drv.o > > -obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o > > diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig > > index 2b6414f0fa75..9bbaa1a69050 100644 > > --- a/drivers/gpu/drm/tiny/Kconfig > > +++ b/drivers/gpu/drm/tiny/Kconfig > > @@ -1,5 +1,15 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > > > +config DRM_ARCPGU > > + tristate "ARC PGU" > > + depends on DRM && OF > > + select DRM_KMS_CMA_HELPER > > + select DRM_KMS_HELPER > > + help > > + Choose this option if you have an ARC PGU controller. > > + > > + If M is selected the module will be called arcpgu. > > + > > config DRM_CIRRUS_QEMU > > tristate "Cirrus driver for QEMU emulated device" > > depends on DRM && PCI && MMU > > diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile > > index 6ae4e9e5a35f..bef6780bdd6f 100644 > > --- a/drivers/gpu/drm/tiny/Makefile > > +++ b/drivers/gpu/drm/tiny/Makefile > > @@ -1,5 +1,6 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > > > +obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o > > obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o > > obj-$(CONFIG_DRM_GM12U320) += gm12u320.o > > obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o > > diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/tiny/arcpgu.c > > similarity index 100% > > rename from drivers/gpu/drm/arc/arcpgu_drv.c > > rename to drivers/gpu/drm/tiny/arcpgu.c > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer >
Hi Daniel. On Wed, Apr 15, 2020 at 09:40:28AM +0200, Daniel Vetter wrote: > Because it is. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Alexey Brodkin <abrodkin@synopsys.com> > --- > MAINTAINERS | 2 +- > drivers/gpu/drm/Kconfig | 2 -- > drivers/gpu/drm/Makefile | 1 - > drivers/gpu/drm/arc/Kconfig | 10 ---------- > drivers/gpu/drm/arc/Makefile | 3 --- > drivers/gpu/drm/tiny/Kconfig | 10 ++++++++++ > drivers/gpu/drm/tiny/Makefile | 1 + > drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} | 0 > 8 files changed, 12 insertions(+), 17 deletions(-) > delete mode 100644 drivers/gpu/drm/arc/Kconfig > delete mode 100644 drivers/gpu/drm/arc/Makefile > rename drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} (100%) We have "DRM: ARC: add HDMI 2.0 TX encoder support" which adds another platform driver to drm/arc/ This speaks against the move to tiny IMO Sam > > diff --git a/MAINTAINERS b/MAINTAINERS > index 0a5cf105ee37..748244b1625b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1295,7 +1295,7 @@ ARC PGU DRM DRIVER > M: Alexey Brodkin <abrodkin@synopsys.com> > S: Supported > F: Documentation/devicetree/bindings/display/snps,arcpgu.txt > -F: drivers/gpu/drm/arc/ > +F: drivers/gpu/drm/tiny/arcpgu.c > > ARCNET NETWORK LAYER > M: Michael Grzeschik <m.grzeschik@pengutronix.de> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 4f4e7fa001c1..a0a89025d6fa 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -354,8 +354,6 @@ source "drivers/gpu/drm/vc4/Kconfig" > > source "drivers/gpu/drm/etnaviv/Kconfig" > > -source "drivers/gpu/drm/arc/Kconfig" > - > source "drivers/gpu/drm/hisilicon/Kconfig" > > source "drivers/gpu/drm/mediatek/Kconfig" > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 2c0e5a7e5953..e69eafbf9e39 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -109,7 +109,6 @@ obj-y += panel/ > obj-y += bridge/ > obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/ > obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/ > -obj-$(CONFIG_DRM_ARCPGU)+= arc/ > obj-y += hisilicon/ > obj-$(CONFIG_DRM_ZTE) += zte/ > obj-$(CONFIG_DRM_MXSFB) += mxsfb/ > diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig > deleted file mode 100644 > index e8f3d63e0b91..000000000000 > --- a/drivers/gpu/drm/arc/Kconfig > +++ /dev/null > @@ -1,10 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0-only > -config DRM_ARCPGU > - tristate "ARC PGU" > - depends on DRM && OF > - select DRM_KMS_CMA_HELPER > - select DRM_KMS_HELPER > - help > - Choose this option if you have an ARC PGU controller. > - > - If M is selected the module will be called arcpgu. > diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile > deleted file mode 100644 > index b26f2495c532..000000000000 > --- a/drivers/gpu/drm/arc/Makefile > +++ /dev/null > @@ -1,3 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0-only > -arcpgu-y := arcpgu_drv.o > -obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o > diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig > index 2b6414f0fa75..9bbaa1a69050 100644 > --- a/drivers/gpu/drm/tiny/Kconfig > +++ b/drivers/gpu/drm/tiny/Kconfig > @@ -1,5 +1,15 @@ > # SPDX-License-Identifier: GPL-2.0-only > > +config DRM_ARCPGU > + tristate "ARC PGU" > + depends on DRM && OF > + select DRM_KMS_CMA_HELPER > + select DRM_KMS_HELPER > + help > + Choose this option if you have an ARC PGU controller. > + > + If M is selected the module will be called arcpgu. > + > config DRM_CIRRUS_QEMU > tristate "Cirrus driver for QEMU emulated device" > depends on DRM && PCI && MMU > diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile > index 6ae4e9e5a35f..bef6780bdd6f 100644 > --- a/drivers/gpu/drm/tiny/Makefile > +++ b/drivers/gpu/drm/tiny/Makefile > @@ -1,5 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0-only > > +obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o > obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o > obj-$(CONFIG_DRM_GM12U320) += gm12u320.o > obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o > diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/tiny/arcpgu.c > similarity index 100% > rename from drivers/gpu/drm/arc/arcpgu_drv.c > rename to drivers/gpu/drm/tiny/arcpgu.c > -- > 2.25.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Daniel, > -----Original Message----- > From: Sam Ravnborg <sam@ravnborg.org> > Sent: Wednesday, April 15, 2020 12:45 PM > To: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>; Alexey Brodkin > <abrodkin@synopsys.com>; DRI Development <dri-devel@lists.freedesktop.org>; Daniel Vetter > <daniel.vetter@intel.com> > Subject: Re: [PATCH 53/59] drm/arc: Move to drm/tiny > > Hi Daniel. > On Wed, Apr 15, 2020 at 09:40:28AM +0200, Daniel Vetter wrote: > > Because it is. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Alexey Brodkin <abrodkin@synopsys.com> > > --- > > MAINTAINERS | 2 +- > > drivers/gpu/drm/Kconfig | 2 -- > > drivers/gpu/drm/Makefile | 1 - > > drivers/gpu/drm/arc/Kconfig | 10 ---------- > > drivers/gpu/drm/arc/Makefile | 3 --- > > drivers/gpu/drm/tiny/Kconfig | 10 ++++++++++ > > drivers/gpu/drm/tiny/Makefile | 1 + > > drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} | 0 > > 8 files changed, 12 insertions(+), 17 deletions(-) > > delete mode 100644 drivers/gpu/drm/arc/Kconfig > > delete mode 100644 drivers/gpu/drm/arc/Makefile > > rename drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} (100%) > > We have "DRM: ARC: add HDMI 2.0 TX encoder support" which > adds another platform driver to drm/arc/ > This speaks against the move to tiny IMO Indeed that's an interesting question, see v3 series here: https://lists.freedesktop.org/archives/dri-devel/2020-April/262352.html BTW should I pull that series in my tree and send you a pull-request or that kind of change needs to go through another tree? Also I'd like to test the change we discuss here to make sure stuff still works. Once we do that I'll send an update. Any hint on when that change needs to be acked/nacked? -Alexey
On Wed, Apr 15, 2020 at 2:03 PM Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > > Hi Daniel, > > > -----Original Message----- > > From: Sam Ravnborg <sam@ravnborg.org> > > Sent: Wednesday, April 15, 2020 12:45 PM > > To: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>; Alexey Brodkin > > <abrodkin@synopsys.com>; DRI Development <dri-devel@lists.freedesktop.org>; Daniel Vetter > > <daniel.vetter@intel.com> > > Subject: Re: [PATCH 53/59] drm/arc: Move to drm/tiny > > > > Hi Daniel. > > On Wed, Apr 15, 2020 at 09:40:28AM +0200, Daniel Vetter wrote: > > > Because it is. > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > Cc: Alexey Brodkin <abrodkin@synopsys.com> > > > --- > > > MAINTAINERS | 2 +- > > > drivers/gpu/drm/Kconfig | 2 -- > > > drivers/gpu/drm/Makefile | 1 - > > > drivers/gpu/drm/arc/Kconfig | 10 ---------- > > > drivers/gpu/drm/arc/Makefile | 3 --- > > > drivers/gpu/drm/tiny/Kconfig | 10 ++++++++++ > > > drivers/gpu/drm/tiny/Makefile | 1 + > > > drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} | 0 > > > 8 files changed, 12 insertions(+), 17 deletions(-) > > > delete mode 100644 drivers/gpu/drm/arc/Kconfig > > > delete mode 100644 drivers/gpu/drm/arc/Makefile > > > rename drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} (100%) > > > > We have "DRM: ARC: add HDMI 2.0 TX encoder support" which > > adds another platform driver to drm/arc/ > > This speaks against the move to tiny IMO > > Indeed that's an interesting question, see v3 series here: > https://lists.freedesktop.org/archives/dri-devel/2020-April/262352.html Looking at this patch series, feels a bit like hand-rolling of bridge code, badly. We should get away from that. Once you have that I think the end result is tiny enough that it can stay, bridges intergrate quite well into simple display pipe drivers. > BTW should I pull that series in my tree and send you a pull-request > or that kind of change needs to go through another tree? > > Also I'd like to test the change we discuss here to make sure stuff > still works. Once we do that I'll send an update. Any hint on > when that change needs to be acked/nacked? Simplest is if this can all land through drm-misc, is arc not maintained in there? And there's plenty of time for testing, I'm just slowly crawling through the tree to get everything polished and cleaned up in this area. -Daniel
On Wed, Apr 15, 2020 at 02:20:35PM +0200, Daniel Vetter wrote: > On Wed, Apr 15, 2020 at 2:03 PM Alexey Brodkin > <Alexey.Brodkin@synopsys.com> wrote: > > > > Hi Daniel, > > > > > -----Original Message----- > > > From: Sam Ravnborg <sam@ravnborg.org> > > > Sent: Wednesday, April 15, 2020 12:45 PM > > > To: Daniel Vetter <daniel.vetter@ffwll.ch> > > > Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>; Alexey Brodkin > > > <abrodkin@synopsys.com>; DRI Development <dri-devel@lists.freedesktop.org>; Daniel Vetter > > > <daniel.vetter@intel.com> > > > Subject: Re: [PATCH 53/59] drm/arc: Move to drm/tiny > > > > > > Hi Daniel. > > > On Wed, Apr 15, 2020 at 09:40:28AM +0200, Daniel Vetter wrote: > > > > Because it is. > > > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > > Cc: Alexey Brodkin <abrodkin@synopsys.com> > > > > --- > > > > MAINTAINERS | 2 +- > > > > drivers/gpu/drm/Kconfig | 2 -- > > > > drivers/gpu/drm/Makefile | 1 - > > > > drivers/gpu/drm/arc/Kconfig | 10 ---------- > > > > drivers/gpu/drm/arc/Makefile | 3 --- > > > > drivers/gpu/drm/tiny/Kconfig | 10 ++++++++++ > > > > drivers/gpu/drm/tiny/Makefile | 1 + > > > > drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} | 0 > > > > 8 files changed, 12 insertions(+), 17 deletions(-) > > > > delete mode 100644 drivers/gpu/drm/arc/Kconfig > > > > delete mode 100644 drivers/gpu/drm/arc/Makefile > > > > rename drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} (100%) > > > > > > We have "DRM: ARC: add HDMI 2.0 TX encoder support" which > > > adds another platform driver to drm/arc/ > > > This speaks against the move to tiny IMO > > > > Indeed that's an interesting question, see v3 series here: > > https://lists.freedesktop.org/archives/dri-devel/2020-April/262352.html > > Looking at this patch series, feels a bit like hand-rolling of bridge > code, badly. We should get away from that. > > Once you have that I think the end result is tiny enough that it can > stay, bridges intergrate quite well into simple display pipe drivers. > > > BTW should I pull that series in my tree and send you a pull-request > > or that kind of change needs to go through another tree? > > > > Also I'd like to test the change we discuss here to make sure stuff > > still works. Once we do that I'll send an update. Any hint on > > when that change needs to be acked/nacked? > > Simplest is if this can all land through drm-misc, is arc not > maintained in there? And there's plenty of time for testing, I'm just > slowly crawling through the tree to get everything polished and > cleaned up in this area. Any updates on testing this pile here? First patch landed now, and I've started to push driver patches. So would be good to get this sorted out too. -Daniel
Hi Daniel, > > Looking at this patch series, feels a bit like hand-rolling of bridge > > code, badly. We should get away from that. > > > > Once you have that I think the end result is tiny enough that it can > > stay, bridges intergrate quite well into simple display pipe drivers. > > > > > BTW should I pull that series in my tree and send you a pull-request > > > or that kind of change needs to go through another tree? > > > > > > Also I'd like to test the change we discuss here to make sure stuff > > > still works. Once we do that I'll send an update. Any hint on > > > when that change needs to be acked/nacked? > > > > Simplest is if this can all land through drm-misc, is arc not > > maintained in there? And there's plenty of time for testing, I'm just > > slowly crawling through the tree to get everything polished and > > cleaned up in this area. > > Any updates on testing this pile here? First patch landed now, and I've > started to push driver patches. So would be good to get this sorted out > too. Sorry we're in the middle of 2 long weekends so missed this one. I guess we'll be able to test it in a week or two from now. Is that OK? -Alexey
On Fri, May 8, 2020 at 3:56 PM Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > > Hi Daniel, > > > > Looking at this patch series, feels a bit like hand-rolling of bridge > > > code, badly. We should get away from that. > > > > > > Once you have that I think the end result is tiny enough that it can > > > stay, bridges intergrate quite well into simple display pipe drivers. > > > > > > > BTW should I pull that series in my tree and send you a pull-request > > > > or that kind of change needs to go through another tree? > > > > > > > > Also I'd like to test the change we discuss here to make sure stuff > > > > still works. Once we do that I'll send an update. Any hint on > > > > when that change needs to be acked/nacked? > > > > > > Simplest is if this can all land through drm-misc, is arc not > > > maintained in there? And there's plenty of time for testing, I'm just > > > slowly crawling through the tree to get everything polished and > > > cleaned up in this area. > > > > Any updates on testing this pile here? First patch landed now, and I've > > started to push driver patches. So would be good to get this sorted out > > too. > > Sorry we're in the middle of 2 long weekends so missed this one. > I guess we'll be able to test it in a week or two from now. > > Is that OK? This aren't high-priority, so totally ok. As long as you don't land a driver rewrite and I have to rebase everything :-) -Daniel > > -Alexey > >
On Fri, May 08, 2020 at 08:07:41PM +0200, Daniel Vetter wrote: > On Fri, May 8, 2020 at 3:56 PM Alexey Brodkin > <Alexey.Brodkin@synopsys.com> wrote: > > > > Hi Daniel, > > > > > > Looking at this patch series, feels a bit like hand-rolling of bridge > > > > code, badly. We should get away from that. > > > > > > > > Once you have that I think the end result is tiny enough that it can > > > > stay, bridges intergrate quite well into simple display pipe drivers. > > > > > > > > > BTW should I pull that series in my tree and send you a pull-request > > > > > or that kind of change needs to go through another tree? > > > > > > > > > > Also I'd like to test the change we discuss here to make sure stuff > > > > > still works. Once we do that I'll send an update. Any hint on > > > > > when that change needs to be acked/nacked? > > > > > > > > Simplest is if this can all land through drm-misc, is arc not > > > > maintained in there? And there's plenty of time for testing, I'm just > > > > slowly crawling through the tree to get everything polished and > > > > cleaned up in this area. > > > > > > Any updates on testing this pile here? First patch landed now, and I've > > > started to push driver patches. So would be good to get this sorted out > > > too. > > > > Sorry we're in the middle of 2 long weekends so missed this one. > > I guess we'll be able to test it in a week or two from now. > > > > Is that OK? > > This aren't high-priority, so totally ok. As long as you don't land a > driver rewrite and I have to rebase everything :-) Ping for a bit of testing on this stuff ... -Daniel
Hi Daniel, I've already tested it (and found several issues), so please check my reply here: https://www.mail-archive.com/linux-snps-arc@lists.infradead.org/msg07403.html Not sure why you didn't receive my reply (probably the reason is because it was sent to your @ffwll.ch mail instead of @intel.com one).
Hi Eugeniy, Apologies, somehow I missed your mail. I looked at the code again, and I think I fumbled something. Does the below diff help to prevent the issues? Thanks, Daniel diff --git a/drivers/gpu/drm/tiny/arcpgu.c b/drivers/gpu/drm/tiny/arcpgu.c index 857812f25bec..33d812a5ad7f 100644 --- a/drivers/gpu/drm/tiny/arcpgu.c +++ b/drivers/gpu/drm/tiny/arcpgu.c @@ -228,6 +228,9 @@ static void arc_pgu_update(struct drm_simple_display_pipe *pipe, struct arcpgu_drm_private *arcpgu; struct drm_gem_cma_object *gem; + if (!pipe->plane.state->fb) + return; + arcpgu = pipe_to_arcpgu_priv(pipe); gem = drm_fb_cma_get_gem_obj(pipe->plane.state->fb, 0); arc_pgu_write(arcpgu, ARCPGU_REG_BUF0_ADDR, gem->paddr);
I've tested your change and one issue gone. However I still see kernel crash (due to invalid read in kernel mode by 0x0 address) on weston stop: ----------------------------------->8------------------------------------------- Oops Path: (null) CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.7.0-rc6-01594-g4ceda91a4176-dirty #6 Workqueue: events drm_mode_rmfb_work_fn Invalid Read @ 0x00000000 by insn @ drm_gem_fb_destroy+0x32/0x130 ECR: 0x00050100 EFA: 0x00000000 ERET: 0x813b9a76 STAT32: 0x80080602 [IE K ] BTA: 0x813b9a72 BLK: drm_gem_fb_destroy+0xc0/0x130 SP: 0x9f055ea4 FP: 0x00000000 LPS: 0x813560ec LPE: 0x813560f0 LPC: 0x00000000 r00: 0x00000000 r01: 0x9f6a6100 r02: 0x00000001 r03: 0x9fd5dde8 r04: 0x810f5de8 r05: 0x00000000 r06: 0x00000000 r07: 0x00000000 r08: 0x000000e1 r09: 0x00000000 r10: 0x00000000 r11: 0x000000e1 r12: 0x813b9b04 Stack Trace: drm_gem_fb_destroy+0x32/0x130 drm_framebuffer_remove+0x1d2/0x358 drm_mode_rmfb_work_fn+0x28/0x38 process_one_work+0x19a/0x358 worker_thread+0x2c4/0x494 kthread+0xec/0x100 ret_from_fork+0x18/0x1c ----------------------------------->8------------------------------------------- The stack traces may vary but always end in drm_gem_fb_destroy: ----------------------------------->8------------------------------------------- Stack Trace: drm_gem_fb_destroy+0x32/0x130 drm_mode_rmfb+0x10e/0x148 drm_ioctl_kernel+0x70/0xa0 drm_ioctl+0x284/0x410 ksys_ioctl+0xea/0xa3c EV_Trap+0xcc/0xd0 ----------------------------------->8------------------------------------------- Stack Trace: drm_gem_fb_destroy+0x32/0x130 drm_fb_release+0x66/0xb0 drm_file_free.part.11+0x112/0x1bc drm_release+0x80/0x120 __fput+0x98/0x1bc task_work_run+0x6e/0xa8 do_exit+0x2b4/0x7fc do_group_exit+0x2a/0x8c get_signal+0x9a/0x5f0 do_signal+0x86/0x23c resume_user_mode_begin+0x88/0xd0 ----------------------------------->8------------------------------------------- --- Eugeniy Paltsev
Hi Eugeniy, Thanks for testing. I looked at the second one (I hoped it would just magically disappear) and I still don't understand what's going on there. My patch series isn't touching that area at all, so really confused. I squashed in the bugfix from the previous round into the right patches, and pushed a branch with just the arcpgu changes here: https://cgit.freedesktop.org/~danvet/drm/log/?h=for-eugeniy Maybe it's something in my pile of not-so-tested stuff :-) Can you pls test this? And if it still fails, try to bisect where it breaks? Thanks, Daniel On Thu, Jun 4, 2020 at 9:00 PM Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> wrote: > > I've tested your change and one issue gone. > > However I still see kernel crash (due to invalid read in kernel mode by 0x0 address) on weston stop: > ----------------------------------->8------------------------------------------- > Oops > Path: (null) > CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.7.0-rc6-01594-g4ceda91a4176-dirty #6 > Workqueue: events drm_mode_rmfb_work_fn > Invalid Read @ 0x00000000 by insn @ drm_gem_fb_destroy+0x32/0x130 > ECR: 0x00050100 EFA: 0x00000000 ERET: 0x813b9a76 > STAT32: 0x80080602 [IE K ] BTA: 0x813b9a72 > BLK: drm_gem_fb_destroy+0xc0/0x130 > SP: 0x9f055ea4 FP: 0x00000000 > LPS: 0x813560ec LPE: 0x813560f0 LPC: 0x00000000 > r00: 0x00000000 r01: 0x9f6a6100 r02: 0x00000001 > r03: 0x9fd5dde8 r04: 0x810f5de8 r05: 0x00000000 > r06: 0x00000000 r07: 0x00000000 r08: 0x000000e1 > r09: 0x00000000 r10: 0x00000000 r11: 0x000000e1 > r12: 0x813b9b04 > > Stack Trace: > drm_gem_fb_destroy+0x32/0x130 > drm_framebuffer_remove+0x1d2/0x358 > drm_mode_rmfb_work_fn+0x28/0x38 > process_one_work+0x19a/0x358 > worker_thread+0x2c4/0x494 > kthread+0xec/0x100 > ret_from_fork+0x18/0x1c > ----------------------------------->8------------------------------------------- > > > The stack traces may vary but always end in drm_gem_fb_destroy: > ----------------------------------->8------------------------------------------- > Stack Trace: > drm_gem_fb_destroy+0x32/0x130 > drm_mode_rmfb+0x10e/0x148 > drm_ioctl_kernel+0x70/0xa0 > drm_ioctl+0x284/0x410 > ksys_ioctl+0xea/0xa3c > EV_Trap+0xcc/0xd0 > ----------------------------------->8------------------------------------------- > Stack Trace: > drm_gem_fb_destroy+0x32/0x130 > drm_fb_release+0x66/0xb0 > drm_file_free.part.11+0x112/0x1bc > drm_release+0x80/0x120 > __fput+0x98/0x1bc > task_work_run+0x6e/0xa8 > do_exit+0x2b4/0x7fc > do_group_exit+0x2a/0x8c > get_signal+0x9a/0x5f0 > do_signal+0x86/0x23c > resume_user_mode_begin+0x88/0xd0 > ----------------------------------->8------------------------------------------- > > > --- > Eugeniy Paltsev > > > ________________________________________ > From: Daniel Vetter <daniel@ffwll.ch> > Sent: Thursday, June 4, 2020 14:19 > To: Eugeniy Paltsev > Cc: Intel Graphics Development; DRI Development; Daniel Vetter; Sam Ravnborg; Alexey Brodkin > Subject: Re: [PATCH 53/59] drm/arc: Move to drm/tiny > > Hi Eugeniy, > > Apologies, somehow I missed your mail. I looked at the code again, and I > think I fumbled something. Does the below diff help to prevent the issues? > > Thanks, Daniel > > > diff --git a/drivers/gpu/drm/tiny/arcpgu.c b/drivers/gpu/drm/tiny/arcpgu.c > index 857812f25bec..33d812a5ad7f 100644 > --- a/drivers/gpu/drm/tiny/arcpgu.c > +++ b/drivers/gpu/drm/tiny/arcpgu.c > @@ -228,6 +228,9 @@ static void arc_pgu_update(struct drm_simple_display_pipe *pipe, > struct arcpgu_drm_private *arcpgu; > struct drm_gem_cma_object *gem; > > + if (!pipe->plane.state->fb) > + return; > + > arcpgu = pipe_to_arcpgu_priv(pipe); > gem = drm_fb_cma_get_gem_obj(pipe->plane.state->fb, 0); > arc_pgu_write(arcpgu, ARCPGU_REG_BUF0_ADDR, gem->paddr); > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - https://urldefense.com/v3/__http://blog.ffwll.ch__;!!A4F2R9G_pg!P0EvyJfMuDwqbeZmHZM5S9po30QWr4KgGrggRirNfgo7wrRXfnUO-8iq0AA4fQCW2WGPlDc$
Hi Daniel, I've got pretty strange results so I need some time to investigate it and probably retest. I'll send you update in a few days. --- Eugeniy Paltsev
Hi Eugeniy, Very much appreciated, and kinda expected. That 2nd backtrace really confuses me, so "something strange is going on" and the bisect looks funny is within expectations. Hopefully we can track down what's going on. Thanks, Daniel On Tue, Jun 9, 2020 at 2:08 PM Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> wrote: > > Hi Daniel, > > I've got pretty strange results so I need some time to investigate it and probably retest. > I'll send you update in a few days. > > --- > Eugeniy Paltsev > > > ________________________________________ > From: Daniel Vetter <daniel@ffwll.ch> > Sent: Friday, June 5, 2020 22:55 > To: Eugeniy Paltsev > Cc: Intel Graphics Development; DRI Development; Daniel Vetter; Sam Ravnborg; Alexey Brodkin; snps-arc@lists.infradead.org > Subject: Re: [PATCH 53/59] drm/arc: Move to drm/tiny > > Hi Eugeniy, > > Thanks for testing. I looked at the second one (I hoped it would just > magically disappear) and I still don't understand what's going on > there. My patch series isn't touching that area at all, so really > confused. > > I squashed in the bugfix from the previous round into the right > patches, and pushed a branch with just the arcpgu changes here: > https://urldefense.com/v3/__https://cgit.freedesktop.org/*danvet/drm/log/?h=for-eugeniy__;fg!!A4F2R9G_pg!IJ1o4XiXVdStPu--Q-SCTUpRbsbqrjX255R34nuD7L7ptPywOy4SKr21dwSpfOkXIVqH5pM$ > > Maybe it's something in my pile of not-so-tested stuff :-) > > Can you pls test this? And if it still fails, try to bisect where it breaks? > > Thanks, Daniel > > On Thu, Jun 4, 2020 at 9:00 PM Eugeniy Paltsev > <Eugeniy.Paltsev@synopsys.com> wrote: > > > > I've tested your change and one issue gone. > > > > However I still see kernel crash (due to invalid read in kernel mode by 0x0 address) on weston stop: > > ----------------------------------->8------------------------------------------- > > Oops > > Path: (null) > > CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.7.0-rc6-01594-g4ceda91a4176-dirty #6 > > Workqueue: events drm_mode_rmfb_work_fn > > Invalid Read @ 0x00000000 by insn @ drm_gem_fb_destroy+0x32/0x130 > > ECR: 0x00050100 EFA: 0x00000000 ERET: 0x813b9a76 > > STAT32: 0x80080602 [IE K ] BTA: 0x813b9a72 > > BLK: drm_gem_fb_destroy+0xc0/0x130 > > SP: 0x9f055ea4 FP: 0x00000000 > > LPS: 0x813560ec LPE: 0x813560f0 LPC: 0x00000000 > > r00: 0x00000000 r01: 0x9f6a6100 r02: 0x00000001 > > r03: 0x9fd5dde8 r04: 0x810f5de8 r05: 0x00000000 > > r06: 0x00000000 r07: 0x00000000 r08: 0x000000e1 > > r09: 0x00000000 r10: 0x00000000 r11: 0x000000e1 > > r12: 0x813b9b04 > > > > Stack Trace: > > drm_gem_fb_destroy+0x32/0x130 > > drm_framebuffer_remove+0x1d2/0x358 > > drm_mode_rmfb_work_fn+0x28/0x38 > > process_one_work+0x19a/0x358 > > worker_thread+0x2c4/0x494 > > kthread+0xec/0x100 > > ret_from_fork+0x18/0x1c > > ----------------------------------->8------------------------------------------- > > > > > > The stack traces may vary but always end in drm_gem_fb_destroy: > > ----------------------------------->8------------------------------------------- > > Stack Trace: > > drm_gem_fb_destroy+0x32/0x130 > > drm_mode_rmfb+0x10e/0x148 > > drm_ioctl_kernel+0x70/0xa0 > > drm_ioctl+0x284/0x410 > > ksys_ioctl+0xea/0xa3c > > EV_Trap+0xcc/0xd0 > > ----------------------------------->8------------------------------------------- > > Stack Trace: > > drm_gem_fb_destroy+0x32/0x130 > > drm_fb_release+0x66/0xb0 > > drm_file_free.part.11+0x112/0x1bc > > drm_release+0x80/0x120 > > __fput+0x98/0x1bc > > task_work_run+0x6e/0xa8 > > do_exit+0x2b4/0x7fc > > do_group_exit+0x2a/0x8c > > get_signal+0x9a/0x5f0 > > do_signal+0x86/0x23c > > resume_user_mode_begin+0x88/0xd0 > > ----------------------------------->8------------------------------------------- > > > > > > --- > > Eugeniy Paltsev > > > > > > ________________________________________ > > From: Daniel Vetter <daniel@ffwll.ch> > > Sent: Thursday, June 4, 2020 14:19 > > To: Eugeniy Paltsev > > Cc: Intel Graphics Development; DRI Development; Daniel Vetter; Sam Ravnborg; Alexey Brodkin > > Subject: Re: [PATCH 53/59] drm/arc: Move to drm/tiny > > > > Hi Eugeniy, > > > > Apologies, somehow I missed your mail. I looked at the code again, and I > > think I fumbled something. Does the below diff help to prevent the issues? > > > > Thanks, Daniel > > > > > > diff --git a/drivers/gpu/drm/tiny/arcpgu.c b/drivers/gpu/drm/tiny/arcpgu.c > > index 857812f25bec..33d812a5ad7f 100644 > > --- a/drivers/gpu/drm/tiny/arcpgu.c > > +++ b/drivers/gpu/drm/tiny/arcpgu.c > > @@ -228,6 +228,9 @@ static void arc_pgu_update(struct drm_simple_display_pipe *pipe, > > struct arcpgu_drm_private *arcpgu; > > struct drm_gem_cma_object *gem; > > > > + if (!pipe->plane.state->fb) > > + return; > > + > > arcpgu = pipe_to_arcpgu_priv(pipe); > > gem = drm_fb_cma_get_gem_obj(pipe->plane.state->fb, 0); > > arc_pgu_write(arcpgu, ARCPGU_REG_BUF0_ADDR, gem->paddr); > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - https://urldefense.com/v3/__http://blog.ffwll.ch__;!!A4F2R9G_pg!P0EvyJfMuDwqbeZmHZM5S9po30QWr4KgGrggRirNfgo7wrRXfnUO-8iq0AA4fQCW2WGPlDc$ > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - https://urldefense.com/v3/__http://blog.ffwll.ch__;!!A4F2R9G_pg!IJ1o4XiXVdStPu--Q-SCTUpRbsbqrjX255R34nuD7L7ptPywOy4SKr21dwSpfOkXpn86Q20$
On Tue, Jun 09, 2020 at 03:02:14PM +0200, Daniel Vetter wrote: > Hi Eugeniy, > > Very much appreciated, and kinda expected. That 2nd backtrace really > confuses me, so "something strange is going on" and the bisect looks > funny is within expectations. Hopefully we can track down what's going > on. I'm going to resend the entire pile with the bugfix below and all rebased, I think retesting it all is probably a good idea now, since quite some time passed. Cheers, Daniel > > Thanks, Daniel > > On Tue, Jun 9, 2020 at 2:08 PM Eugeniy Paltsev > <Eugeniy.Paltsev@synopsys.com> wrote: > > > > Hi Daniel, > > > > I've got pretty strange results so I need some time to investigate it and probably retest. > > I'll send you update in a few days. > > > > --- > > Eugeniy Paltsev > > > > > > ________________________________________ > > From: Daniel Vetter <daniel@ffwll.ch> > > Sent: Friday, June 5, 2020 22:55 > > To: Eugeniy Paltsev > > Cc: Intel Graphics Development; DRI Development; Daniel Vetter; Sam Ravnborg; Alexey Brodkin; snps-arc@lists.infradead.org > > Subject: Re: [PATCH 53/59] drm/arc: Move to drm/tiny > > > > Hi Eugeniy, > > > > Thanks for testing. I looked at the second one (I hoped it would just > > magically disappear) and I still don't understand what's going on > > there. My patch series isn't touching that area at all, so really > > confused. > > > > I squashed in the bugfix from the previous round into the right > > patches, and pushed a branch with just the arcpgu changes here: > > https://urldefense.com/v3/__https://cgit.freedesktop.org/*danvet/drm/log/?h=for-eugeniy__;fg!!A4F2R9G_pg!IJ1o4XiXVdStPu--Q-SCTUpRbsbqrjX255R34nuD7L7ptPywOy4SKr21dwSpfOkXIVqH5pM$ > > > > Maybe it's something in my pile of not-so-tested stuff :-) > > > > Can you pls test this? And if it still fails, try to bisect where it breaks? > > > > Thanks, Daniel > > > > On Thu, Jun 4, 2020 at 9:00 PM Eugeniy Paltsev > > <Eugeniy.Paltsev@synopsys.com> wrote: > > > > > > I've tested your change and one issue gone. > > > > > > However I still see kernel crash (due to invalid read in kernel mode by 0x0 address) on weston stop: > > > ----------------------------------->8------------------------------------------- > > > Oops > > > Path: (null) > > > CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.7.0-rc6-01594-g4ceda91a4176-dirty #6 > > > Workqueue: events drm_mode_rmfb_work_fn > > > Invalid Read @ 0x00000000 by insn @ drm_gem_fb_destroy+0x32/0x130 > > > ECR: 0x00050100 EFA: 0x00000000 ERET: 0x813b9a76 > > > STAT32: 0x80080602 [IE K ] BTA: 0x813b9a72 > > > BLK: drm_gem_fb_destroy+0xc0/0x130 > > > SP: 0x9f055ea4 FP: 0x00000000 > > > LPS: 0x813560ec LPE: 0x813560f0 LPC: 0x00000000 > > > r00: 0x00000000 r01: 0x9f6a6100 r02: 0x00000001 > > > r03: 0x9fd5dde8 r04: 0x810f5de8 r05: 0x00000000 > > > r06: 0x00000000 r07: 0x00000000 r08: 0x000000e1 > > > r09: 0x00000000 r10: 0x00000000 r11: 0x000000e1 > > > r12: 0x813b9b04 > > > > > > Stack Trace: > > > drm_gem_fb_destroy+0x32/0x130 > > > drm_framebuffer_remove+0x1d2/0x358 > > > drm_mode_rmfb_work_fn+0x28/0x38 > > > process_one_work+0x19a/0x358 > > > worker_thread+0x2c4/0x494 > > > kthread+0xec/0x100 > > > ret_from_fork+0x18/0x1c > > > ----------------------------------->8------------------------------------------- > > > > > > > > > The stack traces may vary but always end in drm_gem_fb_destroy: > > > ----------------------------------->8------------------------------------------- > > > Stack Trace: > > > drm_gem_fb_destroy+0x32/0x130 > > > drm_mode_rmfb+0x10e/0x148 > > > drm_ioctl_kernel+0x70/0xa0 > > > drm_ioctl+0x284/0x410 > > > ksys_ioctl+0xea/0xa3c > > > EV_Trap+0xcc/0xd0 > > > ----------------------------------->8------------------------------------------- > > > Stack Trace: > > > drm_gem_fb_destroy+0x32/0x130 > > > drm_fb_release+0x66/0xb0 > > > drm_file_free.part.11+0x112/0x1bc > > > drm_release+0x80/0x120 > > > __fput+0x98/0x1bc > > > task_work_run+0x6e/0xa8 > > > do_exit+0x2b4/0x7fc > > > do_group_exit+0x2a/0x8c > > > get_signal+0x9a/0x5f0 > > > do_signal+0x86/0x23c > > > resume_user_mode_begin+0x88/0xd0 > > > ----------------------------------->8------------------------------------------- > > > > > > > > > --- > > > Eugeniy Paltsev > > > > > > > > > ________________________________________ > > > From: Daniel Vetter <daniel@ffwll.ch> > > > Sent: Thursday, June 4, 2020 14:19 > > > To: Eugeniy Paltsev > > > Cc: Intel Graphics Development; DRI Development; Daniel Vetter; Sam Ravnborg; Alexey Brodkin > > > Subject: Re: [PATCH 53/59] drm/arc: Move to drm/tiny > > > > > > Hi Eugeniy, > > > > > > Apologies, somehow I missed your mail. I looked at the code again, and I > > > think I fumbled something. Does the below diff help to prevent the issues? > > > > > > Thanks, Daniel > > > > > > > > > diff --git a/drivers/gpu/drm/tiny/arcpgu.c b/drivers/gpu/drm/tiny/arcpgu.c > > > index 857812f25bec..33d812a5ad7f 100644 > > > --- a/drivers/gpu/drm/tiny/arcpgu.c > > > +++ b/drivers/gpu/drm/tiny/arcpgu.c > > > @@ -228,6 +228,9 @@ static void arc_pgu_update(struct drm_simple_display_pipe *pipe, > > > struct arcpgu_drm_private *arcpgu; > > > struct drm_gem_cma_object *gem; > > > > > > + if (!pipe->plane.state->fb) > > > + return; > > > + > > > arcpgu = pipe_to_arcpgu_priv(pipe); > > > gem = drm_fb_cma_get_gem_obj(pipe->plane.state->fb, 0); > > > arc_pgu_write(arcpgu, ARCPGU_REG_BUF0_ADDR, gem->paddr); > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > +41 (0) 79 365 57 48 - https://urldefense.com/v3/__http://blog.ffwll.ch__;!!A4F2R9G_pg!P0EvyJfMuDwqbeZmHZM5S9po30QWr4KgGrggRirNfgo7wrRXfnUO-8iq0AA4fQCW2WGPlDc$ > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - https://urldefense.com/v3/__http://blog.ffwll.ch__;!!A4F2R9G_pg!IJ1o4XiXVdStPu--Q-SCTUpRbsbqrjX255R34nuD7L7ptPywOy4SKr21dwSpfOkXpn86Q20$ > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/MAINTAINERS b/MAINTAINERS index 0a5cf105ee37..748244b1625b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1295,7 +1295,7 @@ ARC PGU DRM DRIVER M: Alexey Brodkin <abrodkin@synopsys.com> S: Supported F: Documentation/devicetree/bindings/display/snps,arcpgu.txt -F: drivers/gpu/drm/arc/ +F: drivers/gpu/drm/tiny/arcpgu.c ARCNET NETWORK LAYER M: Michael Grzeschik <m.grzeschik@pengutronix.de> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 4f4e7fa001c1..a0a89025d6fa 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -354,8 +354,6 @@ source "drivers/gpu/drm/vc4/Kconfig" source "drivers/gpu/drm/etnaviv/Kconfig" -source "drivers/gpu/drm/arc/Kconfig" - source "drivers/gpu/drm/hisilicon/Kconfig" source "drivers/gpu/drm/mediatek/Kconfig" diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 2c0e5a7e5953..e69eafbf9e39 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -109,7 +109,6 @@ obj-y += panel/ obj-y += bridge/ obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/ obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/ -obj-$(CONFIG_DRM_ARCPGU)+= arc/ obj-y += hisilicon/ obj-$(CONFIG_DRM_ZTE) += zte/ obj-$(CONFIG_DRM_MXSFB) += mxsfb/ diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig deleted file mode 100644 index e8f3d63e0b91..000000000000 --- a/drivers/gpu/drm/arc/Kconfig +++ /dev/null @@ -1,10 +0,0 @@ -# SPDX-License-Identifier: GPL-2.0-only -config DRM_ARCPGU - tristate "ARC PGU" - depends on DRM && OF - select DRM_KMS_CMA_HELPER - select DRM_KMS_HELPER - help - Choose this option if you have an ARC PGU controller. - - If M is selected the module will be called arcpgu. diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile deleted file mode 100644 index b26f2495c532..000000000000 --- a/drivers/gpu/drm/arc/Makefile +++ /dev/null @@ -1,3 +0,0 @@ -# SPDX-License-Identifier: GPL-2.0-only -arcpgu-y := arcpgu_drv.o -obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index 2b6414f0fa75..9bbaa1a69050 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -1,5 +1,15 @@ # SPDX-License-Identifier: GPL-2.0-only +config DRM_ARCPGU + tristate "ARC PGU" + depends on DRM && OF + select DRM_KMS_CMA_HELPER + select DRM_KMS_HELPER + help + Choose this option if you have an ARC PGU controller. + + If M is selected the module will be called arcpgu. + config DRM_CIRRUS_QEMU tristate "Cirrus driver for QEMU emulated device" depends on DRM && PCI && MMU diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index 6ae4e9e5a35f..bef6780bdd6f 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only +obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o obj-$(CONFIG_DRM_GM12U320) += gm12u320.o obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/tiny/arcpgu.c similarity index 100% rename from drivers/gpu/drm/arc/arcpgu_drv.c rename to drivers/gpu/drm/tiny/arcpgu.c
Because it is. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Alexey Brodkin <abrodkin@synopsys.com> --- MAINTAINERS | 2 +- drivers/gpu/drm/Kconfig | 2 -- drivers/gpu/drm/Makefile | 1 - drivers/gpu/drm/arc/Kconfig | 10 ---------- drivers/gpu/drm/arc/Makefile | 3 --- drivers/gpu/drm/tiny/Kconfig | 10 ++++++++++ drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} | 0 8 files changed, 12 insertions(+), 17 deletions(-) delete mode 100644 drivers/gpu/drm/arc/Kconfig delete mode 100644 drivers/gpu/drm/arc/Makefile rename drivers/gpu/drm/{arc/arcpgu_drv.c => tiny/arcpgu.c} (100%)