Re: [PATCH] drm: renesas: shmobile: shmo_drm_crtc: Fix PM imbalance if RPM_ACTIVE is true
Hi Biju,
On Mon, Jul 8, 2024 at 11:00 AM Biju Das wrote:
> > From: Maxime Ripard
> > On Mon, Jul 08, 2024 at 09:27:09AM GMT, Biju Das wrote:
> > > The pm_runtime_resume_and_get() returns 1 if RPM is active, in this
> > > case it won't call a put. This will result in PM imbalance as it treat
> > > this as an error and propagate this to caller and the caller never
> > > calls corresponding put(). Fix this issue by checking error condition
> > > only.
> > >
> > > Signed-off-by: Biju Das
> > > --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c
> > > +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c
> > > @@ -208,7 +208,7 @@ static void shmob_drm_crtc_atomic_enable(struct
> > > drm_crtc *crtc,
> > > int ret;
> > >
> > > ret = pm_runtime_resume_and_get(dev);
> > > - if (ret)
> > > + if (ret < 0)
> > > return;
> >
> > The documentation of pm_runtime_resume_and_get says that:
> >
> > Resume @dev synchronously and if that is successful, increment its
> > runtime PM usage counter. Return 0 if the runtime PM usage counter of
> > @dev has been incremented or a negative error code otherwise.
> >
> > So it looks like it can't return 1, ever. Are you sure you're not confusing
> > pm_runtime_resume_and_get
> > with pm_runtime_get?
>
> It should be ret < 0 as ret = 1 corresponds to RPM_ACTIVE and the API does
> not call put() when ret = 1; see [1] and [2]
>
> [1]
> https://elixir.bootlin.com/linux/v6.10-rc6/source/drivers/base/power/runtime.c#L778
>
> [2]
> https://elixir.bootlin.com/linux/v6.10-rc6/source/include/linux/pm_runtime.h#L431
>
> Am I miss anything? Please let me know.
Thanks for your patch, but the code for pm_runtime_resume_and_get()
seems to disagree?
https://elixir.bootlin.com/linux/latest/source/include/linux/pm_runtime.h#L436
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] drm: renesas: rcar-du: rcar_lvds: Fix PM imbalance if RPM_ACTIVE
Hi Biju,
On Mon, Jul 8, 2024 at 10:22 AM Biju Das wrote:
> The pm_runtime_resume_and_get() returns 1 if RPM is active, in this
> case it won't call a put. This will result in PM imbalance as it
> treat this as an error and propagate this to caller and the caller
> never calls corresponding put(). Fix this issue by checking error
> condition only.
>
> Signed-off-by: Biju Das
Thanks for your patch, but the code for pm_runtime_resume_and_get()
seems to disagree?
https://elixir.bootlin.com/linux/latest/source/include/linux/pm_runtime.h#L436
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] drm/fbdev-generic: Fix framebuffer on big endian devices
Hi Thomas,
On Mon, Jul 1, 2024 at 10:42 AM Thomas Huth wrote:
> On 28/06/2024 08.07, Thomas Zimmermann wrote:
> > Am 27.06.24 um 19:35 schrieb Thomas Huth:
> >> Starting with kernel 6.7, the framebuffer text console is not working
> >> anymore with the virtio-gpu device on s390x hosts. Such big endian fb
> >> devices are usinga different pixel ordering than little endian devices,
> >> e.g. DRM_FORMAT_BGRX instead of DRM_FORMAT_XRGB.
> >>
> >> This used to work fine as long as drm_client_buffer_addfb() was still
> >> calling drm_mode_addfb() which called drm_driver_legacy_fb_format()
> >> internally to get the right format. But drm_client_buffer_addfb() has
> >> recently been reworked to call drm_mode_addfb2() instead with the
> >> format value that has been passed to it as a parameter (see commit
> >> 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to
> >> drm_mode_addfb2()").
> >>
> >> That format parameter is determined in drm_fbdev_generic_helper_fb_probe()
> >> via the drm_mode_legacy_fb_format() function - which only generates
> >> formats suitable for little endian devices. So to fix this issue
> >> switch to drm_driver_legacy_fb_format() here instead to take the
> >> device endianness into consideration.
> >>
> >> Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to
> >> drm_mode_addfb2()")
> >> Closes: https://issues.redhat.com/browse/RHEL-45158
> >> Signed-off-by: Thomas Huth
> >
> > Acked-by: Thomas Zimmermann
> >
> >
> >> ---
> >> drivers/gpu/drm/drm_fbdev_generic.c | 3 ++-
> >
> > This file is now called drm_fbdev_ttm.c in drm-misc-next.
>
> Oh, ok, shall I send a v2 that is adjusted to that change, or can it be
> fixed while applying my patch?
As this is a regression in mainline, which needs to be backported,
too, it's best to apply your fix to v6.10-rc6, which does not have
drm_fbdev_ttm.c yet.
> > And a similar patch might be necessary for drm_fbdev_dma.c.
>
> Looks similar, indeed. Shall I send a patch for that one, too? ... I
> currently don't have a setup for testing that, though...
Obviously these need to be fixed, too.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] drm/fbdev-generic: Fix framebuffer on big endian devices
Hi Thomas,
On Fri, Jun 28, 2024 at 8:07 AM Thomas Zimmermann wrote:
> Am 27.06.24 um 19:35 schrieb Thomas Huth:
> > Starting with kernel 6.7, the framebuffer text console is not working
> > anymore with the virtio-gpu device on s390x hosts. Such big endian fb
> > devices are usinga different pixel ordering than little endian devices,
> > e.g. DRM_FORMAT_BGRX instead of DRM_FORMAT_XRGB.
> >
> > This used to work fine as long as drm_client_buffer_addfb() was still
> > calling drm_mode_addfb() which called drm_driver_legacy_fb_format()
> > internally to get the right format. But drm_client_buffer_addfb() has
> > recently been reworked to call drm_mode_addfb2() instead with the
> > format value that has been passed to it as a parameter (see commit
> > 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to
> > drm_mode_addfb2()").
> >
> > That format parameter is determined in drm_fbdev_generic_helper_fb_probe()
> > via the drm_mode_legacy_fb_format() function - which only generates
> > formats suitable for little endian devices. So to fix this issue
> > switch to drm_driver_legacy_fb_format() here instead to take the
> > device endianness into consideration.
> >
> > Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to
> > drm_mode_addfb2()")
> > Closes: https://issues.redhat.com/browse/RHEL-45158
> > Signed-off-by: Thomas Huth
>
> Acked-by: Thomas Zimmermann
>
>
> > ---
> > drivers/gpu/drm/drm_fbdev_generic.c | 3 ++-
>
> This file is now called drm_fbdev_ttm.c in drm-misc-next. And a similar
> patch might be necessary for drm_fbdev_dma.c. The code in
> drm_fbdev_shmem.c apparently has it already.
We are getting too many copies of this logic...
(yup, had to fix them all up in my WIP support for R1 ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] drm/fbdev-generic: Fix framebuffer on big endian devices
Hi Thomas,
CC Christian
On Thu, Jun 27, 2024 at 7:35 PM Thomas Huth wrote:
> Starting with kernel 6.7, the framebuffer text console is not working
> anymore with the virtio-gpu device on s390x hosts. Such big endian fb
> devices are usinga different pixel ordering than little endian devices,
> e.g. DRM_FORMAT_BGRX instead of DRM_FORMAT_XRGB.
>
> This used to work fine as long as drm_client_buffer_addfb() was still
> calling drm_mode_addfb() which called drm_driver_legacy_fb_format()
> internally to get the right format. But drm_client_buffer_addfb() has
> recently been reworked to call drm_mode_addfb2() instead with the
> format value that has been passed to it as a parameter (see commit
> 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to
> drm_mode_addfb2()").
>
> That format parameter is determined in drm_fbdev_generic_helper_fb_probe()
> via the drm_mode_legacy_fb_format() function - which only generates
> formats suitable for little endian devices. So to fix this issue
> switch to drm_driver_legacy_fb_format() here instead to take the
> device endianness into consideration.
>
> Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to
> drm_mode_addfb2()")
> Closes: https://issues.redhat.com/browse/RHEL-45158
> Signed-off-by: Thomas Huth
Reviewed-by: Geert Uytterhoeven
works fine on m68k-virt, so:
Tested-by: Geert Uytterhoeven
Christian had reported a similar issue before[1].
I submitted a different solution fixing virtio[2] instead, but that
caused issues with virtio-mouse-pci cursor...
> --- a/drivers/gpu/drm/drm_fbdev_generic.c
> +++ b/drivers/gpu/drm/drm_fbdev_generic.c
> @@ -84,7 +84,8 @@ static int drm_fbdev_generic_helper_fb_probe(struct
> drm_fb_helper *fb_helper,
> sizes->surface_width, sizes->surface_height,
> sizes->surface_bpp);
>
> - format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> sizes->surface_depth);
> + format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp,
> +sizes->surface_depth);
> buffer = drm_client_framebuffer_create(client, sizes->surface_width,
>sizes->surface_height, format);
> if (IS_ERR(buffer))
[1] https://lore.kernel.org/6530cea3-4507-454e-bc36-a6970c8e7...@xenosoft.de/
[2] "[PATCH v2] drm/virtio: Add suppport for non-native buffer formats"
https://lore.kernel.org/47a81d2e0e47b1715718779b6978a8b595cc7c5d.1700140609.git.ge...@linux-m68k.org
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH 1/2] drm/panic: Do not select DRM_KMS_HELPER
DRM core code cannot call into DRM helper code, as this would lead to
circular references in the modular case. Hence drop the selection of
DRM_KMS_HELPER. It was unused anyway, as v10 switched from using
the DRM format helpers to its own color format conversion, cfr. commit
9544309775c334c9 ("drm/panic: Add support for color format
conversion")).
Remove the unneeded include of .
Fixes: bf9fb17c6672868d ("drm/panic: Add a drm panic handler")
Signed-off-by: Geert Uytterhoeven
---
drivers/gpu/drm/Kconfig | 1 -
drivers/gpu/drm/drm_panic.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index b903a2c0b5e8f95c..ce9bf2b6e9d332d4 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -108,7 +108,6 @@ config DRM_KMS_HELPER
config DRM_PANIC
bool "Display a user-friendly message when a kernel panic occurs"
depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
- select DRM_KMS_HELPER
select FONT_SUPPORT
help
Enable a drm panic handler, which will display a user-friendly message
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 8d2eded1fd19ff6c..67f78b5a76b61e3d 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -20,7 +20,6 @@
#include
#include
-#include
#include
#include
#include
--
2.34.1
[PATCH 0/2] drm/panic: Miscellaneous fixes
Hi all,
Here are two more fixes for the DRM panic code.
Thanks for your comments!
Geert Uytterhoeven (2):
drm/panic: Do not select DRM_KMS_HELPER
drm/panic: Restrict graphical logo handling to built-in
drivers/gpu/drm/Kconfig | 1 -
drivers/gpu/drm/drm_panic.c | 3 +--
2 files changed, 1 insertion(+), 3 deletions(-)
--
2.34.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH 2/2] drm/panic: Restrict graphical logo handling to built-in
When CONFIG_DRM_PANIC=y, but CONFIG_DRM=m:
ld: drivers/gpu/drm/drm_panic.o: in function `drm_panic_setup_logo':
drivers/gpu/drm/drm_panic.c:99: multiple definition of `init_module';
drivers/gpu/drm/drm_drv.o:drivers/gpu/drm/drm_drv.c:1079: first defined here
Fix this by restricting the graphical logo handling and its
device_initcall() to the built-in case. Logos are freed during late
kernel initialization, so they are no longer available at module load
time anyway.
Fixes: 294bbd1f2697ff28 ("drm/panic: Add support for drawing a monochrome
graphical logo")
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202406261341.gysblpn1-...@intel.com/
Signed-off-by: Geert Uytterhoeven
---
drivers/gpu/drm/drm_panic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 67f78b5a76b61e3d..948aed00595eb6dd 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -91,7 +91,7 @@ static const struct drm_panic_line logo_ascii[] = {
PANIC_LINE(" \\___)=(___/"),
};
-#ifdef CONFIG_LOGO
+#if defined(CONFIG_LOGO) && !defined(MODULE)
static const struct linux_logo *logo_mono;
static int drm_panic_setup_logo(void)
--
2.34.1
Re: [PATCH 4/4] drm: rcar-du: Add support for R8A779H0
Hi Jacopo,
On Thu, Jun 20, 2024 at 6:48 PM Jacopo Mondi
wrote:
> On Thu, Jun 20, 2024 at 02:48:49PM GMT, Geert Uytterhoeven wrote:
> > On Wed, Jun 19, 2024 at 9:46 PM Laurent Pinchart
> > wrote:
> > > On Wed, Jun 19, 2024 at 12:22:18PM +0200, Jacopo Mondi wrote:
> > > > Add support for R-Car R8A779H0 V4M which has similar characteristics
> > > > as the already supported R-Car V4H R8A779G0, but with a single output
> > > > channel.
> > > >
> > > > Signed-off-by: Jacopo Mondi
> >
> > > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> > > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> > > > @@ -185,11 +187,16 @@ static void rcar_du_group_setup(struct
> > > > rcar_du_group *rgrp)
> > > > dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
> > > > rcar_du_group_write(rgrp, DORCR, dorcr);
> > > >
> > > > - /* Apply planes to CRTCs association. */
> > > > - mutex_lock(>lock);
> > > > - rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> > > > - rgrp->dptsr_planes);
> > > > - mutex_unlock(>lock);
> > > > + /*
> > > > + * Apply planes to CRTCs association, skip for V4M which has a
> > > > single
> > > > + * channel.
> > >
> > > " and doesn't implement the DPTSR register."
> > >
> > > I'm pretty sure writing it is still harmless, but...
> > >
> > > > + */
> > > > + if (rcdu->info->gen < 4 || rgrp->num_crtcs > 1) {
> >
> > Looking at the R-Car Gen3 docs, this check seems to be wrong, and the
> > lack of a check might have been an issue before?
>
> Not sure I got from your comment what part is wrong.
>
> Reading below it seems you're suggesting that writes to DPTSR should
> be skipped for some Gen3 boards as well ?
Indeed.
> > Seems like the register (per pair) is only present if the second CRTC
> > of a CRTC pair is present, so R-Car V3M and V3H (single CRTC) do not
> > have DPTSR at all, and M3-W (triple CRTC) does not have it on the
> > second pair. M3-N does have both, as it lacks the first CRTC of
> > second pair, but does have the second CRTC of the second pair.
> >
>
> /o\
>
> So far however, all Gen3 SoCs you mentioned seem to work with DPTSR
> being written and the BSP [1] only actually skips it for V4M.
I don't doubt it works, I was just reading the documentation.
Many nonexistent registers can be written zero to without ill effects...
> What would you suggesting in this case ? Addressing gen3 as well ?
> That's something that would require testing on all the above boards
> thought.
Ah, what if we could do without all this pesky testing? ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 4/4] drm: rcar-du: Add support for R8A779H0
Hi Laurent, Jacopo,
On Wed, Jun 19, 2024 at 9:46 PM Laurent Pinchart
wrote:
> On Wed, Jun 19, 2024 at 12:22:18PM +0200, Jacopo Mondi wrote:
> > Add support for R-Car R8A779H0 V4M which has similar characteristics
> > as the already supported R-Car V4H R8A779G0, but with a single output
> > channel.
> >
> > Signed-off-by: Jacopo Mondi
> > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> > @@ -185,11 +187,16 @@ static void rcar_du_group_setup(struct rcar_du_group
> > *rgrp)
> > dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
> > rcar_du_group_write(rgrp, DORCR, dorcr);
> >
> > - /* Apply planes to CRTCs association. */
> > - mutex_lock(>lock);
> > - rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> > - rgrp->dptsr_planes);
> > - mutex_unlock(>lock);
> > + /*
> > + * Apply planes to CRTCs association, skip for V4M which has a single
> > + * channel.
>
> " and doesn't implement the DPTSR register."
>
> I'm pretty sure writing it is still harmless, but...
>
> > + */
> > + if (rcdu->info->gen < 4 || rgrp->num_crtcs > 1) {
Looking at the R-Car Gen3 docs, this check seems to be wrong, and the
lack of a check might have been an issue before?
Seems like the register (per pair) is only present if the second CRTC
of a CRTC pair is present, so R-Car V3M and V3H (single CRTC) do not
have DPTSR at all, and M3-W (triple CRTC) does not have it on the
second pair. M3-N does have both, as it lacks the first CRTC of
second pair, but does have the second CRTC of the second pair.
> > + mutex_lock(>lock);
> > + rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> > + rgrp->dptsr_planes);
> > + mutex_unlock(>lock);
> > + }
> > }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] fbdev: c2p_planar: add missing MODULE_DESCRIPTION() macro
On Tue, Jun 18, 2024 at 5:05 AM Jeff Johnson wrote:
> With ARCH=m68k, make allmodconfig && make W=1 C=1 reports:
> WARNING: modpost: missing MODULE_DESCRIPTION() in
> drivers/video/fbdev/c2p_planar.o
>
> Add the missing invocation of the MODULE_DESCRIPTION() macro.
>
> Signed-off-by: Jeff Johnson
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] fbdev: amifb: add missing MODULE_DESCRIPTION() macro
On Tue, Jun 18, 2024 at 5:14 AM Jeff Johnson wrote:
> With ARCH=m68k, make allmodconfig && make W=1 C=1 reports:
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/video/fbdev/amifb.o
>
> Add the missing invocation of the MODULE_DESCRIPTION() macro.
>
> Signed-off-by: Jeff Johnson
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] drm/fbdev-dma: Only set smem_start is enable per module option
Hi Thomas,
On Mon, 17 Jun 2024, Thomas Zimmermann wrote:
Only export struct fb_info.fix.smem_start if that is required by the
user and the memory does not come from vmalloc().
Setting struct fb_info.fix.smem_start breaks systems where DMA
memory is backed by vmalloc address space. An example error is
shown below.
[3.536043] [ cut here ]
[3.540716] virt_to_phys used for non-linear address: 7fc4f540
(0x800086001000)
[3.552628] WARNING: CPU: 4 PID: 61 at arch/arm64/mm/physaddr.c:12
__virt_to_phys+0x68/0x98
[3.565455] Modules linked in:
[3.568525] CPU: 4 PID: 61 Comm: kworker/u12:5 Not tainted
6.6.23-06226-g4986cc3e1b75-dirty #250
[3.577310] Hardware name: NXP i.MX95 19X19 board (DT)
[3.582452] Workqueue: events_unbound deferred_probe_work_func
[3.588291] pstate: 6049 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[3.595233] pc : __virt_to_phys+0x68/0x98
[3.599246] lr : __virt_to_phys+0x68/0x98
[3.603276] sp : 800083603990
[3.677939] Call trace:
[3.680393] __virt_to_phys+0x68/0x98
[3.684067] drm_fbdev_dma_helper_fb_probe+0x138/0x238
[3.689214] __drm_fb_helper_initial_config_and_unlock+0x2b0/0x4c0
[3.695385] drm_fb_helper_initial_config+0x4c/0x68
[3.700264] drm_fbdev_dma_client_hotplug+0x8c/0xe0
[3.705161] drm_client_register+0x60/0xb0
[3.709269] drm_fbdev_dma_setup+0x94/0x148
Additionally, DMA memory is assumed to by contiguous in physical
address space, which is not guaranteed by vmalloc().
Resolve this by checking the module flag drm_leak_fbdev_smem when
DRM allocated the instance of struct fb_info. Fbdev-dma then only
sets smem_start only if required (via FBINFO_HIDE_SMEM_START). Also
guarantee that the framebuffer is not located in vmalloc address
space.
Signed-off-by: Thomas Zimmermann
Reported-by: Peng Fan (OSS)
Closes:
https://lore.kernel.org/dri-devel/20240604080328.4024838-1-peng@oss.nxp.com/
Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for GEM DMA
helpers")
Thanks, this fixes the issue I was seeing on R-Car Gen3/Gen4 with rcar-du.
No regressions on R-Car Gen2 (rcar-du) and R-Mobile A1 (shmobile)
which didn't shown the warning in the first place.
Tested-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 28/43] drm/renesas/rcar-du: Use fbdev-dma
Hi Thomas,
On Fri, Apr 19, 2024 at 10:34 AM Thomas Zimmermann wrote:
> Implement fbdev emulation with fbdev-dma. Fbdev-dma now supports
> damage handling, which is required by rcar-du. Avoids the overhead of
> fbdev-generic's additional shadow buffering. No functional changes.
>
> Signed-off-by: Thomas Zimmermann
> Cc: Laurent Pinchart
> Cc: Kieran Bingham
> Reviewed-by: Laurent Pinchart
Thanks for your patch, which is now commit b3fdbd60d35ce340
("drm/renesas/rcar-du: Use fbdev-dma") in drm-misc/drm-misc-next.
Probably this doesn't come as a surprise, but with CONFIG_DEBUG_VIRTUAL=y,
this triggers the following warning on R-Car Gen3/Gen4 (arm64),
e.g. on White-Hawk:
virt_to_phys used for non-linear address: (ptrval)
(0xffc088001000)
WARNING: CPU: 0 PID: 44 at arch/arm64/mm/physaddr.c:12
__virt_to_phys+0x38/0x70
Modules linked in:
CPU: 0 PID: 44 Comm: kworker/u17:2 Not tainted
6.9.0-rc6-white-hawk-01422-gb3fdbd60d35c-dirty #283
Hardware name: Renesas White Hawk CPU and Breakout boards based on
r8a779g0 (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 6005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __virt_to_phys+0x38/0x70
lr : __virt_to_phys+0x38/0x70
sp : ffc08297b930
x29: ffc08297b930 x28: ff844527c000 x27: ffc080cb6a47
x26: ffc0810c x25: ff84452a7800 x24: ff8445370018
x23: ff8443d34480 x22: ff8443d32c00 x21: ffc08297ba30
x20: ff84452a5800 x19: ffc088001000 x18:
x17: 78302820295f x16: 5f5f5f6c61767274 x15: 0720072007200720
x14: 0720072007200720 x13: 0720072007200720 x12: 0720072007200720
x11: 0180 x10: ffc0810e9aa0 x9 : ffc0813a9c80
x8 : ffc08297b638 x7 : ffc08297b640 x6 : 7fff
x5 : c0007fff x4 : x3 : 0001
x2 : x1 : x0 : ff8441ba2940
Call trace:
__virt_to_phys+0x38/0x70
drm_fbdev_dma_helper_fb_probe+0x178/0x1e8
__drm_fb_helper_initial_config_and_unlock+0x26c/0x4b8
drm_fb_helper_initial_config+0x30/0x44
drm_fbdev_dma_client_hotplug+0x84/0xb4
drm_client_register+0x74/0xb8
drm_fbdev_dma_setup+0x118/0x11c
rcar_du_probe+0x160/0x174
platform_probe+0x64/0xb0
really_probe+0x130/0x260
__driver_probe_device+0xec/0x104
driver_probe_device+0x4c/0xf8
__device_attach_driver+0xa8/0xc8
bus_for_each_drv+0xa4/0xc8
__device_attach+0xe4/0x144
device_initial_probe+0x10/0x18
bus_probe_device+0x38/0xa0
deferred_probe_work_func+0xb8/0xd0
process_scheduled_works+0x314/0x4d4
worker_thread+0x1b8/0x20c
kthread+0xd8/0xe8
ret_from_fork+0x10/0x20
irq event stamp: 7568
hardirqs last enabled at (7567): []
_raw_spin_unlock_irq+0x2c/0x40
hardirqs last disabled at (7568): []
__schedule+0x1cc/0x868
softirqs last enabled at (5004): []
__do_softirq+0x1ac/0x3a8
softirqs last disabled at (4999): []
do_softirq+0xc/0x14
Interestingly, the warning is not triggered on R-Car Gen2 (arm32),
although arch/arm/mm/physaddr.c has a similar check.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 7/7] drm/panic: Add support for drawing a monochrome graphical logo
Hi Jocelyn,
On Mon, Jun 17, 2024 at 11:59 AM Jocelyn Falempe wrote:
> On 13/06/2024 21:18, Geert Uytterhoeven wrote:
> > Re-use the existing support for boot-up logos to draw a monochrome
> > graphical logo in the DRM panic handler. When no suitable graphical
> > logo is available, the code falls back to the ASCII art penguin logo.
> >
> > Note that all graphical boot-up logos are freed during late kernel
> > initialization, hence a copy must be made for later use.
> >
> > Signed-off-by: Geert Uytterhoeven
> > --- a/drivers/gpu/drm/drm_panic.c
> > +++ b/drivers/gpu/drm/drm_panic.c
> > PANIC_LINE(" \\___)=(___/"),
> > };
> >
> > +#ifdef CONFIG_LOGO
> > +static const struct linux_logo *logo_mono;
> > +
> > +static int drm_panic_setup_logo(void)
> > +{
> > + const struct linux_logo *logo = fb_find_logo(1);
> > + const unsigned char *logo_data;
> > + struct linux_logo *logo_dup;
> > +
> > + if (!logo || logo->type != LINUX_LOGO_MONO)
> > + return 0;
> > +
> > + /* The logo is __init, so we must make a copy for later use */
> > + logo_data = kmemdup(logo->data,
> > + size_mul(DIV_ROUND_UP(logo->width,
> > BITS_PER_BYTE), logo->height),
> > + GFP_KERNEL);
> > + if (!logo_data)
> > + return -ENOMEM;
> > +
> > + logo_dup = kmemdup(logo, sizeof(*logo), GFP_KERNEL);
> > + if (!logo_dup) {
> > + kfree(logo_data);
> > + return -ENOMEM;
> > + }
> > +
> > + logo_dup->data = logo_data;
> > + logo_mono = logo_dup;
> > +
> > + return 0;
> > +}
> > +
> > +device_initcall(drm_panic_setup_logo);
> > +#else
> > +#define logo_mono((const struct linux_logo *)NULL)
> > +#endif
>
> I didn't notice that the first time, but the core drm can be built as a
> module.
> That means this will leak memory each time the module is removed.
While I hadn't considered a modular DRM core, there is no memory leak:
after the logos are freed (from late_initcall_sync()), fb_find_logo()
returns NULL. This does mean there won't be a graphical logo on panic,
though.
> But to solve the circular dependency between drm_kms_helper and
> drm_panic, one solution is to depends on drm core to be built-in.
> In this case there won't be a leak.
> So depending on how we solve the circular dependency, it can be acceptable.
So far there is no reason to select DRM_KMS_HELPER, right?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] drm/panic: depends on !VT_CONSOLE
Hi Jocelyn,
On Mon, Jun 17, 2024 at 10:16 AM Jocelyn Falempe wrote:
> On 16/06/2024 14:43, Javier Martinez Canillas wrote:
> > Jocelyn Falempe writes:
> >> The race condition between fbcon and drm_panic can only occurs if
> >> VT_CONSOLE is set. So update drm_panic dependency accordingly.
> >> This will make it easier for Linux distributions to enable drm_panic
> >> by disabling VT_CONSOLE, and keeping fbcon terminal.
> >> The only drawback is that fbcon won't display the boot kmsg, so it
> >> should rely on userspace to do that.
> >> At least plymouth already handle this case with
> >> https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/224
> >>
> >> Suggested-by: Daniel Vetter
> >> Signed-off-by: Jocelyn Falempe
> >> ---
> >> drivers/gpu/drm/Kconfig | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> >> index a9df94291622..f5c989aed7e9 100644
> >> --- a/drivers/gpu/drm/Kconfig
> >> +++ b/drivers/gpu/drm/Kconfig
> >> @@ -107,7 +107,7 @@ config DRM_KMS_HELPER
> >>
> >> config DRM_PANIC
> >> bool "Display a user-friendly message when a kernel panic occurs"
> >> -depends on DRM && !FRAMEBUFFER_CONSOLE
> >> +depends on DRM && !(FRAMEBUFFER_CONSOLE && VT_CONSOLE)
> >
> > I thought the idea was to only make it depend on !VT_CONSOLE, so that
> > distros could also enable fbcon / VT but prevent the race condition to
> > happen due the VT not being a system console for the kernel to print
> > messages ?
>
> Yes, but when writing the patch, I thought that if you have VT_CONSOLE=y
> and FRAMEBUFFER_CONSOLE=n, then there won't be any race condition, and
> drm_panic can be enabled safely.
> I don't know if that really matters, and if VT_CONSOLE has any usage
> apart from fbcon.
It is used for any kind of virtual terminal, so also for vgacon.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 5/7] drm/panic: Convert to drm_fb_clip_offset()
On Sun, Jun 16, 2024 at 11:08 AM Geert Uytterhoeven
wrote:
> On Sat, Jun 15, 2024 at 12:55 PM kernel test robot wrote:
> > kernel test robot noticed the following build errors:
> >
> > [auto build test ERROR on drm-misc/drm-misc-next]
> > [cannot apply to linus/master v6.10-rc3 next-20240613]
> > [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/Geert-Uytterhoeven/drm-panic-Fix-uninitialized-drm_scanout_buffer-set_pixel-crash/20240614-032053
> > base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> > patch link:
> > https://lore.kernel.org/r/3121082eb4beb461773ebb6f656ed9b4286967ee.1718305355.git.geert%2Brenesas%40glider.be
> > patch subject: [PATCH v2 5/7] drm/panic: Convert to drm_fb_clip_offset()
> > config: x86_64-randconfig-003-20240615
> > (https://download.01.org/0day-ci/archive/20240615/202406151811.yeiz6203-...@intel.com/config)
> > compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
> > reproduce (this is a W=1 build):
> > (https://download.01.org/0day-ci/archive/20240615/202406151811.yeiz6203-...@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new
> > version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot
> > | Closes:
> > https://lore.kernel.org/oe-kbuild-all/202406151811.yeiz6203-...@intel.com/
> >
> > All errors (new ones prefixed by >>):
> >
> > >> depmod: ERROR: Cycle detected: drm -> drm_kms_helper -> drm
> >depmod: ERROR: Found 2 modules in dependency cycles!
>
> Oops, so DRM core cannot call any of the helpers, and DRM_PANIC
> selecting DRM_KMS_HELPER was wrong in the first place?
Q: So how does this work with DRM_PANIC calling
drm_fb_helper_emergency_disable()?
A: drm_fb_helper_emergency_disable() is a dummy if
!CONFIG_DRM_FBDEV_EMULATION, so I guess no one tried to build
a failing randconfig with CONFIG_DRM_FBDEV_EMULATION=y yet.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 5/7] drm/panic: Convert to drm_fb_clip_offset()
On Sat, Jun 15, 2024 at 12:55 PM kernel test robot wrote:
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on drm-misc/drm-misc-next]
> [cannot apply to linus/master v6.10-rc3 next-20240613]
> [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/Geert-Uytterhoeven/drm-panic-Fix-uninitialized-drm_scanout_buffer-set_pixel-crash/20240614-032053
> base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> patch link:
> https://lore.kernel.org/r/3121082eb4beb461773ebb6f656ed9b4286967ee.1718305355.git.geert%2Brenesas%40glider.be
> patch subject: [PATCH v2 5/7] drm/panic: Convert to drm_fb_clip_offset()
> config: x86_64-randconfig-003-20240615
> (https://download.01.org/0day-ci/archive/20240615/202406151811.yeiz6203-...@intel.com/config)
> compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
> reproduce (this is a W=1 build):
> (https://download.01.org/0day-ci/archive/20240615/202406151811.yeiz6203-...@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version
> of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot
> | Closes:
> https://lore.kernel.org/oe-kbuild-all/202406151811.yeiz6203-...@intel.com/
>
> All errors (new ones prefixed by >>):
>
> >> depmod: ERROR: Cycle detected: drm -> drm_kms_helper -> drm
>depmod: ERROR: Found 2 modules in dependency cycles!
Oops, so DRM core cannot call any of the helpers, and DRM_PANIC
selecting DRM_KMS_HELPER was wrong in the first place?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 7/7] drm/panic: Add support for drawing a monochrome graphical logo
Hi Jocelyn,
On Fri, Jun 14, 2024 at 11:55 AM Jocelyn Falempe wrote:
> On 13/06/2024 21:18, Geert Uytterhoeven wrote:
> > Re-use the existing support for boot-up logos to draw a monochrome
> > graphical logo in the DRM panic handler. When no suitable graphical
> > logo is available, the code falls back to the ASCII art penguin logo.
> >
> > Note that all graphical boot-up logos are freed during late kernel
> > initialization, hence a copy must be made for later use.
>
> Would it be possible to have the logo not in the __init section if
> DRM_PANIC is set ?
That would be rather complicated. The C source files for the logos
(there can be multiple) are generated by drivers/video/logo/pnmtologo.c.
> The patch looks good to me anyway.
>
> Reviewed-by: Jocelyn Falempe
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 0/3] drm/panic: Fixes and graphical logo
Hi Maxime,
On Fri, Jun 14, 2024 at 10:35 AM Maxime Ripard wrote:
> On Fri, Jun 14, 2024 at 08:58:26AM GMT, Geert Uytterhoeven wrote:
> > Looks like the top commit of last drm-misc PR [1] is part of the
> > drm-misc-next branch. but not of the for-linux-next branch, while
> > Stephen pulls the latter.
> > Is that a drm-misc or a linux-next issue?
>
> This was a drm-misc issue, it should now be solved
Thanks, confirmed!
(and updated my .git/config for next renesas-drivers release again ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 0/3] drm/panic: Fixes and graphical logo
Hi Stephen and Maxime,
On Fri, Jun 14, 2024 at 12:00 AM Stephen Rothwell wrote:
> On Thu, 13 Jun 2024 11:48:15 +0200 Geert Uytterhoeven
> wrote:
> > > > Has the drm-misc git repo moved?
> > >
> > > It moved to gitlab recently, the new url is
> > > g...@gitlab.freedesktop.org:drm/misc/kernel.git
> >
> > Time to tell Stephen...
>
> linux-next has been using that URL for some time.
OK.
Looks like the top commit of last drm-misc PR [1] is part of the
drm-misc-next branch. but not of the for-linux-next branch, while
Stephen pulls the latter.
Is that a drm-misc or a linux-next issue?
Thanks!
[1] a13aaf157467e694a3824d81304106b58d4c20d6
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH v2 7/7] drm/panic: Add support for drawing a monochrome graphical logo
Re-use the existing support for boot-up logos to draw a monochrome
graphical logo in the DRM panic handler. When no suitable graphical
logo is available, the code falls back to the ASCII art penguin logo.
Note that all graphical boot-up logos are freed during late kernel
initialization, hence a copy must be made for later use.
Signed-off-by: Geert Uytterhoeven
---
v2:
- Rebased,
- Inline trivial draw_logo_mono().
---
drivers/gpu/drm/drm_panic.c | 65 +
drivers/video/logo/Kconfig | 2 ++
2 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index f7e22b69bb25d3be..af30f243b2802ad7 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -7,11 +7,15 @@
*/
#include
+#include
#include
#include
#include
+#include
#include
+#include
#include
+#include
#include
#include
@@ -88,6 +92,42 @@ static const struct drm_panic_line logo_ascii[] = {
PANIC_LINE(" \\___)=(___/"),
};
+#ifdef CONFIG_LOGO
+static const struct linux_logo *logo_mono;
+
+static int drm_panic_setup_logo(void)
+{
+ const struct linux_logo *logo = fb_find_logo(1);
+ const unsigned char *logo_data;
+ struct linux_logo *logo_dup;
+
+ if (!logo || logo->type != LINUX_LOGO_MONO)
+ return 0;
+
+ /* The logo is __init, so we must make a copy for later use */
+ logo_data = kmemdup(logo->data,
+ size_mul(DIV_ROUND_UP(logo->width, BITS_PER_BYTE),
logo->height),
+ GFP_KERNEL);
+ if (!logo_data)
+ return -ENOMEM;
+
+ logo_dup = kmemdup(logo, sizeof(*logo), GFP_KERNEL);
+ if (!logo_dup) {
+ kfree(logo_data);
+ return -ENOMEM;
+ }
+
+ logo_dup->data = logo_data;
+ logo_mono = logo_dup;
+
+ return 0;
+}
+
+device_initcall(drm_panic_setup_logo);
+#else
+#define logo_mono ((const struct linux_logo *)NULL)
+#endif
+
/*
* Color conversion
*/
@@ -452,15 +492,22 @@ static void draw_panic_static_user(struct
drm_scanout_buffer *sb)
u32 bg_color = convert_from_xrgb(CONFIG_DRM_PANIC_BACKGROUND_COLOR,
sb->format->format);
const struct font_desc *font = get_default_font(sb->width, sb->height,
NULL, NULL);
struct drm_rect r_screen, r_logo, r_msg;
+ unsigned int logo_width, logo_height;
if (!font)
return;
r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
- r_logo = DRM_RECT_INIT(0, 0,
- get_max_line_len(logo_ascii, logo_ascii_lines) *
font->width,
- logo_ascii_lines * font->height);
+ if (logo_mono) {
+ logo_width = logo_mono->width;
+ logo_height = logo_mono->height;
+ } else {
+ logo_width = get_max_line_len(logo_ascii, logo_ascii_lines) *
font->width;
+ logo_height = logo_ascii_lines * font->height;
+ }
+
+ r_logo = DRM_RECT_INIT(0, 0, logo_width, logo_height);
r_msg = DRM_RECT_INIT(0, 0,
min(get_max_line_len(panic_msg, msg_lines) *
font->width, sb->width),
min(msg_lines * font->height, sb->height));
@@ -471,10 +518,14 @@ static void draw_panic_static_user(struct
drm_scanout_buffer *sb)
/* Fill with the background color, and draw text on top */
drm_panic_fill(sb, _screen, bg_color);
- if ((r_msg.x1 >= drm_rect_width(_logo) || r_msg.y1 >=
drm_rect_height(_logo)) &&
- drm_rect_width(_logo) <= sb->width && drm_rect_height(_logo) <=
sb->height) {
- draw_txt_rectangle(sb, font, logo_ascii, logo_ascii_lines,
false, _logo,
- fg_color);
+ if ((r_msg.x1 >= logo_width || r_msg.y1 >= logo_height) &&
+ logo_width <= sb->width && logo_height <= sb->height) {
+ if (logo_mono)
+ drm_panic_blit(sb, _logo, logo_mono->data,
DIV_ROUND_UP(logo_width, 8),
+ fg_color);
+ else
+ draw_txt_rectangle(sb, font, logo_ascii,
logo_ascii_lines, false, _logo,
+ fg_color);
}
draw_txt_rectangle(sb, font, panic_msg, msg_lines, true, _msg,
fg_color);
}
diff --git a/drivers/video/logo/Kconfig b/drivers/video/logo/Kconfig
index b7d94d1dd1585a84..ce6bb753522d215d 100644
--- a/drivers/video/logo/Kconfig
+++ b/drivers/video/logo/Kconfig
@@ -8,6 +8,8 @@ menuconfig LOGO
depends on FB_CORE || SGI_NEWPORT_CONSOLE
help
Enable and select frame buffer bootup logos.
+ Monochrome logos will also be used by the DRM panic handler, if
+ enabled.
if LOGO
--
2.34.1
[PATCH v2 4/7] drm/panic: Spelling s/formater/formatter/
Fix a misspelling of "formatter".
Fixes: 54034bebb22fd4be ("drm/panic: Add a kmsg panic screen")
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
drivers/gpu/drm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index a9972ce05d7e6fe4..e3c51009d9b476b3 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -138,7 +138,7 @@ config DRM_PANIC_DEBUG
If in doubt, say "N".
config DRM_PANIC_SCREEN
- string "Panic screen formater"
+ string "Panic screen formatter"
default "user"
depends on DRM_PANIC
help
--
2.34.1
[PATCH v2 1/7] drm/panic: Fix uninitialized drm_scanout_buffer.set_pixel() crash
No implementations of drm_plane_helper_funcs.get_scanout_buffer() fill
in the optional drm_scanout_buffer.set_pixel() member. Hence the member
may contain non-zero garbage, causing a crash when deferencing it during
drm panic.
Fix this by pre-initializing the drm_scanout_buffer object before
calling drm_plane_helper_funcs.get_scanout_buffer().
Fixes: 24d07f114e4ec760 ("drm/panic: Add a set_pixel() callback to
drm_scanout_buffer")
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
drivers/gpu/drm/drm_panic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 293d4dcbc80da7ba..fc04ed4e0b399f55 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -582,7 +582,7 @@ static void draw_panic_dispatch(struct drm_scanout_buffer
*sb)
static void draw_panic_plane(struct drm_plane *plane)
{
- struct drm_scanout_buffer sb;
+ struct drm_scanout_buffer sb = { };
int ret;
unsigned long flags;
--
2.34.1
[PATCH v2 6/7] drm/panic: Rename logo to logo_ascii
Rename variables related to the ASCII logo, to prepare for the advent of
support for graphical logos.
Signed-off-by: Geert Uytterhoeven
---
v2:
- Rebased.
---
drivers/gpu/drm/drm_panic.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 5b0acf8c86e402a8..f7e22b69bb25d3be 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -78,7 +78,7 @@ static struct drm_panic_line panic_msg[] = {
PANIC_LINE("Please reboot your computer."),
};
-static const struct drm_panic_line logo[] = {
+static const struct drm_panic_line logo_ascii[] = {
PANIC_LINE(" .--._"),
PANIC_LINE("|o_o | | |"),
PANIC_LINE("|:_/ | | |"),
@@ -447,7 +447,7 @@ static void draw_txt_rectangle(struct drm_scanout_buffer
*sb,
static void draw_panic_static_user(struct drm_scanout_buffer *sb)
{
size_t msg_lines = ARRAY_SIZE(panic_msg);
- size_t logo_lines = ARRAY_SIZE(logo);
+ size_t logo_ascii_lines = ARRAY_SIZE(logo_ascii);
u32 fg_color = convert_from_xrgb(CONFIG_DRM_PANIC_FOREGROUND_COLOR,
sb->format->format);
u32 bg_color = convert_from_xrgb(CONFIG_DRM_PANIC_BACKGROUND_COLOR,
sb->format->format);
const struct font_desc *font = get_default_font(sb->width, sb->height,
NULL, NULL);
@@ -459,8 +459,8 @@ static void draw_panic_static_user(struct
drm_scanout_buffer *sb)
r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
r_logo = DRM_RECT_INIT(0, 0,
- get_max_line_len(logo, logo_lines) * font->width,
- logo_lines * font->height);
+ get_max_line_len(logo_ascii, logo_ascii_lines) *
font->width,
+ logo_ascii_lines * font->height);
r_msg = DRM_RECT_INIT(0, 0,
min(get_max_line_len(panic_msg, msg_lines) *
font->width, sb->width),
min(msg_lines * font->height, sb->height));
@@ -473,7 +473,8 @@ static void draw_panic_static_user(struct
drm_scanout_buffer *sb)
if ((r_msg.x1 >= drm_rect_width(_logo) || r_msg.y1 >=
drm_rect_height(_logo)) &&
drm_rect_width(_logo) <= sb->width && drm_rect_height(_logo) <=
sb->height) {
- draw_txt_rectangle(sb, font, logo, logo_lines, false, _logo,
fg_color);
+ draw_txt_rectangle(sb, font, logo_ascii, logo_ascii_lines,
false, _logo,
+ fg_color);
}
draw_txt_rectangle(sb, font, panic_msg, msg_lines, true, _msg,
fg_color);
}
--
2.34.1
[PATCH v2 0/7] drm/panic: Fixes and graphical logo
Hi all,
If drm/panic is enabled, a user-friendly message is shown on screen when
a kernel panic occurs, together with an ASCII art penguin logo.
Of course we can do better ;-)
Hence this patch series extends drm/panic to draw the monochrome
graphical boot logo, when available, preceded by the customary fixes.
Changes compared to v1:
- Rebase against today's drm-misc-next, where drm_panic is broken on
all current drivers due to an uninitialized pointer dereference.
Presumably this was only tested with an out-of-tree driver change?
- New fixes [1/7], [3/7], and [4/7],
- New cleanup [5/7],
- Inline trivial draw_logo_mono().
This has been tested with rcar-du.
Thanks for your comments!
Geert Uytterhoeven (7):
drm/panic: Fix uninitialized drm_scanout_buffer.set_pixel() crash
drm/panic: Fix off-by-one logo size checks
lib/fonts: Fix visiblity of SUN12x22 and TER16x32 if DRM_PANIC
drm/panic: Spelling s/formater/formatter/
drm/panic: Convert to drm_fb_clip_offset()
drm/panic: Rename logo to logo_ascii
drm/panic: Add support for drawing a monochrome graphical logo
drivers/gpu/drm/Kconfig | 2 +-
drivers/gpu/drm/drm_panic.c | 74 +++--
drivers/video/logo/Kconfig | 2 +
lib/fonts/Kconfig | 6 ++-
4 files changed, 70 insertions(+), 14 deletions(-)
--
2.34.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH v2 5/7] drm/panic: Convert to drm_fb_clip_offset()
Use the drm_fb_clip_offset() helper instead of open-coding the same
operation.
Signed-off-by: Geert Uytterhoeven
---
DRM_PANIC already selects DRM_KMS_HELPER.
v2:
- New.
---
drivers/gpu/drm/drm_panic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 814ef5c20c08ee42..5b0acf8c86e402a8 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -285,7 +285,7 @@ static void drm_panic_blit(struct drm_scanout_buffer *sb,
struct drm_rect *clip,
return drm_panic_blit_pixel(sb, clip, sbuf8, spitch, fg_color);
map = sb->map[0];
- iosys_map_incr(, clip->y1 * sb->pitch[0] + clip->x1 *
sb->format->cpp[0]);
+ iosys_map_incr(, drm_fb_clip_offset(sb->pitch[0], sb->format,
clip));
switch (sb->format->cpp[0]) {
case 2:
@@ -373,7 +373,7 @@ static void drm_panic_fill(struct drm_scanout_buffer *sb,
struct drm_rect *clip,
return drm_panic_fill_pixel(sb, clip, color);
map = sb->map[0];
- iosys_map_incr(, clip->y1 * sb->pitch[0] + clip->x1 *
sb->format->cpp[0]);
+ iosys_map_incr(, drm_fb_clip_offset(sb->pitch[0], sb->format,
clip));
switch (sb->format->cpp[0]) {
case 2:
--
2.34.1
[PATCH v2 3/7] lib/fonts: Fix visiblity of SUN12x22 and TER16x32 if DRM_PANIC
When CONFIG_FONTS ("Select compiled-in fonts") is not enabled, the user
should not be asked about any fonts. However, when CONFIG_DRM_PANIC is
enabled, the user is still asked about the Sparc console 12x22 and
Terminus 16x32 fonts.
Fix this by moving the "|| DRM_PANIC" to where it belongs.
Split the dependency in two rules to improve readability.
Fixes: b94605a3889b9084 ("lib/fonts: Allow to select fonts for drm_panic")
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
lib/fonts/Kconfig | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/fonts/Kconfig b/lib/fonts/Kconfig
index befcb463f7381d1a..3ac26bdbc3ff01a3 100644
--- a/lib/fonts/Kconfig
+++ b/lib/fonts/Kconfig
@@ -105,7 +105,8 @@ config FONT_SUN8x16
config FONT_SUN12x22
bool "Sparc console 12x22 font (not supported by all drivers)"
- depends on (FRAMEBUFFER_CONSOLE && (!SPARC && FONTS || SPARC)) ||
DRM_PANIC
+ depends on FRAMEBUFFER_CONSOLE || DRM_PANIC
+ depends on !SPARC && FONTS
help
This is the high resolution console font for Sun machines with very
big letters (like the letters used in the SPARC PROM). If the
@@ -113,7 +114,8 @@ config FONT_SUN12x22
config FONT_TER16x32
bool "Terminus 16x32 font (not supported by all drivers)"
- depends on (FRAMEBUFFER_CONSOLE && (!SPARC && FONTS || SPARC)) ||
DRM_PANIC
+ depends on FRAMEBUFFER_CONSOLE || DRM_PANIC
+ depends on !SPARC && FONTS || SPARC
help
Terminus Font is a clean, fixed width bitmap font, designed
for long (8 and more hours per day) work with computers.
--
2.34.1
[PATCH v2 2/7] drm/panic: Fix off-by-one logo size checks
Logos that are either just as wide or just as high as the display work
fine.
Fixes: bf9fb17c6672868d ("drm/panic: Add a drm panic handler")
Signed-off-by: Geert Uytterhoeven
---
v2:
- Rebased.
---
drivers/gpu/drm/drm_panic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index fc04ed4e0b399f55..814ef5c20c08ee42 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -472,7 +472,7 @@ static void draw_panic_static_user(struct
drm_scanout_buffer *sb)
drm_panic_fill(sb, _screen, bg_color);
if ((r_msg.x1 >= drm_rect_width(_logo) || r_msg.y1 >=
drm_rect_height(_logo)) &&
- drm_rect_width(_logo) < sb->width && drm_rect_height(_logo) <
sb->height) {
+ drm_rect_width(_logo) <= sb->width && drm_rect_height(_logo) <=
sb->height) {
draw_txt_rectangle(sb, font, logo, logo_lines, false, _logo,
fg_color);
}
draw_txt_rectangle(sb, font, panic_msg, msg_lines, true, _msg,
fg_color);
--
2.34.1
Re: [PATCH 0/3] drm/panic: Fixes and graphical logo
Hi Jocelyn,
CC sfr
On Thu, Jun 13, 2024 at 11:41 AM Jocelyn Falempe wrote:
> On 13/06/2024 11:32, Geert Uytterhoeven wrote:
> > On Thu, Jun 13, 2024 at 10:38 AM Jocelyn Falempe
> > wrote:
> >> On 12/06/2024 15:54, Geert Uytterhoeven wrote:
> >>> If drm/panic is enabled, a user-friendly message is shown on screen when
> >>> a kernel panic occurs, together with an ASCII art penguin logo.
> >>> Of course we can do better ;-)
> >>> Hence this patch series extends drm/panic to draw the monochrome
> >>> graphical boot logo, when available, preceded by the customary fix.
> >>
> >> Thanks for your patch.
> >>
> >> I've tested it, and it works great.
> >
> > Thank you!
> >
> >> You need to rebase your series on top of drm-misc-next, because it
> >> conflicts with a series I pushed last week:
> >> https://patchwork.freedesktop.org/series/134286/
> >
> > I had seen that you said you had pushed this to drm-misc-next[1]
> > before I posted my series, but couldn't find the actual commits in
> > drm-misc/for-linux-next, which is still at commit dfc1209ed5a3861c
> > ("arm/komeda: Remove all CONFIG_DEBUG_FS conditional compilations",
> > so I assumed you just forgot to push?
> > However, the latest pull request[2] does include them, while linux-next
> > does not.
> >
> > Has the drm-misc git repo moved?
>
> It moved to gitlab recently, the new url is
> g...@gitlab.freedesktop.org:drm/misc/kernel.git
Time to tell Stephen...
> and the drm_panic kmsg screen commit is there:
>
> https://gitlab.freedesktop.org/drm/misc/kernel/-/commits/drm-misc-next?ref_type=heads
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 0/3] drm/panic: Fixes and graphical logo
Hi Jocelyn,
On Thu, Jun 13, 2024 at 10:38 AM Jocelyn Falempe wrote:
> On 12/06/2024 15:54, Geert Uytterhoeven wrote:
> > If drm/panic is enabled, a user-friendly message is shown on screen when
> > a kernel panic occurs, together with an ASCII art penguin logo.
> > Of course we can do better ;-)
> > Hence this patch series extends drm/panic to draw the monochrome
> > graphical boot logo, when available, preceded by the customary fix.
>
> Thanks for your patch.
>
> I've tested it, and it works great.
Thank you!
> You need to rebase your series on top of drm-misc-next, because it
> conflicts with a series I pushed last week:
> https://patchwork.freedesktop.org/series/134286/
I had seen that you said you had pushed this to drm-misc-next[1]
before I posted my series, but couldn't find the actual commits in
drm-misc/for-linux-next, which is still at commit dfc1209ed5a3861c
("arm/komeda: Remove all CONFIG_DEBUG_FS conditional compilations",
so I assumed you just forgot to push?
However, the latest pull request[2] does include them, while linux-next
does not.
Has the drm-misc git repo moved?
Thanks!
[1] https://lore.kernel.org/all/3649ff15-df2b-49ba-920f-c418355d7...@redhat.com/
[2] "[PULL] drm-misc-next"
https://lore.kernel.org/all/20240613-cicada-of-infinite-unity-0955ca@houat/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH 2/3] drm/panic: Rename logo to logo_ascii
Rename variables related to the ASCII logo, to prepare for the advent of
support for graphical logos.
Signed-off-by: Geert Uytterhoeven
---
drivers/gpu/drm/drm_panic.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 52d8a96b7dedff2c..05b406a7034bb295 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -71,7 +71,7 @@ static struct drm_panic_line panic_msg[] = {
PANIC_LINE("Please reboot your computer."),
};
-static const struct drm_panic_line logo[] = {
+static const struct drm_panic_line logo_ascii[] = {
PANIC_LINE(" .--._"),
PANIC_LINE("|o_o | | |"),
PANIC_LINE("|:_/ | | |"),
@@ -417,7 +417,7 @@ static void draw_txt_rectangle(struct drm_scanout_buffer
*sb,
static void draw_panic_static(struct drm_scanout_buffer *sb)
{
size_t msg_lines = ARRAY_SIZE(panic_msg);
- size_t logo_lines = ARRAY_SIZE(logo);
+ size_t logo_ascii_lines = ARRAY_SIZE(logo_ascii);
u32 fg_color = CONFIG_DRM_PANIC_FOREGROUND_COLOR;
u32 bg_color = CONFIG_DRM_PANIC_BACKGROUND_COLOR;
const struct font_desc *font = get_default_font(sb->width, sb->height,
NULL, NULL);
@@ -430,8 +430,8 @@ static void draw_panic_static(struct drm_scanout_buffer *sb)
bg_color = convert_from_xrgb(bg_color, sb->format->format);
r_logo = DRM_RECT_INIT(0, 0,
- get_max_line_len(logo, logo_lines) * font->width,
- logo_lines * font->height);
+ get_max_line_len(logo_ascii, logo_ascii_lines) *
font->width,
+ logo_ascii_lines * font->height);
r_msg = DRM_RECT_INIT(0, 0,
min(get_max_line_len(panic_msg, msg_lines) *
font->width, sb->width),
min(msg_lines * font->height, sb->height));
@@ -445,7 +445,8 @@ static void draw_panic_static(struct drm_scanout_buffer *sb)
if ((r_msg.x1 >= drm_rect_width(_logo) || r_msg.y1 >=
drm_rect_height(_logo)) &&
drm_rect_width(_logo) <= sb->width && drm_rect_height(_logo) <=
sb->height) {
- draw_txt_rectangle(sb, font, logo, logo_lines, false, _logo,
fg_color, bg_color);
+ draw_txt_rectangle(sb, font, logo_ascii, logo_ascii_lines,
false, _logo,
+ fg_color, bg_color);
}
draw_txt_rectangle(sb, font, panic_msg, msg_lines, true, _msg,
fg_color, bg_color);
}
--
2.34.1
[PATCH 3/3] drm/panic: Add support for drawing a monochrome graphical logo
Re-use the existing support for boot-up logos to draw a monochrome
graphical logo in the DRM panic handler. When no suitable graphical
logo is available, the code falls back to the ASCII art penguin logo.
Note that all graphical boot-up logos are freed during late kernel
initialization, hence a copy must be made for later use.
Signed-off-by: Geert Uytterhoeven
---
drivers/gpu/drm/drm_panic.c | 78 +
drivers/video/logo/Kconfig | 2 +
2 files changed, 73 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 05b406a7034bb295..f2d7484eff43f5a8 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -7,11 +7,15 @@
*/
#include
+#include
#include
#include
#include
+#include
#include
+#include
#include
+#include
#include
#include
@@ -81,6 +85,42 @@ static const struct drm_panic_line logo_ascii[] = {
PANIC_LINE(" \\___)=(___/"),
};
+#ifdef CONFIG_LOGO
+static const struct linux_logo *logo_mono;
+
+static int drm_panic_setup_logo(void)
+{
+ const struct linux_logo *logo = fb_find_logo(1);
+ const unsigned char *logo_data;
+ struct linux_logo *logo_dup;
+
+ if (!logo || logo->type != LINUX_LOGO_MONO)
+ return 0;
+
+ /* The logo is __init, so we must make a copy for later use */
+ logo_data = kmemdup(logo->data,
+ size_mul(DIV_ROUND_UP(logo->width, BITS_PER_BYTE),
logo->height),
+ GFP_KERNEL);
+ if (!logo_data)
+ return -ENOMEM;
+
+ logo_dup = kmemdup(logo, sizeof(*logo), GFP_KERNEL);
+ if (!logo_dup) {
+ kfree(logo_data);
+ return -ENOMEM;
+ }
+
+ logo_dup->data = logo_data;
+ logo_mono = logo_dup;
+
+ return 0;
+}
+
+device_initcall(drm_panic_setup_logo);
+#else
+#define logo_mono ((const struct linux_logo *)NULL)
+#endif
+
/*
* Color conversion
*/
@@ -357,6 +397,20 @@ static void drm_panic_fill(struct iosys_map *dmap,
unsigned int dpitch,
}
}
+/*
+ * Draw a monochrome logo on a framebuffer.
+ */
+static void draw_logo_mono(struct drm_scanout_buffer *sb, const struct
linux_logo *logo,
+ struct drm_rect *clip, u32 fg_color, u32 bg_color)
+{
+ unsigned int px_width = sb->format->cpp[0];
+ struct iosys_map dst = sb->map[0];
+
+ iosys_map_incr(, clip->y1 * sb->pitch[0] + clip->x1 * px_width);
+ drm_panic_blit(, sb->pitch[0], logo->data,
DIV_ROUND_UP(logo->width, 8),
+ logo->height, logo->width, fg_color, bg_color, px_width);
+}
+
static const u8 *get_char_bitmap(const struct font_desc *font, char c, size_t
font_pitch)
{
return font->data + (c * font->height) * font_pitch;
@@ -421,6 +475,7 @@ static void draw_panic_static(struct drm_scanout_buffer *sb)
u32 fg_color = CONFIG_DRM_PANIC_FOREGROUND_COLOR;
u32 bg_color = CONFIG_DRM_PANIC_BACKGROUND_COLOR;
const struct font_desc *font = get_default_font(sb->width, sb->height,
NULL, NULL);
+ unsigned int logo_width, logo_height;
struct drm_rect r_logo, r_msg;
if (!font)
@@ -429,9 +484,15 @@ static void draw_panic_static(struct drm_scanout_buffer
*sb)
fg_color = convert_from_xrgb(fg_color, sb->format->format);
bg_color = convert_from_xrgb(bg_color, sb->format->format);
- r_logo = DRM_RECT_INIT(0, 0,
- get_max_line_len(logo_ascii, logo_ascii_lines) *
font->width,
- logo_ascii_lines * font->height);
+ if (logo_mono) {
+ logo_width = logo_mono->width;
+ logo_height = logo_mono->height;
+ } else {
+ logo_width = get_max_line_len(logo_ascii, logo_ascii_lines) *
font->width;
+ logo_height = logo_ascii_lines * font->height;
+ }
+
+ r_logo = DRM_RECT_INIT(0, 0, logo_width, logo_height);
r_msg = DRM_RECT_INIT(0, 0,
min(get_max_line_len(panic_msg, msg_lines) *
font->width, sb->width),
min(msg_lines * font->height, sb->height));
@@ -443,10 +504,13 @@ static void draw_panic_static(struct drm_scanout_buffer
*sb)
drm_panic_fill(>map[0], sb->pitch[0], sb->height, sb->width,
bg_color, sb->format->cpp[0]);
- if ((r_msg.x1 >= drm_rect_width(_logo) || r_msg.y1 >=
drm_rect_height(_logo)) &&
- drm_rect_width(_logo) <= sb->width && drm_rect_height(_logo) <=
sb->height) {
- draw_txt_rectangle(sb, font, logo_ascii, logo_ascii_lines,
false, _logo,
- fg_color, bg_color);
+ if ((r_msg.x1 >= logo_width || r
[PATCH 1/3] drm/panic: Fix off-by-one logo size checks
Logos that are either just as wide or just as high as the display work
fine.
Fixes: bf9fb17c6672868d ("drm/panic: Add a drm panic handler")
Signed-off-by: Geert Uytterhoeven
---
drivers/gpu/drm/drm_panic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 7ece67086cecb79f..52d8a96b7dedff2c 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -444,7 +444,7 @@ static void draw_panic_static(struct drm_scanout_buffer *sb)
bg_color, sb->format->cpp[0]);
if ((r_msg.x1 >= drm_rect_width(_logo) || r_msg.y1 >=
drm_rect_height(_logo)) &&
- drm_rect_width(_logo) < sb->width && drm_rect_height(_logo) <
sb->height) {
+ drm_rect_width(_logo) <= sb->width && drm_rect_height(_logo) <=
sb->height) {
draw_txt_rectangle(sb, font, logo, logo_lines, false, _logo,
fg_color, bg_color);
}
draw_txt_rectangle(sb, font, panic_msg, msg_lines, true, _msg,
fg_color, bg_color);
--
2.34.1
[PATCH 0/3] drm/panic: Fixes and graphical logo
Hi all,
If drm/panic is enabled, a user-friendly message is shown on screen when
a kernel panic occurs, together with an ASCII art penguin logo.
Of course we can do better ;-)
Hence this patch series extends drm/panic to draw the monochrome
graphical boot logo, when available, preceded by the customary fix.
This has been tested with rcar-du.
Thanks for your comments!
Geert Uytterhoeven (3):
drm/panic: Fix off-by-one logo size checks
drm/panic: Rename logo to logo_ascii
drm/panic: Add support for drawing a monochrome graphical logo
drivers/gpu/drm/drm_panic.c | 81 +
drivers/video/logo/Kconfig | 2 +
2 files changed, 75 insertions(+), 8 deletions(-)
--
2.34.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH] video/logo: Remove linux_serial_image comments
The last user of the serial_console ASCII image was removed in v2.1.115. Signed-off-by: Geert Uytterhoeven --- include/linux/linux_logo.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/linux/linux_logo.h b/include/linux/linux_logo.h index d4d5b93efe8435bd..e37699b7e8393df0 100644 --- a/include/linux/linux_logo.h +++ b/include/linux/linux_logo.h @@ -10,9 +10,6 @@ * Copyright (C) 2001 Greg Banks * Copyright (C) 2001 Jan-Benedict Glaw * Copyright (C) 2003 Geert Uytterhoeven - * - * Serial_console ascii image can be any size, - * but should contain %s to display the version */ #include -- 2.34.1
[PATCH] video/logo: Make logo data const again
As gcc-4.1 is no longer supported, the logo data can be made const
again. Hence revert commit 15e3252464432a29 ("fbdev: work around old
compiler bug").
Signed-off-by: Geert Uytterhoeven
---
drivers/video/logo/pnmtologo.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/logo/pnmtologo.c b/drivers/video/logo/pnmtologo.c
index 8080c4d9c4a23fbb..28d9f0b907a99a05 100644
--- a/drivers/video/logo/pnmtologo.c
+++ b/drivers/video/logo/pnmtologo.c
@@ -238,7 +238,7 @@ static void write_header(void)
fprintf(out, " * Linux logo %s\n", logoname);
fputs(" */\n\n", out);
fputs("#include \n\n", out);
- fprintf(out, "static unsigned char %s_data[] __initdata = {\n",
+ fprintf(out, "static const unsigned char %s_data[] __initconst = {\n",
logoname);
}
@@ -375,7 +375,7 @@ static void write_logo_clut224(void)
fputs("\n};\n\n", out);
/* write logo clut */
- fprintf(out, "static unsigned char %s_clut[] __initdata = {\n",
+ fprintf(out, "static const unsigned char %s_clut[] __initconst = {\n",
logoname);
write_hex_cnt = 0;
for (i = 0; i < logo_clutsize; i++) {
--
2.34.1
[PATCH v3] drm: renesas: shmobile: Call drm_atomic_helper_shutdown() at shutdown time
From: Douglas Anderson
Based on grepping through the source code, this driver appears to be
missing a call to drm_atomic_helper_shutdown() at system shutdown time.
This is important because drm_atomic_helper_shutdown() will cause
panels to get disabled cleanly which may be important for their power
sequencing. Future changes will remove any custom powering off in
individual panel drivers so the DRM drivers need to start getting this
right.
The fact that we should call drm_atomic_helper_shutdown() in the case of
OS shutdown comes straight out of the kernel doc "driver instance
overview" in drm_drv.c.
Suggested-by: Maxime Ripard
Signed-off-by: Douglas Anderson
Link:
https://lore.kernel.org/r/20230901164111.RFT.15.Iaf638a1d4c8b3c307a6192efabb4cbb06b195f15@changeid
[geert: s/drm_helper_force_disable_all/drm_atomic_helper_shutdown/]
[geert: shmob_drm_remove() already calls drm_atomic_helper_shutdown]
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Laurent Pinchart
Reviewed-by: Sui Jingfeng
---
v3:
- Add Reviewed-by,
- Fix remaining references to drm_helper_force_disable_all(),
v2:
- Add Reviewed-by.
Tested on Atmark Techno Armadillo-800-EVA.
---
drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
index e83c3e52251dedf9..0250d5f00bf102dc 100644
--- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
@@ -171,6 +171,13 @@ static void shmob_drm_remove(struct platform_device *pdev)
drm_kms_helper_poll_fini(ddev);
}
+static void shmob_drm_shutdown(struct platform_device *pdev)
+{
+ struct shmob_drm_device *sdev = platform_get_drvdata(pdev);
+
+ drm_atomic_helper_shutdown(>ddev);
+}
+
static int shmob_drm_probe(struct platform_device *pdev)
{
struct shmob_drm_platform_data *pdata = pdev->dev.platform_data;
@@ -273,6 +280,7 @@ static const struct of_device_id shmob_drm_of_table[]
__maybe_unused = {
static struct platform_driver shmob_drm_platform_driver = {
.probe = shmob_drm_probe,
.remove_new = shmob_drm_remove,
+ .shutdown = shmob_drm_shutdown,
.driver = {
.name = "shmob-drm",
.of_match_table = of_match_ptr(shmob_drm_of_table),
--
2.34.1
Re: [PATCH resend v2] drm: renesas: shmobile: Call drm_helper_force_disable_all() at shutdown time
Hi Doug,
On Tue, Jun 11, 2024 at 7:33 PM Doug Anderson wrote:
> On Wed, May 29, 2024 at 5:16 AM Geert Uytterhoeven
> wrote:
> >
> > From: Douglas Anderson
> >
> > Based on grepping through the source code, this driver appears to be
> > missing a call to drm_atomic_helper_shutdown() at system shutdown time.
> > This is important because drm_helper_force_disable_all() will cause
> > panels to get disabled cleanly which may be important for their power
> > sequencing. Future changes will remove any custom powering off in
> > individual panel drivers so the DRM drivers need to start getting this
> > right.
> >
> > The fact that we should call drm_atomic_helper_shutdown() in the case of
> > OS shutdown comes straight out of the kernel doc "driver instance
> > overview" in drm_drv.c.
> >
> > Suggested-by: Maxime Ripard
> > Signed-off-by: Douglas Anderson
> > Link:
> > https://lore.kernel.org/r/20230901164111.RFT.15.Iaf638a1d4c8b3c307a6192efabb4cbb06b195f15@changeid
> > [geert: s/drm_helper_force_disable_all/drm_atomic_helper_shutdown/]
> > [geert: shmob_drm_remove() already calls drm_atomic_helper_shutdown]
> > Signed-off-by: Geert Uytterhoeven
> > Reviewed-by: Laurent Pinchart
> > ---
> > v2:
> > - Add Reviewed-by.
> >
> > Tested on Atmark Techno Armadillo-800-EVA.
> > ---
> > drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 8
> > 1 file changed, 8 insertions(+)
>
> FWIW: I've created a patch to list DRM modeset drivers that handle
> shutdown properly [1]. For now "shmob-drm" is not part of that
> patchset. Assuming my patch lands we'll have to later add it to the
> list.
Ouch, keeping such a list is ugly ;-)
> [1]
> https://lore.kernel.org/r/20240611074846.1.Ieb287c2c3ee3f6d3b0d5f49b29f746b93621749c@changeid
>
> I will also note that the subject/description of this patch could be
> adjusted. They still reference "drm_helper_force_disable_all" which
> should have been changed to "drm_atomic_helper_shutdown".
Thanks, v3 sent.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] docs: document python version used for compilation
Hi Thierry,
On Thu, May 30, 2024 at 7:07 PM Thierry Reding wrote:
> Alternatively, maybe Kconfig could be taught about build dependencies?
git grep "depends on \$(" -- "*Kconf*"
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [DO NOT MERGE v8 13/36] dt-bindings: clock: sh7750-cpg: Add renesas,sh7750-cpg header.
Hi Sato-san,
Thanks for the update!
On Wed, May 29, 2024 at 10:01 AM Yoshinori Sato
wrote:
> SH7750 CPG Clock output define.
(from my comments on v6 and v7) Please improve the patch description.
> Signed-off-by: Yoshinori Sato
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,sh7750-cpg.yaml
> @@ -0,0 +1,107 @@
> +examples:
> + - |
> +#include
> +cpg: clock-controller@ffc0 {
> +compatible = "renesas,sh7751r-cpg";
> +reg = <0xffc0 20>, <0xfe0a 16>;
> +reg-names = "FRQCR", "CLKSTP00";
> +clocks = <>;
> +clock-names = "extal";
> +renesas,mode = <0>;
Nit: please move "renesas,mode" last.
> +#clock-cells = <1>;
> +#power-domain-cells = <0>;
> +};
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH resend v2] drm: renesas: shmobile: Call drm_helper_force_disable_all() at shutdown time
From: Douglas Anderson
Based on grepping through the source code, this driver appears to be
missing a call to drm_atomic_helper_shutdown() at system shutdown time.
This is important because drm_helper_force_disable_all() will cause
panels to get disabled cleanly which may be important for their power
sequencing. Future changes will remove any custom powering off in
individual panel drivers so the DRM drivers need to start getting this
right.
The fact that we should call drm_atomic_helper_shutdown() in the case of
OS shutdown comes straight out of the kernel doc "driver instance
overview" in drm_drv.c.
Suggested-by: Maxime Ripard
Signed-off-by: Douglas Anderson
Link:
https://lore.kernel.org/r/20230901164111.RFT.15.Iaf638a1d4c8b3c307a6192efabb4cbb06b195f15@changeid
[geert: s/drm_helper_force_disable_all/drm_atomic_helper_shutdown/]
[geert: shmob_drm_remove() already calls drm_atomic_helper_shutdown]
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Laurent Pinchart
---
v2:
- Add Reviewed-by.
Tested on Atmark Techno Armadillo-800-EVA.
---
drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
index e83c3e52251dedf9..0250d5f00bf102dc 100644
--- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
@@ -171,6 +171,13 @@ static void shmob_drm_remove(struct platform_device *pdev)
drm_kms_helper_poll_fini(ddev);
}
+static void shmob_drm_shutdown(struct platform_device *pdev)
+{
+ struct shmob_drm_device *sdev = platform_get_drvdata(pdev);
+
+ drm_atomic_helper_shutdown(>ddev);
+}
+
static int shmob_drm_probe(struct platform_device *pdev)
{
struct shmob_drm_platform_data *pdata = pdev->dev.platform_data;
@@ -273,6 +280,7 @@ static const struct of_device_id shmob_drm_of_table[]
__maybe_unused = {
static struct platform_driver shmob_drm_platform_driver = {
.probe = shmob_drm_probe,
.remove_new = shmob_drm_remove,
+ .shutdown = shmob_drm_shutdown,
.driver = {
.name = "shmob-drm",
.of_match_table = of_match_ptr(shmob_drm_of_table),
--
2.34.1
[PATCH] drm: renesas: rcar-du: Add drm_panic support for non-vsp
Add support for the drm_panic module for DU variants not using the
VSP-compositor, to display a message on the screen when a kernel panic
occurs.
Signed-off-by: Geert Uytterhoeven
---
Tested on Koelsch (R-Car M2-W).
Support for DU variants using the VSP-compositor is more convoluted,
and left to the DU experts.
---
drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
index e445fac8e0b46c21..c546ab0805d656f6 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
@@ -680,6 +680,12 @@ static const struct drm_plane_helper_funcs
rcar_du_plane_helper_funcs = {
.atomic_update = rcar_du_plane_atomic_update,
};
+static const struct drm_plane_helper_funcs rcar_du_primary_plane_helper_funcs
= {
+ .atomic_check = rcar_du_plane_atomic_check,
+ .atomic_update = rcar_du_plane_atomic_update,
+ .get_scanout_buffer = drm_fb_dma_get_scanout_buffer,
+};
+
static struct drm_plane_state *
rcar_du_plane_atomic_duplicate_state(struct drm_plane *plane)
{
@@ -812,8 +818,12 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
if (ret < 0)
return ret;
- drm_plane_helper_add(>plane,
-_du_plane_helper_funcs);
+ if (type == DRM_PLANE_TYPE_PRIMARY)
+ drm_plane_helper_add(>plane,
+
_du_primary_plane_helper_funcs);
+ else
+ drm_plane_helper_add(>plane,
+_du_plane_helper_funcs);
drm_plane_create_alpha_property(>plane);
--
2.34.1
[PATCH] drm: renesas: shmobile: Add drm_panic support
Add support for the drm_panic module, which displays a message on
the screen when a kernel panic occurs.
Signed-off-by: Geert Uytterhoeven
---
Tested on Armadillo-800-EVA.
---
drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
index 07ad17d24294d5e6..9d166ab2af8bd231 100644
--- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
+++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
@@ -273,6 +273,13 @@ static const struct drm_plane_helper_funcs
shmob_drm_plane_helper_funcs = {
.atomic_disable = shmob_drm_plane_atomic_disable,
};
+static const struct drm_plane_helper_funcs
shmob_drm_primary_plane_helper_funcs = {
+ .atomic_check = shmob_drm_plane_atomic_check,
+ .atomic_update = shmob_drm_plane_atomic_update,
+ .atomic_disable = shmob_drm_plane_atomic_disable,
+ .get_scanout_buffer = drm_fb_dma_get_scanout_buffer,
+};
+
static const struct drm_plane_funcs shmob_drm_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
@@ -310,7 +317,12 @@ struct drm_plane *shmob_drm_plane_create(struct
shmob_drm_device *sdev,
splane->index = index;
- drm_plane_helper_add(>base, _drm_plane_helper_funcs);
+ if (type == DRM_PLANE_TYPE_PRIMARY)
+ drm_plane_helper_add(>base,
+_drm_primary_plane_helper_funcs);
+ else
+ drm_plane_helper_add(>base,
+_drm_plane_helper_funcs);
return >base;
}
--
2.34.1
Re: Build regressions/improvements in v6.10-rc1
On Mon, 27 May 2024, Geert Uytterhoeven wrote:
Below is the list of build error/warning regressions/improvements in
v6.10-rc1[1] compared to v6.9[2].
Summarized:
- build errors: +27/-20
- build warnings: +3/-1601
Happy fixing! ;-)
Thanks to the linux-next team for providing the build service.
[1]
http://kisskb.ellerman.id.au/kisskb/branch/linus/head/1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0/
(all 138 configs)
[2]
http://kisskb.ellerman.id.au/kisskb/branch/linus/head/a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6/
(all 138 configs)
*** ERRORS ***
27 error regressions:
+ /kisskb/src/arch/sparc/prom/p1275.c: error: no previous prototype for
'prom_cif_init' [-Werror=missing-prototypes]: => 52:6
sparc64-gcc13/sparc64-allmodconfig (seen before)
+
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:
error: the frame size of 2192 bytes is larger than 2048 bytes
[-Werror=frame-larger-than=]: => 5118:1
+
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20v2.c:
error: the frame size of 2280 bytes is larger than 2048 bytes
[-Werror=frame-larger-than=]: => 5234:1
+
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:
error: the frame size of 2096 bytes is larger than 2048 bytes
[-Werror=frame-larger-than=]: => 5188:1
+
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:
error: the frame size of 2184 bytes is larger than 2048 bytes
[-Werror=frame-larger-than=]: => 3049:1
+
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:
error: the frame size of 2264 bytes is larger than 2048 bytes
[-Werror=frame-larger-than=]: => 3274:1
+
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn314/display_mode_vba_314.c:
error: the frame size of 2232 bytes is larger than 2048 bytes
[-Werror=frame-larger-than=]: => 3296:1
+
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn314/display_rq_dlg_calc_314.c:
error: the frame size of 2080 bytes is larger than 2048 bytes
[-Werror=frame-larger-than=]: => 1646:1
powerpc-gcc5/ppc32_allmodconfig
+ /kisskb/src/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c: error: unknown option
after '#pragma GCC diagnostic' kind [-Werror=pragmas]: => 16:9
+ /kisskb/src/drivers/gpu/drm/msm/adreno/adreno_gen7_9_0_snapshot.h: error:
'gen7_9_0_external_core_regs' defined but not used [-Werror=unused-variable]:
=> 1438:19
+ /kisskb/src/drivers/gpu/drm/msm/adreno/adreno_gen7_9_0_snapshot.h: error:
'gen7_9_0_sptp_clusters' defined but not used [-Werror=unused-variable]: =>
1188:43
arm64-gcc5/arm64-allmodconfig
powerpc-gcc5/powerpc-all{mod,yes}config
powerpc-gcc5/ppc32_allmodconfig
powerpc-gcc5/ppc64_book3e_allmodconfig
powerpc-gcc5/ppc64le_allmodconfig
sparc64-gcc5/sparc64-allmodconfig
Looks like #pragma "-Wunused-const-variable" is not supported by gcc-5
+ /kisskb/src/drivers/gpu/drm/nouveau/nvif/object.c: error: 'memcpy' accessing
4294967240 or more bytes at offsets 0 and 56 overlaps 6442450833 bytes at offset
-2147483593 [-Werror=restrict]: => 298:17
+ /kisskb/src/drivers/gpu/drm/nouveau/nvif/object.c: error: 'memcpy' accessing
4294967264 or more bytes at offsets 0 and 32 overlaps 6442450881 bytes at offset
-2147483617 [-Werror=restrict]: => 161:9
parisc-gcc13/generic-32bit_defconfig
parisc-gcc13/parisc-{def,allmod}config
+ /kisskb/src/include/linux/kern_levels.h: error: format '%lu' expects argument
of type 'long unsigned int', but argument 4 has type 'unsigned int'
[-Werror=format=]: => 5:18, 5:25
mips-gcc{8,13}/mips-allmodconfig
parisc-gcc13/parisc-allmodconfig
powerpc-gcc{5,13}/ppc32_allmodconfig
sparc64-gcc{5,13}/sparc-allmodconfig
xtensa-gcc13/xtensa-allmodconfig
drivers/scsi/mpi3mr/mpi3mr_transport.c: In function 'mpi3mr_sas_port_add':
drivers/scsi/mpi3mr/mpi3mr_transport.c:1367:62: note: format string is defined
here
ioc_warn(mrioc, "skipping port %u, max allowed value is %lu\n",
~~^
%u
+ /kisskb/src/kernel/bpf/verifier.c: error: ‘pcpu_hot’ undeclared (first use in
this function): => 20317:85
+ /kisskb/src/lib/iomap.c: error: no previous prototype for ‘ioread64_hi_lo’
[-Werror=missing-prototypes]: => 163:5
+ /kisskb/src/lib/iomap.c: error: no previous prototype for ‘ioread64_lo_hi’
[-Werror=missing-prototypes]: => 156:5
+ /kisskb/src/lib/iomap.c: error: no previous prototype for ‘ioread64be_hi_lo’
[-Werror=missing-prototypes]: => 178:5
+ /kisskb/src/lib/iomap.c: error: no previous prototype for ‘ioread64be_lo_hi’
[-Werror=missing-prototypes]: => 170:5
+ /kisskb/src/lib/iomap.c: error: no previous prototype for ‘iowrite64_hi_lo’
[-Werror=missing-prototypes]: => 272:6
+ /kisskb/src/lib/iomap.c: error: no previous prototype f
Re: [RESEND v7 15/37] clk: renesas: Add SH7750/7751 CPG Driver
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
wrote:
> Renesas SH7750 and SH7751 series CPG driver.
> This driver supported frequency control and clock gating.
>
> Signed-off-by: Yoshinori Sato
Thanks for the update!
As you plan to send a v8 soon, I'm sending you a comment from the
(incomplete) review I started a while ago...
> --- /dev/null
> +++ b/drivers/clk/renesas/clk-sh7750.c
> +static int register_pll(struct device_node *node, struct cpg_priv *cpg)
> +{
> + const char *clk_name = node->name;
> + const char *parent_name;
> + struct clk_init_data init = {
> + .name = PLLOUT,
> + .ops = _ops,
> + .flags = 0,
> + .num_parents = 1,
> + };
> + int ret;
> +
> + parent_name = of_clk_get_parent_name(node, 0);
> + init.parent_names = _name;
> + cpg->hw.init =
> +
> + ret = of_clk_hw_register(node, >hw);
> + if (ret < 0)
> + pr_err("%pOF: failed to add provider %s (%d)\n",
I think you retained the wrong error message?
"%s: failed to register %s pll clock (%d)\n" sounds more suitable to me.
> + node, clk_name, ret);
> + return ret;
> +}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND v7 00/37] Device Tree support for SH7751 based board
On Mon, May 20, 2024 at 5:25 PM John Paul Adrian Glaubitz
wrote:
> On Mon, 2024-05-20 at 22:06 +0900, Yoshinori Sato wrote:
> > I'll be posting v8 soon.
>
> Sounds good! Maybe we can start merging the patches that contain fixes only
> and that have already been reviewed. This way, we can reduce the overall size
> of the series a bit.
+1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 00/11] drm: Restore helper usability
Hi Jani,
CC kbuild
On Mon, Apr 22, 2024 at 7:00 PM Jani Nikula wrote:
> On Mon, 22 Apr 2024, "Arnd Bergmann" wrote:
> > I'm not sure where this misunderstanding comes from, as you
> > seem to be repeating the same incorrect assumption about
> > how select works that Maxime wrote in his changelog. To clarify,
> > this works exactly as one would expect:
> >
> > config HELPER_A
> >tristate
> >
> > config HELPER_B
> >tristate
> >select HELPER_A
> >
> > config DRIVER
> >tristate "Turn on the driver and the helpers it uses"
> >select HELPER_B # this recursively selects HELPER_A
> >
> > Whereas this one is broken:
> >
> > config FEATURE_A
> >tristate "user visible if I2C is enabled"
> >depends on I2C
> >
> > config HELPER_B
> >tristate # hidden
> >select FEATURE_A
> >
> > config DRIVER
> >tristate "This driver is broken if I2C is disabled"
> >select HELPER_B
>
> This case is really what I was referring to, although I was sloppy with
> words there. I understand that select does work recursively for selects.
>
> >> There is no end to this, it just goes on and on, as the
> >> dependencies of the selected symbols change over time. Often the
> >> selects require unintuitive if patterns that are about the
> >> implementation details of the symbol being selected.
> >
> > Agreed, that is the problem I frequently face with drivers/gpu/drm,
> > and most of the time it can only be solved by rewriting the whole
> > system to not select user-visible symbol at all.
> >
> > Using 'depends on' by itself is unfortunately not enough to
> > avoid /all/ the problems. See e.g. today's failure
> >
> > config DRM_DISPLAY_HELPER
> >tristate "DRM Display Helpers"
> >default y
> >
> > config DRM_DISPLAY_DP_HELPER
> >bool "DRM DisplayPort Helpers"
> >depends on DRM_DISPLAY_HELPER
> >
> > config DRM_PANEL_LG_SW43408
> >tristate "LG SW43408 panel"
> >depends on DRM_DISPLAY_DP_HELPER
> >
> > This version is still broken for DRM_DISPLAY_HELPER=m,
> > DRM_DISPLAY_DP_HELPER=m, DRM_PANEL_LG_SW43408=y because
> > the dependency on the bool symbol is not enough to
> > ensure that DRM_DISPLAY_HELPER is also built-in, so you
> > still need explicit dependencies on both
> > DRM_DISPLAY_HELPER and DRM_DISPLAY_DP_HELPER in the users.
> >
> > This can be solved by making DRM_DISPLAY_DP_HELPER a
> > tristate symbol and adjusting the #ifdef checks and
> > Makefile logic accordingly, which is exactly what you'd
> > need to do to make it work with 'select' as well.
>
> So bool is kind of problematic for depends on and select even when it's
> not really used for describing builtin vs. no, but rather yes vs. no?
Yes, the underlying issue is that bool is used for two different things:
A. To enable a driver module that can be only built-in,
B. To enable an option or feature of a driver or subsystem.
Without this distinction, dependencies cannot be auto-propagated 100%
correctly. Fixing that would require introducing a third type (and possibly
renaming the existing ones to end up with 3 good names).
Actually two types could work:
1. driver,
2. option,
as case A is just a driver that can only be built-in (i.e. "depends on y",
which is similar to the behavior with CONFIG_MODULES=n).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 00/11] drm: Restore helper usability
Hi Jani,
On Mon, Apr 22, 2024 at 7:15 PM Jani Nikula wrote:
> On Mon, 22 Apr 2024, Geert Uytterhoeven wrote:
> > On Mon, Apr 22, 2024 at 3:55 PM Arnd Bergmann wrote:
> >> I'm not sure where this misunderstanding comes from, as you
> >> seem to be repeating the same incorrect assumption about
> >> how select works that Maxime wrote in his changelog. To clarify,
> >> this works exactly as one would expect:
> >>
> >> config HELPER_A
> >>tristate
> >>
> >> config HELPER_B
> >>tristate
> >>select HELPER_A
> >>
> >> config DRIVER
> >>tristate "Turn on the driver and the helpers it uses"
> >>select HELPER_B # this recursively selects HELPER_A
> >>
> >> Whereas this one is broken:
> >>
> >> config FEATURE_A
> >>tristate "user visible if I2C is enabled"
> >>depends on I2C
> >>
> >> config HELPER_B
> >>tristate # hidden
> >>select FEATURE_A
> >>
> >> config DRIVER
> >>tristate "This driver is broken if I2C is disabled"
> >>select HELPER_B
> >
> > So the DRIVER section should gain a "depends on I2C" statement.
>
> Why should DRIVER have to care that HELPER_B needs either FEATURE_A or
> I2C? It should only have to care about HELPER_B. And if the dependencies
> of FEATURE_A or HELPER_B later change, that's their business, not
> DRIVER's.
That's correct. But currently the dependency on I2C is not handled
automatically.
> > Yamada-san: would it be difficult to modify Kconfig to ignore symbols
> > like DRIVER that select other symbols with unmet dependencies?
> > Currently it already warns about that.
> >
> > Handling this implicitly (instead of the current explict "depends
> > on") would have the disadvantage though: a user who is not aware of
> > the implicit dependency may wonder why DRIVER is invisible in his
> > config interface.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 00/11] drm: Restore helper usability
Hi Arnd,
CC kbuild
On Mon, Apr 22, 2024 at 3:55 PM Arnd Bergmann wrote:
> On Mon, Apr 22, 2024, at 15:28, Jani Nikula wrote:
> > On Mon, 22 Apr 2024, "Arnd Bergmann" wrote:
> >> 2. Using "select" on user visible symbols that have dependencies
> >>is a common source for bugs, and this is is a problem in
> >>drivers/gpu/drm more than elsewhere in the kernel, as these
> >>drivers traditionally select entire subsystems or drivers
> >>(I2C, VIRTIO, INPUT, ACPI_WMI, BACKLIGHT_CLASS_DEVICE,
> >>POWER_SUPPLY, SND_PCM, INTERCONNECT, ...). This regularly
> >>leads to circular dependencies and we should fix all of them.
> >
> > What annoys me is that the fixes tend to fall in two categories:
> >
> > - Play catch with selecting the dependencies of the selected
> > symbols. "depends on" handles this recursively, while select does
> > not.
>
> I'm not sure where this misunderstanding comes from, as you
> seem to be repeating the same incorrect assumption about
> how select works that Maxime wrote in his changelog. To clarify,
> this works exactly as one would expect:
>
> config HELPER_A
>tristate
>
> config HELPER_B
>tristate
>select HELPER_A
>
> config DRIVER
>tristate "Turn on the driver and the helpers it uses"
>select HELPER_B # this recursively selects HELPER_A
>
> Whereas this one is broken:
>
> config FEATURE_A
>tristate "user visible if I2C is enabled"
>depends on I2C
>
> config HELPER_B
>tristate # hidden
>select FEATURE_A
>
> config DRIVER
>tristate "This driver is broken if I2C is disabled"
>select HELPER_B
So the DRIVER section should gain a "depends on I2C" statement.
Yamada-san: would it be difficult to modify Kconfig to ignore symbols
like DRIVER that select other symbols with unmet dependencies?
Currently it already warns about that.
Handling this implicitly (instead of the current explict "depends
on") would have the disadvantage though: a user who is not aware of
the implicit dependency may wonder why DRIVER is invisible in his
config interface.
>
> > There is no end to this, it just goes on and on, as the
> > dependencies of the selected symbols change over time. Often the
> > selects require unintuitive if patterns that are about the
> > implementation details of the symbol being selected.
>
> Agreed, that is the problem I frequently face with drivers/gpu/drm,
> and most of the time it can only be solved by rewriting the whole
> system to not select user-visible symbol at all.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH 11/11] Revert "drm/display: Make all helpers visible and switch to depends on"
This reverts commit d674858ff979550a0e97b4ac766f2640f0d9d7e7, as helper code should always be selected by the driver that needs it, for the convenience of the final user configuring a kernel. The user who configures a kernel should not need to know which helpers are needed for the driver he is interested in. Making a driver depend on helper code means that the user needs to know which helpers to enable first, which is very user-unfriendly. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/display/Kconfig | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index c77e7f85bd674dc9..864a6488bfdf1499 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -1,21 +1,20 @@ # SPDX-License-Identifier: MIT config DRM_DISPLAY_HELPER - tristate "DRM Display Helpers" + tristate depends on DRM help DRM helpers for display adapters. config DRM_DISPLAY_DP_AUX_BUS - tristate "DRM DisplayPort AUX bus support" + tristate depends on DRM depends on OF || COMPILE_TEST config DRM_DISPLAY_DP_AUX_CEC bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support" - depends on DRM - depends on DRM_DISPLAY_HELPER - depends on DRM_DISPLAY_DP_HELPER + depends on DRM && DRM_DISPLAY_HELPER + select DRM_DISPLAY_DP_HELPER select CEC_CORE help Choose this option if you want to enable HDMI CEC support for @@ -25,24 +24,23 @@ config DRM_DISPLAY_DP_AUX_CEC that do support this they often do not hook up the CEC pin. config DRM_DISPLAY_DP_AUX_CHARDEV - bool "DRM DisplayPort AUX Interface" - depends on DRM - depends on DRM_DISPLAY_HELPER - depends on DRM_DISPLAY_DP_HELPER + bool "DRM DP AUX Interface" + depends on DRM && DRM_DISPLAY_HELPER + select DRM_DISPLAY_DP_HELPER help Choose this option to enable a /dev/drm_dp_auxN node that allows to read and write values to arbitrary DPCD registers on the DP aux channel. config DRM_DISPLAY_DP_HELPER - bool "DRM DisplayPort Helpers" + bool depends on DRM_DISPLAY_HELPER help DRM display helpers for DisplayPort. config DRM_DISPLAY_DP_TUNNEL - bool "DRM DisplayPort tunnels support" - depends on DRM_DISPLAY_DP_HELPER + bool + select DRM_DISPLAY_DP_HELPER help Enable support for DisplayPort tunnels. This allows drivers to use DP tunnel features like the Bandwidth Allocation mode to maximize the @@ -62,13 +60,13 @@ config DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG If in doubt, say "N". config DRM_DISPLAY_HDCP_HELPER - bool "DRM HDCD Helpers" + bool depends on DRM_DISPLAY_HELPER help DRM display helpers for HDCP. config DRM_DISPLAY_HDMI_HELPER - bool "DRM HDMI Helpers" + bool depends on DRM_DISPLAY_HELPER help DRM display helpers for HDMI. -- 2.34.1
[PATCH 09/11] Revert "drm: Switch DRM_DISPLAY_HELPER to depends on"
This reverts commit e075e496f516bf92bc0cbaf94d64e8d4a6b58321, as helper code should always be selected by the driver that needs it, for the convenience of the final user configuring a kernel. The user who configures a kernel should not need to know which helpers are needed for the driver he is interested in. Making a driver depend on helper code means that the user needs to know which helpers to enable first, which is very user-unfriendly. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/Kconfig | 6 ++ drivers/gpu/drm/amd/amdgpu/Kconfig | 6 ++ drivers/gpu/drm/bridge/Kconfig | 10 +- drivers/gpu/drm/bridge/analogix/Kconfig | 6 +++--- drivers/gpu/drm/bridge/cadence/Kconfig | 4 ++-- drivers/gpu/drm/bridge/synopsys/Kconfig | 2 +- drivers/gpu/drm/display/Kconfig | 1 - drivers/gpu/drm/exynos/Kconfig | 2 +- drivers/gpu/drm/i915/Kconfig| 2 +- drivers/gpu/drm/mediatek/Kconfig| 2 +- drivers/gpu/drm/msm/Kconfig | 4 ++-- drivers/gpu/drm/nouveau/Kconfig | 6 ++ drivers/gpu/drm/panel/Kconfig | 20 ++-- drivers/gpu/drm/radeon/Kconfig | 6 ++ drivers/gpu/drm/rockchip/Kconfig| 4 ++-- drivers/gpu/drm/tegra/Kconfig | 2 +- drivers/gpu/drm/vc4/Kconfig | 8 drivers/gpu/drm/xe/Kconfig | 7 ++- drivers/gpu/drm/xlnx/Kconfig| 6 ++ 19 files changed, 45 insertions(+), 59 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 33792ca3eeb7ae8d..bf4020915e299861 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -74,12 +74,10 @@ config DRM_KUNIT_TEST_HELPERS config DRM_KUNIT_TEST tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS - depends on DRM - depends on DRM_DISPLAY_HELPER - depends on KUNIT - depends on MMU + depends on DRM && KUNIT && MMU select DRM_BUDDY select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HELPER select DRM_EXEC select DRM_EXPORT_FOR_TESTS if m select DRM_GEM_SHMEM_HELPER diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index cf931b94a1889746..22d88f8ef5279a0f 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -2,15 +2,13 @@ config DRM_AMDGPU tristate "AMD GPU" - depends on DRM - depends on DRM_DISPLAY_HELPER - depends on MMU - depends on PCI + depends on DRM && PCI && MMU depends on !UML select FW_LOADER select DRM_DISPLAY_DP_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HDCP_HELPER + select DRM_DISPLAY_HELPER select DRM_KMS_HELPER select DRM_SCHED select DRM_TTM diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index a51ad2b3a0fb01e2..f71d57216ae0602a 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -92,10 +92,10 @@ config DRM_FSL_LDB config DRM_ITE_IT6505 tristate "ITE IT6505 DisplayPort bridge" - depends on DRM_DISPLAY_HELPER depends on OF select DRM_DISPLAY_DP_HELPER select DRM_DISPLAY_HDCP_HELPER + select DRM_DISPLAY_HELPER select DRM_DISPLAY_DP_AUX_BUS select DRM_KMS_HELPER select EXTCON @@ -225,9 +225,9 @@ config DRM_PARADE_PS8622 config DRM_PARADE_PS8640 tristate "Parade PS8640 MIPI DSI to eDP Converter" - depends on DRM_DISPLAY_HELPER depends on OF select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HELPER select DRM_DISPLAY_DP_AUX_BUS select DRM_KMS_HELPER select DRM_MIPI_DSI @@ -312,9 +312,9 @@ config DRM_TOSHIBA_TC358764 config DRM_TOSHIBA_TC358767 tristate "Toshiba TC358767 eDP bridge" - depends on DRM_DISPLAY_HELPER depends on OF select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HELPER select DRM_KMS_HELPER select REGMAP_I2C select DRM_MIPI_DSI @@ -335,9 +335,9 @@ config DRM_TOSHIBA_TC358768 config DRM_TOSHIBA_TC358775 tristate "Toshiba TC358775 DSI/LVDS bridge" - depends on DRM_DISPLAY_HELPER depends on OF select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HELPER select DRM_KMS_HELPER select REGMAP_I2C select DRM_PANEL @@ -380,9 +380,9 @@ config DRM_TI_SN65DSI83 config DRM_TI_SN65DSI86 tristate "TI SN65DSI86 DSI to eDP bridge" - depends on DRM_DISPLAY_HELPER depends on OF select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HELPER select DRM_KMS_HELPER select REGMAP_I2C select DRM_PANEL diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/dri
[PATCH 04/11] Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies"
This reverts commit d1ef8fc18be6adbbffdee06fbb5b33699e2852be, as the commit it fixes will be reverted, too. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/exynos/Kconfig | 2 +- drivers/gpu/drm/rockchip/Kconfig | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig index 58cd772207413e94..6a26a0b8eff2c021 100644 --- a/drivers/gpu/drm/exynos/Kconfig +++ b/drivers/gpu/drm/exynos/Kconfig @@ -68,7 +68,7 @@ config DRM_EXYNOS_DP bool "Exynos specific extensions for Analogix DP driver" depends on DRM_EXYNOS_FIMD || DRM_EXYNOS7_DECON depends on DRM_DISPLAY_DP_HELPER - depends on DRM_DISPLAY_HELPER=y || (DRM_DISPLAY_HELPER=m && DRM_EXYNOS=m) + depends on DRM_DISPLAY_HELPER select DRM_ANALOGIX_DP default DRM_EXYNOS select DRM_PANEL diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig index 4b4ad75032fda17e..4b49a14758fe0412 100644 --- a/drivers/gpu/drm/rockchip/Kconfig +++ b/drivers/gpu/drm/rockchip/Kconfig @@ -46,7 +46,7 @@ config ROCKCHIP_ANALOGIX_DP config ROCKCHIP_CDN_DP bool "Rockchip cdn DP" depends on DRM_DISPLAY_DP_HELPER - depends on DRM_DISPLAY_HELPER=y || (DRM_DISPLAY_HELPER=m && DRM_ROCKCHIP=m) + depends on DRM_DISPLAY_HELPER depends on EXTCON=y || (EXTCON=m && DRM_ROCKCHIP=m) help This selects support for Rockchip SoC specific extensions -- 2.34.1
[PATCH 02/11] Revert "drm/display: Select DRM_KMS_HELPER for DP helpers"
This reverts commit 7fa678cc0a5648b5ea28629a2d21b9d4b6ac8f56, as the commit it fixes will be reverted, too. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/display/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index a38962a556c2e847..01f2a231aa5f04bd 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -39,7 +39,6 @@ config DRM_DISPLAY_DP_AUX_CHARDEV config DRM_DISPLAY_DP_HELPER bool "DRM DisplayPort Helpers" depends on DRM_DISPLAY_HELPER - select DRM_KMS_HELPER default y help DRM display helpers for DisplayPort. -- 2.34.1
[PATCH 08/11] Revert "drm: Switch DRM_DISPLAY_DP_AUX_BUS to depends on"
This reverts commit 4d15125d7fe637f401e64e33c99513adf6586fdd, as helper code should always be selected by the driver that needs it, for the convenience of the final user configuring a kernel. The user who configures a kernel should not need to know which helpers are needed for the driver he is interested in. Making a driver depend on helper code means that the user needs to know which helpers to enable first, which is very user-unfriendly. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/bridge/Kconfig | 6 +++--- drivers/gpu/drm/bridge/analogix/Kconfig | 2 +- drivers/gpu/drm/display/Kconfig | 1 - drivers/gpu/drm/mediatek/Kconfig| 2 +- drivers/gpu/drm/msm/Kconfig | 2 +- drivers/gpu/drm/panel/Kconfig | 4 ++-- drivers/gpu/drm/tegra/Kconfig | 2 +- 7 files changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 6015e2e4c2f620cf..a51ad2b3a0fb01e2 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -92,11 +92,11 @@ config DRM_FSL_LDB config DRM_ITE_IT6505 tristate "ITE IT6505 DisplayPort bridge" - depends on DRM_DISPLAY_DP_AUX_BUS depends on DRM_DISPLAY_HELPER depends on OF select DRM_DISPLAY_DP_HELPER select DRM_DISPLAY_HDCP_HELPER + select DRM_DISPLAY_DP_AUX_BUS select DRM_KMS_HELPER select EXTCON select CRYPTO @@ -225,10 +225,10 @@ config DRM_PARADE_PS8622 config DRM_PARADE_PS8640 tristate "Parade PS8640 MIPI DSI to eDP Converter" - depends on DRM_DISPLAY_DP_AUX_BUS depends on DRM_DISPLAY_HELPER depends on OF select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DP_AUX_BUS select DRM_KMS_HELPER select DRM_MIPI_DSI select DRM_PANEL @@ -380,7 +380,6 @@ config DRM_TI_SN65DSI83 config DRM_TI_SN65DSI86 tristate "TI SN65DSI86 DSI to eDP bridge" - depends on DRM_DISPLAY_DP_AUX_BUS depends on DRM_DISPLAY_HELPER depends on OF select DRM_DISPLAY_DP_HELPER @@ -389,6 +388,7 @@ config DRM_TI_SN65DSI86 select DRM_PANEL select DRM_MIPI_DSI select AUXILIARY_BUS + select DRM_DISPLAY_DP_AUX_BUS help Texas Instruments SN65DSI86 DSI to eDP Bridge driver diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig index ec98c94535736c0a..16d18dde483ae9c4 100644 --- a/drivers/gpu/drm/bridge/analogix/Kconfig +++ b/drivers/gpu/drm/bridge/analogix/Kconfig @@ -33,11 +33,11 @@ config DRM_ANALOGIX_DP config DRM_ANALOGIX_ANX7625 tristate "Analogix Anx7625 MIPI to DP interface support" depends on DRM - depends on DRM_DISPLAY_DP_AUX_BUS depends on DRM_DISPLAY_HELPER depends on OF select DRM_DISPLAY_DP_HELPER select DRM_DISPLAY_HDCP_HELPER + select DRM_DISPLAY_DP_AUX_BUS select DRM_MIPI_DSI help ANX7625 is an ultra-low power 4K mobile HD transmitter diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index 0cd4396914224226..cffa2acdbc6c0988 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -11,7 +11,6 @@ config DRM_DISPLAY_DP_AUX_BUS tristate "DRM DisplayPort AUX bus support" depends on DRM depends on OF || COMPILE_TEST - default y config DRM_DISPLAY_DP_AUX_CEC bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support" diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig index 2add54486ac4ab11..50bb28327f65fbf5 100644 --- a/drivers/gpu/drm/mediatek/Kconfig +++ b/drivers/gpu/drm/mediatek/Kconfig @@ -22,11 +22,11 @@ config DRM_MEDIATEK config DRM_MEDIATEK_DP tristate "DRM DPTX Support for MediaTek SoCs" - depends on DRM_DISPLAY_DP_AUX_BUS depends on DRM_DISPLAY_HELPER depends on DRM_MEDIATEK select PHY_MTK_DP select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DP_AUX_BUS help DRM/KMS Display Port driver for MediaTek SoCs. diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 28a898722ace789b..2055266506e5adf0 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -5,7 +5,6 @@ config DRM_MSM depends on ARCH_QCOM || SOC_IMX5 || COMPILE_TEST depends on COMMON_CLK depends on DRM - depends on DRM_DISPLAY_DP_AUX_BUS depends on DRM_DISPLAY_HELPER depends on IOMMU_SUPPORT depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n @@ -16,6 +15,7 @@ config DRM_MSM select IOMMU_IO_PGTABLE select QCOM_MDT_LOADER if ARCH_QCOM select REGULATOR + select DRM_DISPLAY_DP_AUX_BUS select DRM_DISPLAY_DP_HELPER select DRM_EXEC
[PATCH 07/11] Revert "drm: Switch DRM_DISPLAY_DP_HELPER to depends on"
This reverts commit 0323287de87d7e6e9c22c57d7440aa353a2298d0, as helper code should always be selected by the driver that needs it, for the convenience of the final user configuring a kernel. The user who configures a kernel should not need to know which helpers are needed for the driver he is interested in. Making a driver depend on helper code means that the user needs to know which helpers to enable first, which is very user-unfriendly. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/Kconfig | 2 +- drivers/gpu/drm/amd/amdgpu/Kconfig | 2 +- drivers/gpu/drm/bridge/Kconfig | 10 +- drivers/gpu/drm/bridge/analogix/Kconfig | 6 +++--- drivers/gpu/drm/bridge/cadence/Kconfig | 2 +- drivers/gpu/drm/display/Kconfig | 1 - drivers/gpu/drm/exynos/Kconfig | 2 +- drivers/gpu/drm/i915/Kconfig| 2 +- drivers/gpu/drm/mediatek/Kconfig| 2 +- drivers/gpu/drm/msm/Kconfig | 2 +- drivers/gpu/drm/nouveau/Kconfig | 2 +- drivers/gpu/drm/panel/Kconfig | 8 drivers/gpu/drm/radeon/Kconfig | 2 +- drivers/gpu/drm/rockchip/Kconfig| 4 ++-- drivers/gpu/drm/tegra/Kconfig | 2 +- drivers/gpu/drm/xe/Kconfig | 2 +- drivers/gpu/drm/xlnx/Kconfig| 2 +- 17 files changed, 26 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 959b19a0410188d9..33792ca3eeb7ae8d 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -75,11 +75,11 @@ config DRM_KUNIT_TEST_HELPERS config DRM_KUNIT_TEST tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS depends on DRM - depends on DRM_DISPLAY_DP_HELPER depends on DRM_DISPLAY_HELPER depends on KUNIT depends on MMU select DRM_BUDDY + select DRM_DISPLAY_DP_HELPER select DRM_EXEC select DRM_EXPORT_FOR_TESTS if m select DRM_GEM_SHMEM_HELPER diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index ba09121e7debb247..cf931b94a1889746 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -3,12 +3,12 @@ config DRM_AMDGPU tristate "AMD GPU" depends on DRM - depends on DRM_DISPLAY_DP_HELPER depends on DRM_DISPLAY_HELPER depends on MMU depends on PCI depends on !UML select FW_LOADER + select DRM_DISPLAY_DP_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_KMS_HELPER diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index ad38cd201b006251..6015e2e4c2f620cf 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -93,9 +93,9 @@ config DRM_FSL_LDB config DRM_ITE_IT6505 tristate "ITE IT6505 DisplayPort bridge" depends on DRM_DISPLAY_DP_AUX_BUS - depends on DRM_DISPLAY_DP_HELPER depends on DRM_DISPLAY_HELPER depends on OF + select DRM_DISPLAY_DP_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_KMS_HELPER select EXTCON @@ -226,9 +226,9 @@ config DRM_PARADE_PS8622 config DRM_PARADE_PS8640 tristate "Parade PS8640 MIPI DSI to eDP Converter" depends on DRM_DISPLAY_DP_AUX_BUS - depends on DRM_DISPLAY_DP_HELPER depends on DRM_DISPLAY_HELPER depends on OF + select DRM_DISPLAY_DP_HELPER select DRM_KMS_HELPER select DRM_MIPI_DSI select DRM_PANEL @@ -312,9 +312,9 @@ config DRM_TOSHIBA_TC358764 config DRM_TOSHIBA_TC358767 tristate "Toshiba TC358767 eDP bridge" - depends on DRM_DISPLAY_DP_HELPER depends on DRM_DISPLAY_HELPER depends on OF + select DRM_DISPLAY_DP_HELPER select DRM_KMS_HELPER select REGMAP_I2C select DRM_MIPI_DSI @@ -335,9 +335,9 @@ config DRM_TOSHIBA_TC358768 config DRM_TOSHIBA_TC358775 tristate "Toshiba TC358775 DSI/LVDS bridge" - depends on DRM_DISPLAY_DP_HELPER depends on DRM_DISPLAY_HELPER depends on OF + select DRM_DISPLAY_DP_HELPER select DRM_KMS_HELPER select REGMAP_I2C select DRM_PANEL @@ -381,9 +381,9 @@ config DRM_TI_SN65DSI83 config DRM_TI_SN65DSI86 tristate "TI SN65DSI86 DSI to eDP bridge" depends on DRM_DISPLAY_DP_AUX_BUS - depends on DRM_DISPLAY_DP_HELPER depends on DRM_DISPLAY_HELPER depends on OF + select DRM_DISPLAY_DP_HELPER select DRM_KMS_HELPER select REGMAP_I2C select DRM_PANEL diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig index 9659df6718de6db4..ec98c94535736c0a 100644 --- a/drivers/gpu/drm/bridge/analogix/Kconfig +++ b/drivers/gpu/drm/bridge/analogix/Kcon
[PATCH 03/11] Revert "drm/bridge: dw-hdmi: Make DRM_DW_HDMI selectable"
This reverts commit 0209df3b4731516fe77638bfc52ba2e9629c67cd, as the commit it fixes (which is BTW not the commit in the Fixes: tag!) will be reverted, too. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/bridge/synopsys/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig index 1252fd30d4a4b461..387f5bd86089fb5c 100644 --- a/drivers/gpu/drm/bridge/synopsys/Kconfig +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only config DRM_DW_HDMI - tristate "Synopsys Designware HDMI TX Controller" + tristate depends on DRM_DISPLAY_HDMI_HELPER depends on DRM_DISPLAY_HELPER select DRM_KMS_HELPER -- 2.34.1
[PATCH 05/11] Revert "drm: Switch DRM_DISPLAY_HDMI_HELPER to depends on"
This reverts commit f6d2dc03fa8546b284dd8c1af027d9fac5725921, as helper code should always be selected by the driver that needs it, for the convenience of the final user configuring a kernel. The user who configures a kernel should not need to know which helpers are needed for the driver he is interested in. Making a driver depend on helper code means that the user needs to know which helpers to enable first, which is very user-unfriendly. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/amd/amdgpu/Kconfig | 2 +- drivers/gpu/drm/bridge/synopsys/Kconfig | 2 +- drivers/gpu/drm/display/Kconfig | 1 - drivers/gpu/drm/i915/Kconfig| 2 +- drivers/gpu/drm/nouveau/Kconfig | 2 +- drivers/gpu/drm/tegra/Kconfig | 2 +- drivers/gpu/drm/vc4/Kconfig | 2 +- drivers/gpu/drm/xe/Kconfig | 2 +- 8 files changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index b0365cc1374ee50a..1662dc49f18ed11e 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -5,12 +5,12 @@ config DRM_AMDGPU depends on DRM depends on DRM_DISPLAY_DP_HELPER depends on DRM_DISPLAY_HDCP_HELPER - depends on DRM_DISPLAY_HDMI_HELPER depends on DRM_DISPLAY_HELPER depends on MMU depends on PCI depends on !UML select FW_LOADER + select DRM_DISPLAY_HDMI_HELPER select DRM_KMS_HELPER select DRM_SCHED select DRM_TTM diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig index 387f5bd86089fb5c..f366ece471462a70 100644 --- a/drivers/gpu/drm/bridge/synopsys/Kconfig +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig @@ -1,8 +1,8 @@ # SPDX-License-Identifier: GPL-2.0-only config DRM_DW_HDMI tristate - depends on DRM_DISPLAY_HDMI_HELPER depends on DRM_DISPLAY_HELPER + select DRM_DISPLAY_HDMI_HELPER select DRM_KMS_HELPER select REGMAP_MMIO select CEC_CORE if CEC_NOTIFIER diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index 01f2a231aa5f04bd..d65f1a37c08c7cf8 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -74,6 +74,5 @@ config DRM_DISPLAY_HDCP_HELPER config DRM_DISPLAY_HDMI_HELPER bool "DRM HDMI Helpers" depends on DRM_DISPLAY_HELPER - default y help DRM display helpers for HDMI. diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 4f0d18a16b0f4f99..87ef8c4d72a53768 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -4,7 +4,6 @@ config DRM_I915 depends on DRM depends on DRM_DISPLAY_DP_HELPER depends on DRM_DISPLAY_HDCP_HELPER - depends on DRM_DISPLAY_HDMI_HELPER depends on DRM_DISPLAY_HELPER depends on X86 && PCI depends on !PREEMPT_RT @@ -14,6 +13,7 @@ config DRM_I915 # the shmem_readpage() which depends upon tmpfs select SHMEM select TMPFS + select DRM_DISPLAY_HDMI_HELPER select DRM_KMS_HELPER select DRM_PANEL select DRM_MIPI_DSI diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig index 4c10b400658c519c..7cc305b2826d6abc 100644 --- a/drivers/gpu/drm/nouveau/Kconfig +++ b/drivers/gpu/drm/nouveau/Kconfig @@ -3,12 +3,12 @@ config DRM_NOUVEAU tristate "Nouveau (NVIDIA) cards" depends on DRM depends on DRM_DISPLAY_DP_HELPER - depends on DRM_DISPLAY_HDMI_HELPER depends on DRM_DISPLAY_HELPER depends on PCI depends on MMU select IOMMU_API select FW_LOADER + select DRM_DISPLAY_HDMI_HELPER select DRM_KMS_HELPER select DRM_TTM select DRM_TTM_HELPER diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig index 6974caa99ece94e1..bb6e35261f1144b1 100644 --- a/drivers/gpu/drm/tegra/Kconfig +++ b/drivers/gpu/drm/tegra/Kconfig @@ -6,9 +6,9 @@ config DRM_TEGRA depends on DRM depends on DRM_DISPLAY_DP_AUX_BUS depends on DRM_DISPLAY_DP_HELPER - depends on DRM_DISPLAY_HDMI_HELPER depends on DRM_DISPLAY_HELPER depends on OF + select DRM_DISPLAY_HDMI_HELPER select DRM_KMS_HELPER select DRM_MIPI_DSI select DRM_PANEL diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig index 4801f8b64d3d70cd..98772a6b5bf0df54 100644 --- a/drivers/gpu/drm/vc4/Kconfig +++ b/drivers/gpu/drm/vc4/Kconfig @@ -4,13 +4,13 @@ config DRM_VC4 depends on ARCH_BCM || ARCH_BCM2835 || COMPILE_TEST depends on COMMON_CLK depends on DRM - depends on DRM_DISPLAY_HDMI_HELPER depends on DRM_DISPLAY_HELPER depends on PM # Make sure not 'y' when RASPBERRYPI_FI
[PATCH 01/11] Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies, part 2"
This reverts commit a57e191ebbaa0363dbf352cc37447c2230573e29, as the commits it fixes will be reverted, too. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/bridge/analogix/Kconfig | 2 +- drivers/gpu/drm/rockchip/Kconfig| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig index 5b564fded6d6c8e7..12bfea53bf24b2c2 100644 --- a/drivers/gpu/drm/bridge/analogix/Kconfig +++ b/drivers/gpu/drm/bridge/analogix/Kconfig @@ -28,7 +28,7 @@ config DRM_ANALOGIX_ANX78XX config DRM_ANALOGIX_DP tristate - depends on DRM_DISPLAY_HELPER + depends on DRM config DRM_ANALOGIX_ANX7625 tristate "Analogix Anx7625 MIPI to DP interface support" diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig index 4c7072e6e34eafb5..4b4ad75032fda17e 100644 --- a/drivers/gpu/drm/rockchip/Kconfig +++ b/drivers/gpu/drm/rockchip/Kconfig @@ -36,7 +36,7 @@ config ROCKCHIP_VOP2 config ROCKCHIP_ANALOGIX_DP bool "Rockchip specific extensions for Analogix DP driver" depends on DRM_DISPLAY_DP_HELPER - depends on DRM_DISPLAY_HELPER=y || (DRM_DISPLAY_HELPER=m && DRM_ROCKCHIP=m) + depends on DRM_DISPLAY_HELPER depends on ROCKCHIP_VOP help This selects support for Rockchip SoC specific extensions -- 2.34.1
[PATCH 00/11] drm: Restore helper usability
Hi all,
As discussed on IRC with Maxime and Arnd, this series reverts the
conversion of select to depends for various DRM helpers in series
"[PATCH v3 00/13] drm/display: Convert helpers Kconfig symbols to
depends on"[1], and various fixes for it. This conversion introduced a
big usability issue when configuring a kernel and enabling DRM drivers
that use DRM helper code: as drivers now depend on helpers, the user
needs to know which helpers to enable, before the driver he is
interested even becomes visible. The user should not need to know that,
and drivers should select the helpers they need.
Hence revert back to what we had before, where drivers selected the
helpers (and any of their dependencies, if they can be met) they need.
In general, when a symbol selects another symbol, it should just make
sure the dependencies of the target symbol are met, which may mean
adding dependencies to the source symbol.
Thanks for applying!
[1]
https://lore.kernel.org/r/20240327-kms-kconfig-helpers-v3-0-eafee11b8...@kernel.org/
Geert Uytterhoeven (11):
Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies, part 2"
Revert "drm/display: Select DRM_KMS_HELPER for DP helpers"
Revert "drm/bridge: dw-hdmi: Make DRM_DW_HDMI selectable"
Revert "drm: fix DRM_DISPLAY_DP_HELPER dependencies"
Revert "drm: Switch DRM_DISPLAY_HDMI_HELPER to depends on"
Revert "drm: Switch DRM_DISPLAY_HDCP_HELPER to depends on"
Revert "drm: Switch DRM_DISPLAY_DP_HELPER to depends on"
Revert "drm: Switch DRM_DISPLAY_DP_AUX_BUS to depends on"
Revert "drm: Switch DRM_DISPLAY_HELPER to depends on"
Revert "drm: Make drivers depends on DRM_DW_HDMI"
Revert "drm/display: Make all helpers visible and switch to depends
on"
drivers/gpu/drm/Kconfig | 8 +++
drivers/gpu/drm/amd/amdgpu/Kconfig | 12 --
drivers/gpu/drm/bridge/Kconfig | 28 +++---
drivers/gpu/drm/bridge/analogix/Kconfig | 18 +++---
drivers/gpu/drm/bridge/cadence/Kconfig | 8 +++
drivers/gpu/drm/bridge/imx/Kconfig | 4 ++--
drivers/gpu/drm/bridge/synopsys/Kconfig | 6 ++---
drivers/gpu/drm/display/Kconfig | 32 ++---
drivers/gpu/drm/exynos/Kconfig | 4 ++--
drivers/gpu/drm/i915/Kconfig| 8 +++
drivers/gpu/drm/imx/ipuv3/Kconfig | 5 ++--
drivers/gpu/drm/ingenic/Kconfig | 2 +-
drivers/gpu/drm/mediatek/Kconfig| 6 ++---
drivers/gpu/drm/meson/Kconfig | 2 +-
drivers/gpu/drm/msm/Kconfig | 8 +++
drivers/gpu/drm/nouveau/Kconfig | 10
drivers/gpu/drm/panel/Kconfig | 32 -
drivers/gpu/drm/radeon/Kconfig | 8 +++
drivers/gpu/drm/renesas/rcar-du/Kconfig | 2 +-
drivers/gpu/drm/rockchip/Kconfig| 10
drivers/gpu/drm/sun4i/Kconfig | 2 +-
drivers/gpu/drm/tegra/Kconfig | 8 +++
drivers/gpu/drm/vc4/Kconfig | 10
drivers/gpu/drm/xe/Kconfig | 13 --
drivers/gpu/drm/xlnx/Kconfig| 8 +++
25 files changed, 116 insertions(+), 138 deletions(-)
--
2.34.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH 10/11] Revert "drm: Make drivers depends on DRM_DW_HDMI"
This reverts commit c0e0f139354c01e0213204e4a96e7076e5a3e396, as helper code should always be selected by the driver that needs it, for the convenience of the final user configuring a kernel. The user who configures a kernel should not need to know which helpers are needed for the driver he is interested in. Making a driver depend on helper code means that the user needs to know which helpers to enable first, which is very user-unfriendly. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/bridge/imx/Kconfig | 4 ++-- drivers/gpu/drm/imx/ipuv3/Kconfig | 5 ++--- drivers/gpu/drm/ingenic/Kconfig | 2 +- drivers/gpu/drm/meson/Kconfig | 2 +- drivers/gpu/drm/renesas/rcar-du/Kconfig | 2 +- drivers/gpu/drm/rockchip/Kconfig| 2 +- drivers/gpu/drm/sun4i/Kconfig | 2 +- 7 files changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig index 7687ed652df5b9d3..5965e8027529a19c 100644 --- a/drivers/gpu/drm/bridge/imx/Kconfig +++ b/drivers/gpu/drm/bridge/imx/Kconfig @@ -5,9 +5,9 @@ config DRM_IMX_LDB_HELPER config DRM_IMX8MP_DW_HDMI_BRIDGE tristate "Freescale i.MX8MP HDMI-TX bridge support" - depends on COMMON_CLK - depends on DRM_DW_HDMI depends on OF + depends on COMMON_CLK + select DRM_DW_HDMI select DRM_IMX8MP_HDMI_PVI select PHY_FSL_SAMSUNG_HDMI_PHY help diff --git a/drivers/gpu/drm/imx/ipuv3/Kconfig b/drivers/gpu/drm/imx/ipuv3/Kconfig index 5d810ac02171d9c5..bacf0655ebaf3f5f 100644 --- a/drivers/gpu/drm/imx/ipuv3/Kconfig +++ b/drivers/gpu/drm/imx/ipuv3/Kconfig @@ -35,8 +35,7 @@ config DRM_IMX_LDB config DRM_IMX_HDMI tristate "Freescale i.MX DRM HDMI" - depends on DRM_DW_HDMI - depends on DRM_IMX - depends on OF + select DRM_DW_HDMI + depends on DRM_IMX && OF help Choose this if you want to use HDMI on i.MX6. diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig index 23effeb2ac721749..3db117c5edd9161b 100644 --- a/drivers/gpu/drm/ingenic/Kconfig +++ b/drivers/gpu/drm/ingenic/Kconfig @@ -27,8 +27,8 @@ config DRM_INGENIC_IPU config DRM_INGENIC_DW_HDMI tristate "Ingenic specific support for Synopsys DW HDMI" - depends on DRM_DW_HDMI depends on MACH_JZ4780 + select DRM_DW_HDMI help Choose this option to enable Synopsys DesignWare HDMI based driver. If you want to enable HDMI on Ingenic JZ4780 based SoC, you should diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig index 5520b9e3f0107c4d..615fdd0ce41b433a 100644 --- a/drivers/gpu/drm/meson/Kconfig +++ b/drivers/gpu/drm/meson/Kconfig @@ -13,9 +13,9 @@ config DRM_MESON config DRM_MESON_DW_HDMI tristate "HDMI Synopsys Controller support for Amlogic Meson Display" - depends on DRM_DW_HDMI depends on DRM_MESON default y if DRM_MESON + select DRM_DW_HDMI imply DRM_DW_HDMI_I2S_AUDIO config DRM_MESON_DW_MIPI_DSI diff --git a/drivers/gpu/drm/renesas/rcar-du/Kconfig b/drivers/gpu/drm/renesas/rcar-du/Kconfig index 2dc739db2ba33b99..53c356aed5d52070 100644 --- a/drivers/gpu/drm/renesas/rcar-du/Kconfig +++ b/drivers/gpu/drm/renesas/rcar-du/Kconfig @@ -25,8 +25,8 @@ config DRM_RCAR_CMM config DRM_RCAR_DW_HDMI tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support" depends on DRM && OF - depends on DRM_DW_HDMI depends on DRM_RCAR_DU || COMPILE_TEST + select DRM_DW_HDMI help Enable support for R-Car Gen3 or RZ/G2 internal HDMI encoder. diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig index 0d5260e10f272d3b..1bf3e2829cd07b6b 100644 --- a/drivers/gpu/drm/rockchip/Kconfig +++ b/drivers/gpu/drm/rockchip/Kconfig @@ -7,6 +7,7 @@ config DRM_ROCKCHIP select DRM_PANEL select VIDEOMODE_HELPERS select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP + select DRM_DW_HDMI if ROCKCHIP_DW_HDMI select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI @@ -56,7 +57,6 @@ config ROCKCHIP_CDN_DP config ROCKCHIP_DW_HDMI bool "Rockchip specific extensions for Synopsys DW HDMI" - depends on DRM_DW_HDMI help This selects support for Rockchip SoC specific extensions for the Synopsys DesignWare HDMI driver. If you want to diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig index 5b19c7cb7b7ebf53..4741d9f6544c201b 100644 --- a/drivers/gpu/drm/sun4i/Kconfig +++ b/drivers/gpu/drm/sun4i/Kconfig @@ -57,8 +57,8 @@ config DRM_SUN6I_DSI config DRM_SUN8I_DW_HDMI tristate "Support for Allwinner version of DesignWare HDMI" depend
[PATCH 06/11] Revert "drm: Switch DRM_DISPLAY_HDCP_HELPER to depends on"
This reverts commit 3166e7e6d935caaef07605a5c90773fbf9ffeaf4, as helper code should always be selected by the driver that needs it, for the convenience of the final user configuring a kernel. The user who configures a kernel should not need to know which helpers are needed for the driver he is interested in. Making a driver depend on helper code means that the user needs to know which helpers to enable first, which is very user-unfriendly. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/amd/amdgpu/Kconfig | 2 +- drivers/gpu/drm/bridge/Kconfig | 2 +- drivers/gpu/drm/bridge/analogix/Kconfig | 2 +- drivers/gpu/drm/bridge/cadence/Kconfig | 2 +- drivers/gpu/drm/display/Kconfig | 1 - drivers/gpu/drm/i915/Kconfig| 2 +- drivers/gpu/drm/xe/Kconfig | 2 +- 7 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index 1662dc49f18ed11e..ba09121e7debb247 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -4,13 +4,13 @@ config DRM_AMDGPU tristate "AMD GPU" depends on DRM depends on DRM_DISPLAY_DP_HELPER - depends on DRM_DISPLAY_HDCP_HELPER depends on DRM_DISPLAY_HELPER depends on MMU depends on PCI depends on !UML select FW_LOADER select DRM_DISPLAY_HDMI_HELPER + select DRM_DISPLAY_HDCP_HELPER select DRM_KMS_HELPER select DRM_SCHED select DRM_TTM diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index d1fbf8796fea8dbe..ad38cd201b006251 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -94,9 +94,9 @@ config DRM_ITE_IT6505 tristate "ITE IT6505 DisplayPort bridge" depends on DRM_DISPLAY_DP_AUX_BUS depends on DRM_DISPLAY_DP_HELPER - depends on DRM_DISPLAY_HDCP_HELPER depends on DRM_DISPLAY_HELPER depends on OF + select DRM_DISPLAY_HDCP_HELPER select DRM_KMS_HELPER select EXTCON select CRYPTO diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig index 12bfea53bf24b2c2..9659df6718de6db4 100644 --- a/drivers/gpu/drm/bridge/analogix/Kconfig +++ b/drivers/gpu/drm/bridge/analogix/Kconfig @@ -35,9 +35,9 @@ config DRM_ANALOGIX_ANX7625 depends on DRM depends on DRM_DISPLAY_DP_AUX_BUS depends on DRM_DISPLAY_DP_HELPER - depends on DRM_DISPLAY_HDCP_HELPER depends on DRM_DISPLAY_HELPER depends on OF + select DRM_DISPLAY_HDCP_HELPER select DRM_MIPI_DSI help ANX7625 is an ultra-low power 4K mobile HD transmitter diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig b/drivers/gpu/drm/bridge/cadence/Kconfig index 7817f6f56607141f..3480fd4d0a5f76e4 100644 --- a/drivers/gpu/drm/bridge/cadence/Kconfig +++ b/drivers/gpu/drm/bridge/cadence/Kconfig @@ -24,9 +24,9 @@ endif config DRM_CDNS_MHDP8546 tristate "Cadence DPI/DP bridge" depends on DRM_DISPLAY_DP_HELPER - depends on DRM_DISPLAY_HDCP_HELPER depends on DRM_DISPLAY_HELPER depends on OF + select DRM_DISPLAY_HDCP_HELPER select DRM_KMS_HELPER select DRM_PANEL_BRIDGE help diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index d65f1a37c08c7cf8..9801f47a3704530d 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -67,7 +67,6 @@ config DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG config DRM_DISPLAY_HDCP_HELPER bool "DRM HDCD Helpers" depends on DRM_DISPLAY_HELPER - default y help DRM display helpers for HDCP. diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 87ef8c4d72a53768..dbde4e29d93a79dc 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -3,7 +3,6 @@ config DRM_I915 tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics" depends on DRM depends on DRM_DISPLAY_DP_HELPER - depends on DRM_DISPLAY_HDCP_HELPER depends on DRM_DISPLAY_HELPER depends on X86 && PCI depends on !PREEMPT_RT @@ -13,6 +12,7 @@ config DRM_I915 # the shmem_readpage() which depends upon tmpfs select SHMEM select TMPFS + select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_KMS_HELPER select DRM_PANEL diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig index 1fa8ef75823c7a6b..02da2faf5ae33bdb 100644 --- a/drivers/gpu/drm/xe/Kconfig +++ b/drivers/gpu/drm/xe/Kconfig @@ -4,7 +4,6 @@ config DRM_XE depends on (m || (y && KUNIT=y)) depends on DRM depends on DRM_DISPLAY_DP_HELPER - depends on DRM_DISPLAY_HDCP_HELPER depends on DRM_DISPLAY_H
Re: [PATCH v6 3/5] drm: Add support to get EDID from ACPI
Hi Mario,
On Thu, Feb 15, 2024 at 8:04 PM Mario Limonciello
wrote:
> On 2/15/2024 12:47, Ville Syrjälä wrote:
> > On Thu, Feb 15, 2024 at 12:20:56PM -0600, Mario Limonciello wrote:
> >> On 2/14/2024 17:13, Ville Syrjälä wrote:
> >>> On Wed, Feb 14, 2024 at 03:57:54PM -0600, Mario Limonciello wrote:
> >>>> --- a/include/drm/drm_connector.h
> >>>> +++ b/include/drm/drm_connector.h
> >>>> @@ -1886,6 +1886,12 @@ struct drm_connector {
> >>>>
> >>>>/** @hdr_sink_metadata: HDR Metadata Information read from
> >>>> sink */
> >>>>struct hdr_sink_metadata hdr_sink_metadata;
> >>>> +
> >>>> + /**
> >>>> + * @acpi_edid_allowed: Get the EDID from the BIOS, if available.
> >>>> + * This is only applicable to eDP and LVDS displays.
> >>>> + */
> >>>> + bool acpi_edid_allowed;
> >>>
> >>> Aren't there other bools/small stuff in there for tighter packing?
> >>
> >> Does the compiler automatically do the packing if you put bools nearby
> >> in a struct? If so; TIL.
> >
> > Yes. Well, depends on the types and their alignment requirements
> > of course, and/or whether you specified __packed or not.
> >
> > You can use 'pahole' to find the holes in structures.
>
> Thanks! I don't see a __packed attribute on struct drm_connector, but
> I'll put it near by other bools in case that changes in the future.
FTR, don't add __packed unless you have a very good reason to do so.
With __packed, the compiler will emit multiple byte-accesses to
access multi-byte integrals on platforms that do not support unaligned
memory access.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 08/43] drm/fbdev: Add fbdev-shmem
Hi Thomas,
On Tue, Apr 16, 2024 at 2:07 PM Thomas Zimmermann wrote:
> Am 16.04.24 um 13:25 schrieb Javier Martinez Canillas:
> > Thomas Zimmermann writes:
> > Do I understand correctly that info->fix.smem_start doesn't have to be set
> > because that's only used for I/O memory?
>
> It's the start of the framebuffer memory in physical memory. Setting
> smem_start only makes sense if the framebuffer is physically continuous,
> which isn't the case here.
Nothing really needs fix.smem_start, it's mainly for informative use.
However, if smem_start is not page-aligned, userspace does need to
know the start offset inside the page (see below).
> > This also made me think why info->fix.smem_len is really needed. Can't we
> > make the fbdev core to only look at that if info->screen_size is not set ?
>
> The fbdev core doesn't use smem_len AFAICT. But smem_len is part of the
> fbdev UAPI, so I set it. I assume that programs use it to go to the end
> of the framebuffer memory.
On fbdev drivers also exporting MMIO to userspace, /dev/fbX contains
two parts: first the frame buffer, followed by the MMIO registers.
Both parts are an integral number of pages, based on fix.smem_{start,len}
resp. fix.mmio_{start,len}.
Old XFree86 used the MMIO part to implement hardware acceleration
when running on top of fbdev.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 00/13] drm/display: Convert helpers Kconfig symbols to depends on
Hi Jani,
On Tue, Apr 9, 2024 at 1:13 PM Jani Nikula wrote:
> On Tue, 09 Apr 2024, Geert Uytterhoeven wrote:
> > On Tue, Apr 9, 2024 at 12:04 PM Jani Nikula
> > wrote:
> >> On Tue, 09 Apr 2024, Geert Uytterhoeven wrote:
> >> > The user should not need to know which helpers are needed for the driver
> >> > he is interested in. When a symbol selects another symbol, it should
> >> > just make sure the dependencies of the target symbol are met.
> >>
> >> It's really not "just make sure". This leads to perpetual illegal
> >> configurations, and duct tape fixes. Select should not be used for
> >> visible symbols or symbols with dependencies [1].
> >
> > In other words: none of these helpers should be visible...
>
> ...and should have no dependencies? :p
Unless they do have dependencies.
> >> What we'd need for usability is not more abuse of select, but rather 1)
> >> warnings for selecting symbols with dependencies, and 2) a way to enable
> >
> > Kconfig already warns if dependencies of selected symbols are not met.
>
> But it does lead to cases where a builtin tries to use a symbol from a
> module, failing at link time, not config time. Then I regularly see
> patches trying to fix this with IS_REACHABLE(), making it a silent
> runtime failure instead, when it should've been a config issue.
If a symbol for a builtin selects a symbol for a module, the latter
becomes builtin, too, so that does not cause such issues?
You can get such issues when a boolean symbol depends on a tristate
symbol...
> >> a kconfig option with all its dependencies, recursively. This is what we
> >> lack.
> >
> > You cannot force-enable all dependencies of the target symbol, as some
> > of these dependencies may be impossible to meet on the system you are
> > configuring a kernel for.
>
> Surely kconfig should be able to figure out if they're possible or not.
>
> > The current proper way is to add these dependencies to the source
> > symbol, which is what we have been doing everywhere else. Another
> > solution may be to teach Kconfig to ignore any symbols that select a
> > symbol with unmet dependencies.
>
> ...
>
> It seems like your main argument in favour of using select is that it's
> more convenient for people who configure the kernel. Because the user
> should be able to just enable a driver, and that would select everything
> that's needed. But where do we draw the line? Then what qualifies for
> "depends on"?
Hard (platform and subsystem) dependencies.
> Look at config DRM_I915 and where select abuse has lead us. Like, why
> don't we just select DRM, PCI and X86 as well instead of depend. :p
X86 and PCI are hard platform dependencies.
DRM is a subsystem dependency.
> A lot of things we have to select because it appears to generally be the
> case that if some places select and some places depends on a symbol,
> it'll lead to circular dependencies.
True. So all library code (incl. helpers) should be selected, and
not be used as a dependency.
The user shouldn't be aware that the driver uses library code (or not).
> Sure there may be a usability issue with using depends on. But the
> proper fix isn't hacking in kconfig files, it's to fix the usability in
> kconfig the tool UI. But nobody steps up, because at least I find the
> kconfig source to be inpenetrable. I've tried many times, and given up.
As long as Kconfig does not handle dependencies of selected symbols
automatically, adding explicit dependencies to the origin symbols is
the only workable solution.
> I mean, if you want to enable a driver D, it could, at a minimum, show
> you a tree of (possibly alternative) things you also need to enable. But
And this series is actually making that harder, by turning all these
selects of helpers into dependencies...
> if the dependencies aren't there, you won't even see the config for
> D. That's not something that should be "fixed" by abusing select in
> kconfig files.
I consider not seeing symbols when a hard dependency is not met as
a good thing. If everything was visible all the time, you would
have a very hard (well, harder than the current already-hard ;-)
time configuring your kernel.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND v7 08/37] clocksource: sh_tmu: CLOCKSOURCE support.
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
wrote:
> Allows initialization as CLOCKSOURCE.
>
> Signed-off-by: Yoshinori Sato
Thanks for your patch!
> --- a/drivers/clocksource/sh_tmu.c
> +++ b/drivers/clocksource/sh_tmu.c
> @@ -495,7 +514,12 @@ static int sh_tmu_map_memory(struct sh_tmu_device *tmu)
>
> static int sh_tmu_parse_dt(struct sh_tmu_device *tmu)
> {
> - struct device_node *np = tmu->pdev->dev.of_node;
> + struct device_node *np;
Technically, np might be used uninitialized.
> +
> + if (tmu->pdev)
> + np = tmu->pdev->dev.of_node;
If you would set up tmu->np in sh_tmu_setup_pdev()...
> + if (tmu->np)
> + np = tmu->np;
... you could just assign np = tmu->np unconditionally.
>
> tmu->model = SH_TMU;
> tmu->num_channels = 3;
> @@ -665,6 +734,7 @@ static void __exit sh_tmu_exit(void)
> platform_driver_unregister(_tmu_device_driver);
> }
>
> +TIMER_OF_DECLARE(sh_tmu, "renesas,tmu", sh_tmu_of_register);
As there are now two entry points, the device is actually probed twice:
once from TIMER_OF_DECLARE/sh_tmu_of_register(), and a second
time from platform_driver/sh_tmu_probe().
E.g. on Armadillo-800-EVA with R-Mobile A1 (booting Linux on ARM
(not SH), and using TMU as the main clock source):
timer@fff8 ch0: used for clock events
timer@fff8 ch0: used for periodic clock events
timer@fff8 ch1: used as clock source
clocksource: timer@fff8: mask: 0x max_cycles:
0x, max_idle_ns: 154445288668 ns
...
fff8.timer ch0: used for clock events
genirq: Flags mismatch irq 16. 00015a04 (fff8.timer) vs.
00015a04 (timer@fff8)
fff8.timer ch0: failed to request irq 16
fff8.timer ch1: used as clock source
clocksource: fff8.timer: mask: 0x max_cycles:
0x, max_idle_ns: 154445288668 ns
After this, the timer seems to be stuck, and the boot is blocked.
On Marzen with R-Car H1 (booting Linux on ARM (not SH), and using
arm_global_timer as the main clock source), I also see the double
timer probe, but no such lock-up. I expect you to see the double
timer probe on SH775x, too?
The double probe can be fixed by adding a call to
of_node_set_flag(np, OF_POPULATED) at the end of sh_tmu_of_register()
in case of success, cfr. [1].
I haven't found the cause of the stuck timer on R-Mobile A1 yet;
both the TMU clock and the A4R power domain seem to be activated...
> #ifdef CONFIG_SUPERH
> sh_early_platform_init("earlytimer", _tmu_device_driver);
> #endif
[1] "[PATCH] clocksource/drivers/renesas-ostm: Avoid reprobe after
successful early probe"
https://lore.kernel.org/all/bd027379713cbaafa21ffe9e848ebb7f475ca0e7.1710930542.git.geert+rene...@glider.be/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 00/13] drm/display: Convert helpers Kconfig symbols to depends on
Hi Jani,
On Tue, Apr 9, 2024 at 12:04 PM Jani Nikula wrote:
> On Tue, 09 Apr 2024, Geert Uytterhoeven wrote:
> > The user should not need to know which helpers are needed for the driver
> > he is interested in. When a symbol selects another symbol, it should
> > just make sure the dependencies of the target symbol are met.
>
> It's really not "just make sure". This leads to perpetual illegal
> configurations, and duct tape fixes. Select should not be used for
> visible symbols or symbols with dependencies [1].
In other words: none of these helpers should be visible...
> What we'd need for usability is not more abuse of select, but rather 1)
> warnings for selecting symbols with dependencies, and 2) a way to enable
Kconfig already warns if dependencies of selected symbols are not met.
> a kconfig option with all its dependencies, recursively. This is what we
> lack.
You cannot force-enable all dependencies of the target symbol, as some
of these dependencies may be impossible to meet on the system you are
configuring a kernel for.
The current proper way is to add these dependencies to the source
symbol, which is what we have been doing everywhere else. Another
solution may be to teach Kconfig to ignore any symbols that select a
symbol with unmet dependencies.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 00/13] drm/display: Convert helpers Kconfig symbols to depends on
Hi Maxime,
On Wed, 27 Mar 2024, Maxime Ripard wrote:
Jani recently pointed out that the Kconfig symbols are a bit difficult
to work with at the moment when they depend on each other, and that
using depends on would be a better idea, but no one really did the work
so far.
So here it goes :)
It's been tested by comparing the riscv defconfig, arm
multi_v7_defconfig, arm64 defconfig, drm-misc-arm, drm-misc-arm64 and
drm-misc-x86 before and after this series and making sure they are
identical.
That is not true: comparing drm-misc/for-linux-next to v6.9-rc2,
arm/multi_v7_defconfig, arm64/defconfig, and riscv/defconfig lost
several of:
- CONFIG_DRM_DW_HDMI,
- CONFIG_DRM_DW_HDMI_AHB_AUDIO,
- CONFIG_DRM_DW_HDMI_CEC,
- CONFIG_DRM_DW_HDMI_I2S_AUDIO,
- CONFIG_DRM_IMX_HDMI.
- CONFIG_DRM_MESON_DW_HDMI,
- CONFIG_DRM_RCAR_DW_HDMI,
- CONFIG_DRM_SUN8I_DW_HDMI,
- CONFIG_ROCKCHIP_DW_HDMI,
- CONFIG_SND_MESON_G12A_TOHDMITX,
Let me know what you think,
IMHO this series looks like a big usuability issue for anyone
configuring the kernel, and you try to work around this by sprinkling
"default y" around.
The user should not need to know which helpers are needed for the driver
he is interested in. When a symbol selects another symbol, it should
just make sure the dependencies of the target symbol are met.
Thanks for reverting ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 09/13] drm: Switch DRM_DISPLAY_DP_AUX_BUS to depends on
Hi Maxime,
Thanks for your patch, which is now commit 4d15125d7fe637f4
("drm: Switch DRM_DISPLAY_DP_AUX_BUS to depends on") in
drm/drm-next (next-20240402 and later).
On Wed, 27 Mar 2024, Maxime Ripard wrote:
Most of our helpers have relied on being selected so far through
Kconfig, but that creates issues when we have multiple layers of helpers
with some depending on others.
Indeed, select doesn't select a dependency's dependencies, and thus
isn't super intuitive. Depends on however doesn't have that limitation,
(Almost?) Everywhere else we fixed that by also selecting the
dependencies, which is more user-friendly.
so we can just switch all the drivers that were selecting
DRM_DISPLAY_DP_AUX_BUS to depend on it.
Reviewed-by: Jani Nikula
Signed-off-by: Maxime Ripard
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -9,10 +9,11 @@ config DRM_DISPLAY_HELPER
config DRM_DISPLAY_DP_AUX_BUS
tristate "DRM DisplayPort AUX bus support"
depends on DRM
depends on OF || COMPILE_TEST
+ default y
(quoting Linus) "What is so special about your driver, that it needs to
default to enabled?".
Especially as there is no help available for this option, so the casual
user has no idea if this is needed or not.
And a general comment for this series: many defconfigs need to be
updated, as drivers are no longer enabled because they need
functionality that now needs to be enabled explicitly.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 20/21] drm/rcar-du: Allow build with COMPILE_TEST=y
On Mon, Apr 8, 2024 at 7:05 PM Ville Syrjala
wrote:
> From: Ville Syrjälä
>
> Allow rcar-du to be built with COMPILE_TEST=y for greater
> coverage. Builds fine on x86/x86_64 at least.
Also on rv64 and m68k.
> Cc: Laurent Pinchart
> Cc: Kieran Bingham
> Cc: linux-renesas-...@vger.kernel.org
> Signed-off-by: Ville Syrjälä
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND v7 21/37] dt-bindings: serial: renesas, scif: Add scif-sh7751.
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
wrote:
> Add Renesas SH7751 SCIF.
>
> Signed-off-by: Yoshinori Sato
> Reviewed-by: Geert Uytterhoeven
> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> @@ -18,6 +18,7 @@ properties:
>- items:
>- enum:
>- renesas,scif-r7s72100 # RZ/A1H
> + - renesas,scif-sh7751 # SH7751
>- const: renesas,scif # generic SCIF compatible UART
>
>- items:
If this is applied after "[PATCH v2 2/2] dt-bindings: serial:
renesas,scif: Validate 'interrupts' and 'interrupt-names'"[1], an extra
"- renesas,scif-sh7751" line should be added to the 4-interrupt section
(below "- renesas,scif-r7s72100").
[1]
https://lore.kernel.org/all/20240307114217.34784-3-prabhakar.mahadev-lad...@bp.renesas.com/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND v7 28/37] dt-bindings: soc: renesas: sh: Add SH7751 based target
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
wrote:
> Signed-off-by: Yoshinori Sato
Thanks for the update!
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/renesas/sh.yaml
> + compatible:
> +oneOf:
As adding more SoCs is expected, having oneOf from the start is fine.
> + - description: SH7751R based platform
> +items:
> + - enum:
> + - renesas,rts7751r2d # Renesas SH4 2D graphics board
> + - iodata,landisk # LANDISK HDL-U
> + - iodata,usl-5p # USL-5P
> + - const: renesas,sh7751r
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND v7 23/37] dt-bindings: display: sm501 register definition helper
Hi Sato-san,
Thanks for your patch!
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
wrote:
> Miscellaneous Timing and Miscellaneous Control registers definition.
Please do not put raw register value definitions into DT bindings.
> Signed-off-by: Yoshinori Sato
> --- /dev/null
> +++ b/include/dt-bindings/display/sm501.h
> +/* Miscellaneous timing */
> +#define SM501_MISC_TIMING_EX_HOLD_00
> +#define SM501_MISC_TIMING_EX_HOLD_16 1
> +#define SM501_MISC_TIMING_EX_HOLD_32 2
> +#define SM501_MISC_TIMING_EX_HOLD_48 3
> +#define SM501_MISC_TIMING_EX_HOLD_64 4
> +#define SM501_MISC_TIMING_EX_HOLD_80 5
> +#define SM501_MISC_TIMING_EX_HOLD_96 6
> +#define SM501_MISC_TIMING_EX_HOLD_112 7
> +#define SM501_MISC_TIMING_EX_HOLD_128 8
> +#define SM501_MISC_TIMING_EX_HOLD_144 9
> +#define SM501_MISC_TIMING_EX_HOLD_160 10
> +#define SM501_MISC_TIMING_EX_HOLD_176 11
> +#define SM501_MISC_TIMING_EX_HOLD_192 12
> +#define SM501_MISC_TIMING_EX_HOLD_208 13
> +#define SM501_MISC_TIMING_EX_HOLD_224 14
> +#define SM501_MISC_TIMING_EX_HOLD_240 15
E.g. these are used by the (not very descriptive) "ex" property:
ex:
$ref: /schemas/types.yaml#/definitions/uint32
description: Extend bus holding time.
Please instead use an enum for the actual holding time ([ 0, 16, 32,
...]) in the DT bindings, and convert from actual holding time to
register value in the driver.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND v7 14/37] clk: Compatible with narrow registers
K_DIVIDER_REG_16BIT - by default 32bit register accesses are used for
> + * the gate register. Setting this flag makes the register accesses
> 16bit.
> */
> struct clk_divider {
> struct clk_hw hw;
> void __iomem*reg;
> u8 shift;
> u8 width;
> - u8 flags;
> + u16 flags;
> const struct clk_div_table *table;
> spinlock_t *lock;
> };
> @@ -726,18 +738,18 @@ struct clk_hw *__clk_hw_register_divider(struct device
> *dev,
> struct device_node *np, const char *name,
> const char *parent_name, const struct clk_hw *parent_hw,
> const struct clk_parent_data *parent_data, unsigned long
> flags,
> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
> + void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
"u16 clk_divider_flags", to match clk_divider.flags.
> const struct clk_div_table *table, spinlock_t *lock);
> struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
> struct device_node *np, const char *name,
> const char *parent_name, const struct clk_hw *parent_hw,
> const struct clk_parent_data *parent_data, unsigned long
> flags,
> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
> + void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
Likewise.
> const struct clk_div_table *table, spinlock_t *lock);
> struct clk *clk_register_divider_table(struct device *dev, const char *name,
> const char *parent_name, unsigned long flags,
> void __iomem *reg, u8 shift, u8 width,
> - u8 clk_divider_flags, const struct clk_div_table *table,
> + u32 clk_divider_flags, const struct clk_div_table *table,
Likewise.
> spinlock_t *lock);
> /**
> * clk_register_divider - register a divider clock with the clock framework
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND v7 34/37] sh: Add dtbs target support.
On Thu, Apr 4, 2024 at 7:16 AM Yoshinori Sato
wrote:
> Signed-off-by: Yoshinori Sato
My
Reviewed-by: Geert Uytterhoeven
on v6 is still valid.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND v7 33/37] sh: j2_mimas_v2.dts update
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:16 AM Yoshinori Sato
wrote:
> Signed-off-by: Yoshinori Sato
>From my comments for v6:
Please enhance the one-line summary, e.g.
sh: j2_mimas_v2: Update CPU compatible value
For the actual changes:
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND v7 13/37] dt-bindings: clock: sh7750-cpg: Add renesas,sh7750-cpg header.
Hi Sato-san,
Thanks for the update!
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
wrote:
>
> SH7750 CPG Clock output define.
(from my comments on v6) Please improve the patch description.
> Signed-off-by: Yoshinori Sato
> index ..04c10b0834ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,sh7750-cpg.yaml
The actual bindings LGTM.
> +examples:
> + - |
> +#include
> +cpg: clock-controller@ffc0 {
> +#clock-cells = <1>;
> +#power-domain-cells = <0>;
> +compatible = "renesas,sh7751r-cpg";
> +clocks = <>;
> +clock-names = "extal";
> +reg = <0xffc0 20>, <0xfe0a 16>;
> +reg-names = "FRQCR", "CLKSTP00";
> +renesas,mode = <0>;
> +};
Please read
https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND v7 09/37] dt-binding: Add compatible SH7750 SoC
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
wrote:
> Signed-off-by: Yoshinori Sato
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH] drm: DRM_DEBUG_MODESET_LOCK should depend on DRM
There is no point in asking the user about enabling DRM debug tracing when configuring a kernel without DRM support. Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/Kconfig | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 2e1b23ccf30423a9..a24c48acf235449a 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -119,9 +119,7 @@ config DRM_DEBUG_DP_MST_TOPOLOGY_REFS config DRM_DEBUG_MODESET_LOCK bool "Enable backtrace history for lock contention" - depends on STACKTRACE_SUPPORT - depends on DEBUG_KERNEL - depends on EXPERT + depends on DRM && STACKTRACE_SUPPORT && DEBUG_KERNEL && EXPERT select STACKDEPOT default y if DEBUG_WW_MUTEX_SLOWPATH help -- 2.34.1
[PATCH] drm: DRM_WERROR should depend on DRM
There is no point in asking the user about enforcing the DRM compiler
warning policy when configuring a kernel without DRM support.
Fixes: f89632a9e5fa6c47 ("drm: Add CONFIG_DRM_WERROR")
Signed-off-by: Geert Uytterhoeven
---
drivers/gpu/drm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index f2bcf5504aa77679..2e1b23ccf30423a9 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -423,7 +423,7 @@ config DRM_PRIVACY_SCREEN
config DRM_WERROR
bool "Compile the drm subsystem with warnings as errors"
- depends on EXPERT
+ depends on DRM && EXPERT
default n
help
A kernel build should not cause any compiler warnings, and this
--
2.34.1
Re: Build regressions/improvements in v6.9-rc1
On Mon, 25 Mar 2024, Geert Uytterhoeven wrote:
Below is the list of build error/warning regressions/improvements in
v6.9-rc1[1] compared to v6.8[2].
Summarized:
- build errors: +8/-8
+ /kisskb/src/crypto/scompress.c: error: unused variable 'dst_page'
[-Werror=unused-variable]: => 174:38
xtensa-gcc13/xtensa-allmodconfig
+ /kisskb/src/drivers/gpu/drm/msm/adreno/adreno_gen7_0_0_snapshot.h: error:
'gen7_0_0_external_core_regs' defined but not used [-Werror=unused-variable]:
=> 924:19
+ /kisskb/src/drivers/gpu/drm/msm/adreno/adreno_gen7_2_0_snapshot.h: error:
'gen7_2_0_external_core_regs' defined but not used [-Werror=unused-variable]:
=> 748:19
arm64-gcc5/arm64-allmodconfig
powerpc-gcc5/powerpc-allmodconfig
powerpc-gcc5/powerpc-allyesconfig
powerpc-gcc5/ppc32_allmodconfig
powerpc-gcc5/ppc64_book3e_allmodconfig
powerpc-gcc5/ppc64le_allmodconfig
sparc64-gcc5/sparc64-allmodconfig
+ /kisskb/src/drivers/gpu/drm/xe/xe_lrc.c: error: "END" redefined [-Werror]:
=> 100
mips-gcc8/mips-allmodconfig
mips-gcc13/mips-allmodconfig
+ error: arch/sparc/kernel/process_32.o: relocation truncated to fit:
R_SPARC_WDISP22 against `.text': => (.fixup+0xc), (.fixup+0x4)
+ error: arch/sparc/kernel/signal_32.o: relocation truncated to fit:
R_SPARC_WDISP22 against `.text': => (.fixup+0x18), (.fixup+0x8), (.fixup+0x0),
(.fixup+0x20), (.fixup+0x10)
+ error: relocation truncated to fit: R_SPARC_WDISP22 against `.init.text':
=> (.head.text+0x5100), (.head.text+0x5040)
+ error: relocation truncated to fit: R_SPARC_WDISP22 against symbol
`leon_smp_cpu_startup' defined in .text section in
arch/sparc/kernel/trampoline_32.o: => (.init.text+0xa4)
sparc64-gcc13/sparc-allmodconfig
[1]
http://kisskb.ellerman.id.au/kisskb/branch/linus/head/4cece764965020c22cff7665b18a012006359095/
(all 138 configs)
[2]
http://kisskb.ellerman.id.au/kisskb/branch/linus/head/e8f897f4afef0031fe618a8e94127a0934896aba/
(all 138 configs)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 00/14] Add support for suppressing warning backtraces
Hi Günter,
On Tue, Mar 12, 2024 at 6:03 PM Guenter Roeck wrote:
> Some unit tests intentionally trigger warning backtraces by passing bad
> parameters to kernel API functions. Such unit tests typically check the
> return value from such calls, not the existence of the warning backtrace.
>
> Such intentionally generated warning backtraces are neither desirable
> nor useful for a number of reasons.
> - They can result in overlooked real problems.
> - A warning that suddenly starts to show up in unit tests needs to be
> investigated and has to be marked to be ignored, for example by
> adjusting filter scripts. Such filters are ad-hoc because there is
> no real standard format for warnings. On top of that, such filter
> scripts would require constant maintenance.
>
> One option to address problem would be to add messages such as "expected
> warning backtraces start / end here" to the kernel log. However, that
> would again require filter scripts, it might result in missing real
> problematic warning backtraces triggered while the test is running, and
> the irrelevant backtrace(s) would still clog the kernel log.
>
> Solve the problem by providing a means to identify and suppress specific
> warning backtraces while executing test code. Support suppressing multiple
> backtraces while at the same time limiting changes to generic code to the
> absolute minimum. Architecture specific changes are kept at minimum by
> retaining function names only if both CONFIG_DEBUG_BUGVERBOSE and
> CONFIG_KUNIT are enabled.
>
> The first patch of the series introduces the necessary infrastructure.
> The second patch introduces support for counting suppressed backtraces.
> This capability is used in patch three to implement unit tests.
> Patch four documents the new API.
> The next two patches add support for suppressing backtraces in drm_rect
> and dev_addr_lists unit tests. These patches are intended to serve as
> examples for the use of the functionality introduced with this series.
> The remaining patches implement the necessary changes for all
> architectures with GENERIC_BUG support.
Thanks for your series!
I gave it a try on m68k, just running backtrace-suppression-test,
and that seems to work fine.
> Design note:
> Function pointers are only added to the __bug_table section if both
> CONFIG_KUNIT and CONFIG_DEBUG_BUGVERBOSE are enabled to avoid image
> size increases if CONFIG_KUNIT=n. There would be some benefits to
> adding those pointers all the time (reduced complexity, ability to
> display function names in BUG/WARNING messages). That change, if
> desired, can be made later.
Unfortunately this also increases kernel size in the CONFIG_KUNIT=m
case (ca. 80 KiB for atari_defconfig), making it less attractive to have
kunit and all tests enabled as modules in my standard kernel.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: arm: ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
On Thu, Mar 14, 2024 at 10:27 AM Geert Uytterhoeven
wrote:
> On Sat, Mar 9, 2024 at 3:34 PM David Laight wrote:
> > From: Maxime Ripard
> > > Sent: 04 March 2024 11:46
> > >
> > > On Mon, Mar 04, 2024 at 12:11:36PM +0100, Arnd Bergmann wrote:
> > > > On Mon, Mar 4, 2024, at 09:07, Naresh Kamboju wrote:
> > > > > The arm defconfig builds failed on today's Linux next tag
> > > > > next-20240304.
> > > > >
> > > > > Build log:
> > > > > -
> > > > > ERROR: modpost: "__aeabi_uldivmod"
> > > > > [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
> > > > >
> > > >
> > > > Apparently caused by the 64-bit division in 358e76fd613a
> > > > ("drm/sun4i: hdmi: Consolidate atomic_check and mode_valid"):
> > > >
> > > >
> > > > +static enum drm_mode_status
> > > > +sun4i_hdmi_connector_clock_valid(const struct drm_connector *connector,
> > > > +const struct drm_display_mode *mode,
> > > > +unsigned long long clock)
> > > > {
> > > > - struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);
> > > > - unsigned long rate = mode->clock * 1000;
> > > > - unsigned long diff = rate / 200; /* +-0.5% allowed by HDMI spec
> > > > */
> > > > + const struct sun4i_hdmi *hdmi =
> > > > drm_connector_to_sun4i_hdmi(connector);
> > > > + unsigned long diff = clock / 200; /* +-0.5% allowed by HDMI
> > > > spec */
> > > > long rounded_rate;
> > > >
> > > > This used to be a 32-bit division. If the rate is never more than
> > > > 4.2GHz, clock could be turned back into 'unsigned long' to avoid
> > > > the expensive div_u64().
> > >
> > > I sent a fix for it this morning:
> > > https://lore.kernel.org/r/20240304091225.366325-1-mrip...@kernel.org
> > >
> > > The framework will pass an unsigned long long because HDMI character
> > > rates can go up to 5.9GHz.
> >
> > You could do:
> > /* The max clock is 5.9GHz, split the divide */
> > u32 diff = (u32)(clock / 8) / (200/8);
>
> +1, as the issue is still present in current next, as per the recent
> nagging from the build bots.
Oops, s/next/upstream/.
Well, less 32-bit build testing for the next few days :-(
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: arm: ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
On Sat, Mar 9, 2024 at 3:34 PM David Laight wrote:
> From: Maxime Ripard
> > Sent: 04 March 2024 11:46
> >
> > On Mon, Mar 04, 2024 at 12:11:36PM +0100, Arnd Bergmann wrote:
> > > On Mon, Mar 4, 2024, at 09:07, Naresh Kamboju wrote:
> > > > The arm defconfig builds failed on today's Linux next tag next-20240304.
> > > >
> > > > Build log:
> > > > -
> > > > ERROR: modpost: "__aeabi_uldivmod"
> > > > [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
> > > >
> > >
> > > Apparently caused by the 64-bit division in 358e76fd613a
> > > ("drm/sun4i: hdmi: Consolidate atomic_check and mode_valid"):
> > >
> > >
> > > +static enum drm_mode_status
> > > +sun4i_hdmi_connector_clock_valid(const struct drm_connector *connector,
> > > +const struct drm_display_mode *mode,
> > > +unsigned long long clock)
> > > {
> > > - struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);
> > > - unsigned long rate = mode->clock * 1000;
> > > - unsigned long diff = rate / 200; /* +-0.5% allowed by HDMI spec */
> > > + const struct sun4i_hdmi *hdmi =
> > > drm_connector_to_sun4i_hdmi(connector);
> > > + unsigned long diff = clock / 200; /* +-0.5% allowed by HDMI spec
> > > */
> > > long rounded_rate;
> > >
> > > This used to be a 32-bit division. If the rate is never more than
> > > 4.2GHz, clock could be turned back into 'unsigned long' to avoid
> > > the expensive div_u64().
> >
> > I sent a fix for it this morning:
> > https://lore.kernel.org/r/20240304091225.366325-1-mrip...@kernel.org
> >
> > The framework will pass an unsigned long long because HDMI character
> > rates can go up to 5.9GHz.
>
> You could do:
> /* The max clock is 5.9GHz, split the divide */
> u32 diff = (u32)(clock / 8) / (200/8);
+1, as the issue is still present in current next, as per the recent
nagging from the build bots.
> The code should really use u32 and u64.
> Otherwise the sizes are different on 32bit.
The code is already using a variety of types (long, unsigned long long,
unsigned long) :-(
max_t(unsigned long, rounded_rate, clock) -
min_t(unsigned long, rounded_rate, clock) < diff)
At least u64 should make it very clear clock does not fit in 32-bit.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 1/6] auxdisplay/ht16k33: Replace use of fb_blank with backlight helper
On Wed, Mar 13, 2024 at 4:49 PM Thomas Zimmermann wrote:
> Replace the use of struct backlight_properties.fb_blank with a
> call to backlight_get_brightness(). The helper implement the same
> logic as the driver's function.
>
> Signed-off-by: Thomas Zimmermann
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 11/14] s390: Add support for suppressing warning backtraces
Hi Günter,
On Tue, Mar 12, 2024 at 6:06 PM Guenter Roeck wrote:
> Add name of functions triggering warning backtraces to the __bug_table
> object section to enable support for suppressing WARNING backtraces.
>
> To limit image size impact, the pointer to the function name is only added
> to the __bug_table section if both CONFIG_KUNIT and CONFIG_DEBUG_BUGVERBOSE
> are enabled. Otherwise, the __func__ assembly parameter is replaced with a
> (dummy) NULL parameter to avoid an image size increase due to unused
> __func__ entries (this is necessary because __func__ is not a define but a
> virtual variable).
>
> Signed-off-by: Guenter Roeck
Thanks for your patch!
> --- a/arch/s390/include/asm/bug.h
> +++ b/arch/s390/include/asm/bug.h
> @@ -8,19 +8,30 @@
>
> #ifdef CONFIG_DEBUG_BUGVERBOSE
>
> +#if IS_ENABLED(CONFIG_KUNIT)
> +# define HAVE_BUG_FUNCTION
> +# define __BUG_FUNC_PTR" .long %0-.\n"
> +# define __BUG_FUNC__func__
> +#else
> +# define __BUG_FUNC_PTR
> +# define __BUG_FUNCNULL
> +#endif /* IS_ENABLED(CONFIG_KUNIT) */
> +
> #define __EMIT_BUG(x) do { \
> asm_inline volatile(\
> "0: mc 0,0\n" \
> ".section .rodata.str,\"aMS\",@progbits,1\n"\
> "1: .asciz \""__FILE__"\"\n" \
> ".previous\n" \
> - ".section __bug_table,\"awM\",@progbits,%2\n" \
> + ".section __bug_table,\"awM\",@progbits,%3\n" \
This change conflicts with commit 3938490e78f443fb ("s390/bug:
remove entry size from __bug_table section") in linus/master.
I guess it should just be dropped?
> "2: .long 0b-.\n" \
> " .long 1b-.\n" \
> - " .short %0,%1\n"\
> - " .org2b+%2\n"\
> + __BUG_FUNC_PTR \
> + " .short %1,%2\n"\
> + " .org2b+%3\n"\
> ".previous\n" \
> - : : "i" (__LINE__), \
> + : : "i" (__BUG_FUNC), \
> + "i" (__LINE__), \
> "i" (x),\
> "i" (sizeof(struct bug_entry)));\
> } while (0)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 08/43] drm/fbdev: Add fbdev-shmem
Hi Thomas,
On Wed, Mar 13, 2024 at 10:24 AM Thomas Zimmermann wrote:
> Am 13.03.24 um 10:03 schrieb Geert Uytterhoeven:
> > On Wed, Mar 13, 2024 at 9:19 AM Thomas Zimmermann
> > wrote:
> >> Am 12.03.24 um 17:14 schrieb Geert Uytterhoeven:
> >>> On Tue, Mar 12, 2024 at 4:48 PM Thomas Zimmermann
> >>> wrote:
> >>>> Add an fbdev emulation for SHMEM-based memory managers. The code is
> >>>> similar to fbdev-generic, but does not require an addition shadow
> >>>> buffer for mmap(). Fbdev-shmem operates directly on the buffer object's
> >>>> SHMEM pages. Fbdev's deferred-I/O mechanism updates the hardware state
> >>>> on write operations.
> >>>>
> >>>> Signed-off-by: Thomas Zimmermann
> >>> Thanks for your patch!
> >>>
> >>>> --- /dev/null
> >>>> +++ b/drivers/gpu/drm/drm_fbdev_shmem.c
> >>>> +static int drm_fbdev_shmem_helper_fb_probe(struct drm_fb_helper
> >>>> *fb_helper,
> >>>> + struct
> >>>> drm_fb_helper_surface_size *sizes)
> >>>> +{
> >>>> + struct drm_client_dev *client = _helper->client;
> >>>> + struct drm_device *dev = fb_helper->dev;
> >>>> + struct drm_client_buffer *buffer;
> >>>> + struct drm_gem_shmem_object *shmem;
> >>>> + struct drm_framebuffer *fb;
> >>>> + struct fb_info *info;
> >>>> + u32 format;
> >>>> + struct iosys_map map;
> >>>> + int ret;
> >>>> +
> >>>> + drm_dbg_kms(dev, "surface width(%d), height(%d) and bpp(%d)\n",
> >>>> + sizes->surface_width, sizes->surface_height,
> >>>> + sizes->surface_bpp);
> >>>> +
> >>>> + format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> >>>> sizes->surface_depth);
> >>> Oops, one more caller of the imprecise
> >>> let's-guess-the-format-from-bpp-and-depth machinery to get rid of...
> >> Right, that has been discussed in another thread. I'll change this call
> >> to the drm_driver_() function.
> > You mean drm_driver_legacy_fb_format()? That has the same issues.
>
> We have the video= parameter with its bpp argument. So we won't ever
> fully remove that function.
For that to work with monochrome and greyscale displays, it should
fall back to DRM_FORMAT_Rx (or _Dx) if depth <= 8 and DRM_FORMAT_Cx
is not supported by the underlying primary plane or driver.
Hmm, perhaps I should indeed implement the fallback in
drm_driver_legacy_fb_format() instead of drm_fb_helper_find_format()
(like I did in [1]).
> >>>> + buffer = drm_client_framebuffer_create(client,
> >>>> sizes->surface_width,
> >>>> + sizes->surface_height,
> >>>> format);
> >>> [...]
> >>>
> >>>> +}
> >>>> +/**
> >>>> + * drm_fbdev_shmem_setup() - Setup fbdev emulation for GEM SHMEM helpers
> >>>> + * @dev: DRM device
> >>>> + * @preferred_bpp: Preferred bits per pixel for the device.
> >>>> + * 32 is used if this is zero.
> >>>> + *
> >>>> + * This function sets up fbdev emulation for GEM DMA drivers that
> >>>> support
> >>>> + * dumb buffers with a virtual address and that can be mmap'ed.
> >>>> + * drm_fbdev_shmem_setup() shall be called after the DRM driver
> >>>> registered
> >>>> + * the new DRM device with drm_dev_register().
> >>>> + *
> >>>> + * Restore, hotplug events and teardown are all taken care of. Drivers
> >>>> that do
> >>>> + * suspend/resume need to call drm_fb_helper_set_suspend_unlocked()
> >>>> themselves.
> >>>> + * Simple drivers might use drm_mode_config_helper_suspend().
> >>>> + *
> >>>> + * This function is safe to call even when there are no connectors
> >>>> present.
> >>>> + * Setup will be retried on the next hotplug event.
> >>>> + *
> >>>> + * The fbdev is destroyed by drm_dev_unregister().
> >>>> + */
> >>>> +void drm_fbdev_shmem_setup(struct drm_device *dev, unsigned int
> >>>> preferred_bpp)
> >>> As t
Re: [PATCH 08/43] drm/fbdev: Add fbdev-shmem
Hi Thomas,
On Wed, Mar 13, 2024 at 9:19 AM Thomas Zimmermann wrote:
> Am 12.03.24 um 17:14 schrieb Geert Uytterhoeven:
> > On Tue, Mar 12, 2024 at 4:48 PM Thomas Zimmermann
> > wrote:
> >> Add an fbdev emulation for SHMEM-based memory managers. The code is
> >> similar to fbdev-generic, but does not require an addition shadow
> >> buffer for mmap(). Fbdev-shmem operates directly on the buffer object's
> >> SHMEM pages. Fbdev's deferred-I/O mechanism updates the hardware state
> >> on write operations.
> >>
> >> Signed-off-by: Thomas Zimmermann
> > Thanks for your patch!
> >
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/drm_fbdev_shmem.c
> >> +static int drm_fbdev_shmem_helper_fb_probe(struct drm_fb_helper
> >> *fb_helper,
> >> + struct
> >> drm_fb_helper_surface_size *sizes)
> >> +{
> >> + struct drm_client_dev *client = _helper->client;
> >> + struct drm_device *dev = fb_helper->dev;
> >> + struct drm_client_buffer *buffer;
> >> + struct drm_gem_shmem_object *shmem;
> >> + struct drm_framebuffer *fb;
> >> + struct fb_info *info;
> >> + u32 format;
> >> + struct iosys_map map;
> >> + int ret;
> >> +
> >> + drm_dbg_kms(dev, "surface width(%d), height(%d) and bpp(%d)\n",
> >> + sizes->surface_width, sizes->surface_height,
> >> + sizes->surface_bpp);
> >> +
> >> + format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> >> sizes->surface_depth);
> > Oops, one more caller of the imprecise
> > let's-guess-the-format-from-bpp-and-depth machinery to get rid of...
>
> Right, that has been discussed in another thread. I'll change this call
> to the drm_driver_() function.
You mean drm_driver_legacy_fb_format()? That has the same issues.
> >> + buffer = drm_client_framebuffer_create(client,
> >> sizes->surface_width,
> >> + sizes->surface_height,
> >> format);
> > [...]
> >
> >> +}
> >> +/**
> >> + * drm_fbdev_shmem_setup() - Setup fbdev emulation for GEM SHMEM helpers
> >> + * @dev: DRM device
> >> + * @preferred_bpp: Preferred bits per pixel for the device.
> >> + * 32 is used if this is zero.
> >> + *
> >> + * This function sets up fbdev emulation for GEM DMA drivers that support
> >> + * dumb buffers with a virtual address and that can be mmap'ed.
> >> + * drm_fbdev_shmem_setup() shall be called after the DRM driver registered
> >> + * the new DRM device with drm_dev_register().
> >> + *
> >> + * Restore, hotplug events and teardown are all taken care of. Drivers
> >> that do
> >> + * suspend/resume need to call drm_fb_helper_set_suspend_unlocked()
> >> themselves.
> >> + * Simple drivers might use drm_mode_config_helper_suspend().
> >> + *
> >> + * This function is safe to call even when there are no connectors
> >> present.
> >> + * Setup will be retried on the next hotplug event.
> >> + *
> >> + * The fbdev is destroyed by drm_dev_unregister().
> >> + */
> >> +void drm_fbdev_shmem_setup(struct drm_device *dev, unsigned int
> >> preferred_bpp)
> > As this is a completely new function, can we please get a
> > preferred_format parameter instead?
>
> An understandable question. But as it is, the patchset has a trivial
> change in each driver. And the preferred_bpp parameter has the same
> meaning as the bpp value in the video= parameter. So it's ok-ish for now.
OK.
> Using a format parameter here is really a much larger update and touches
> the internals of the fbdev emulation. I'm not even sure that we should
> have a parameter at all. Since in-kernel clients should behave like
> userspace clients, we could try to figure out the format from the
> driver's primary planes. That's a patchset of its own.
How do you figure out "the" format from the driver's primary plane?
Isn't that a list of formats (always including XR24) , so the driver
still needs to specify a preferred format?
A while ago, I had a look into replacing preferred_{depth,bpp} by
preferred_format, but I was held back by the inconsistencies in some
drivers (e.g. depth 24 vs. 32). Perhaps an incremental approach
(use preferred_format if available, else fall back to legacy
preferred_{depth,bpp} handling) would be more suitable?
FTR, my main use-case is letting fbdev emulation distinguish between
DRM_FORMAT_Rx and DRM_FORMAT_Cx, which share the values of depth
and bpp.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 08/43] drm/fbdev: Add fbdev-shmem
Hi Thomas,
On Tue, Mar 12, 2024 at 4:48 PM Thomas Zimmermann wrote:
> Add an fbdev emulation for SHMEM-based memory managers. The code is
> similar to fbdev-generic, but does not require an addition shadow
> buffer for mmap(). Fbdev-shmem operates directly on the buffer object's
> SHMEM pages. Fbdev's deferred-I/O mechanism updates the hardware state
> on write operations.
>
> Signed-off-by: Thomas Zimmermann
Thanks for your patch!
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_fbdev_shmem.c
> +static int drm_fbdev_shmem_helper_fb_probe(struct drm_fb_helper *fb_helper,
> + struct drm_fb_helper_surface_size
> *sizes)
> +{
> + struct drm_client_dev *client = _helper->client;
> + struct drm_device *dev = fb_helper->dev;
> + struct drm_client_buffer *buffer;
> + struct drm_gem_shmem_object *shmem;
> + struct drm_framebuffer *fb;
> + struct fb_info *info;
> + u32 format;
> + struct iosys_map map;
> + int ret;
> +
> + drm_dbg_kms(dev, "surface width(%d), height(%d) and bpp(%d)\n",
> + sizes->surface_width, sizes->surface_height,
> + sizes->surface_bpp);
> +
> + format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> sizes->surface_depth);
Oops, one more caller of the imprecise
let's-guess-the-format-from-bpp-and-depth machinery to get rid of...
> + buffer = drm_client_framebuffer_create(client, sizes->surface_width,
> + sizes->surface_height, format);
[...]
> +}
> +/**
> + * drm_fbdev_shmem_setup() - Setup fbdev emulation for GEM SHMEM helpers
> + * @dev: DRM device
> + * @preferred_bpp: Preferred bits per pixel for the device.
> + * 32 is used if this is zero.
> + *
> + * This function sets up fbdev emulation for GEM DMA drivers that support
> + * dumb buffers with a virtual address and that can be mmap'ed.
> + * drm_fbdev_shmem_setup() shall be called after the DRM driver registered
> + * the new DRM device with drm_dev_register().
> + *
> + * Restore, hotplug events and teardown are all taken care of. Drivers that
> do
> + * suspend/resume need to call drm_fb_helper_set_suspend_unlocked()
> themselves.
> + * Simple drivers might use drm_mode_config_helper_suspend().
> + *
> + * This function is safe to call even when there are no connectors present.
> + * Setup will be retried on the next hotplug event.
> + *
> + * The fbdev is destroyed by drm_dev_unregister().
> + */
> +void drm_fbdev_shmem_setup(struct drm_device *dev, unsigned int
> preferred_bpp)
As this is a completely new function, can we please get a
preferred_format parameter instead?
> +{
> + struct drm_fb_helper *fb_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");
> +
> + fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
> + if (!fb_helper)
> + return;
> + drm_fb_helper_prepare(dev, fb_helper, preferred_bpp,
> _fbdev_shmem_helper_funcs);
> +
> + ret = drm_client_init(dev, _helper->client, "fbdev",
> _fbdev_shmem_client_funcs);
> + if (ret) {
> + drm_err(dev, "Failed to register client: %d\n", ret);
> + goto err_drm_client_init;
> + }
> +
> + drm_client_register(_helper->client);
> +
> + return;
> +
> +err_drm_client_init:
> + drm_fb_helper_unprepare(fb_helper);
> + kfree(fb_helper);
> +}
> +EXPORT_SYMBOL(drm_fbdev_shmem_setup);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 22/22] drm: ensure drm headers are self-contained and pass kernel-doc
Hi Jani,
On Thu, Mar 7, 2024 at 9:44 AM Jani Nikula wrote:
> On Thu, 07 Mar 2024, kernel test robot wrote:
> > kernel test robot noticed the following build errors:
>
> So I'm trying to make include/drm/ttm/ttm_caching.h self-contained by
> including with [1], but it fails like below.
>
> Cc: Thomas and Geert, better ideas for the include there? Looks like
> include/asm-generic/pgtable-nop4d.h isn't self-contained on m68k.
I have sent a fix
https://lore.kernel.org/r/ba359be013f379ff10f3afcea13e2f78dd9717be.1709804021.git.ge...@linux-m68k.org
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH] m68k: pgtable: Add missing #include
When just including :
include/asm-generic/pgtable-nop4d.h:9:18: error: unknown type name ‘pgd_t’
9 | typedef struct { pgd_t pgd; } p4d_t;
| ^
Make self-contained by including .
Reported-by: Jani Nikula
Closes: https://lore.kernel.org/r/878r2uxwha@intel.com
Signed-off-by: Geert Uytterhoeven
---
Jani: Feel free to pick this up as a dependency.
Else I will queue this in the m68k tree for v6.10.
arch/m68k/include/asm/pgtable.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/m68k/include/asm/pgtable.h b/arch/m68k/include/asm/pgtable.h
index 27525c6a12fd0c7f..49fcfd7348600594 100644
--- a/arch/m68k/include/asm/pgtable.h
+++ b/arch/m68k/include/asm/pgtable.h
@@ -2,6 +2,8 @@
#ifndef __M68K_PGTABLE_H
#define __M68K_PGTABLE_H
+#include
+
#ifdef __uClinux__
#include
#else
--
2.34.1
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
Hi Maxime,
On Mon, Mar 4, 2024 at 11:20 AM Maxime Ripard wrote:
> On Mon, Mar 04, 2024 at 11:07:22AM +0100, Geert Uytterhoeven wrote:
> > On Mon, Mar 4, 2024 at 10:15 AM Maxime Ripard wrote:
> > > On Mon, Mar 04, 2024 at 09:12:38AM +0100, Geert Uytterhoeven wrote:
> > > > On Sun, Mar 3, 2024 at 10:30 AM Geert Uytterhoeven
> > > > wrote:
> > > > ERROR: modpost: "__udivdi3" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko]
> > > > undefined!
> > > > make[3]: *** [scripts/Makefile.modpost:145: Module.symvers] Error 1
> > > > make[2]: *** [Makefile:1871: modpost] Error 2
> > > > make[1]: *** [Makefile:240: __sub-make] Error 2
> > > > make: *** [Makefile:240: __sub-make] Error 2
> > > >
> > > > No warnings found in log.
> > > > --->8---
> > >
> > > The driver is meant for a controller featured in an SoC with a Cortex-A8
> > > ARM CPU and less than a GiB/s memory bandwidth.
> >
> > Good, so the hardware cannot possibly need 64-bit pixel clock values ;-)
>
> This is an early patch to convert that function into a framework hook
> implementation. HDMI 2.1 has a max TMDS character rate of slightly less
> than 6GHz, so larger than 2^32 - 1.
>
> So yes, this driver doesn't need to. The framework does however.
That's gonna be interesting, as the Common Clock Framework does not
support 64-bit clock rates on 32-bit platforms yet...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH v2] drm: renesas: shmobile: Call drm_helper_force_disable_all() at shutdown time
From: Douglas Anderson
Based on grepping through the source code, this driver appears to be
missing a call to drm_atomic_helper_shutdown() at system shutdown time.
This is important because drm_helper_force_disable_all() will cause
panels to get disabled cleanly which may be important for their power
sequencing. Future changes will remove any custom powering off in
individual panel drivers so the DRM drivers need to start getting this
right.
The fact that we should call drm_atomic_helper_shutdown() in the case of
OS shutdown comes straight out of the kernel doc "driver instance
overview" in drm_drv.c.
Suggested-by: Maxime Ripard
Signed-off-by: Douglas Anderson
Link:
https://lore.kernel.org/r/20230901164111.RFT.15.Iaf638a1d4c8b3c307a6192efabb4cbb06b195f15@changeid
[geert: s/drm_helper_force_disable_all/drm_atomic_helper_shutdown/]
[geert: shmob_drm_remove() already calls drm_atomic_helper_shutdown]
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Laurent Pinchart
---
v2:
- Add Reviewed-by.
Tested on Atmark Techno Armadillo-800-EVA.
---
drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
index e83c3e52251dedf9..0250d5f00bf102dc 100644
--- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c
@@ -171,6 +171,13 @@ static void shmob_drm_remove(struct platform_device *pdev)
drm_kms_helper_poll_fini(ddev);
}
+static void shmob_drm_shutdown(struct platform_device *pdev)
+{
+ struct shmob_drm_device *sdev = platform_get_drvdata(pdev);
+
+ drm_atomic_helper_shutdown(>ddev);
+}
+
static int shmob_drm_probe(struct platform_device *pdev)
{
struct shmob_drm_platform_data *pdata = pdev->dev.platform_data;
@@ -273,6 +280,7 @@ static const struct of_device_id shmob_drm_of_table[]
__maybe_unused = {
static struct platform_driver shmob_drm_platform_driver = {
.probe = shmob_drm_probe,
.remove_new = shmob_drm_remove,
+ .shutdown = shmob_drm_shutdown,
.driver = {
.name = "shmob-drm",
.of_match_table = of_match_ptr(shmob_drm_of_table),
--
2.34.1
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
Hi Maxime,
On Mon, Mar 4, 2024 at 10:15 AM Maxime Ripard wrote:
> On Mon, Mar 04, 2024 at 09:12:38AM +0100, Geert Uytterhoeven wrote:
> > On Sun, Mar 3, 2024 at 10:30 AM Geert Uytterhoeven
> > wrote:
> > > On Sun, Mar 3, 2024 at 3:30 AM Randy Dunlap wrote:
> > > > On 3/2/24 14:10, Guenter Roeck wrote:
> > > > > While checkpatch is indeed of arguable value, I think it would help a
> > > > > lot not having to bother about the persistent _build_ failures on
> > > > > 32-bit systems. You mentioned the fancy drm CI system above, but they
> > > > > don't run tests and not even test builds on 32-bit targets, which has
> > > > > repeatedly caused (and currently does cause) build failures in drm
> > > > > code when trying to build, say, arm:allmodconfig in linux-next. Most
> > > > > trivial build failures in linux-next (and, yes, sometimes mainline)
> > > > > could be prevented with a simple generic CI.
> > > >
> > > > Yes, definitely. Thanks for bringing that up.
> > >
> > > +1
> >
> > > Kisskb can send out email when builds get broken, and when they get
> > > fixed again. I receive such emails for the m68k builds.
> >
> > Like this (yes, one more in DRM; sometimes I wonder if DRM is meant only
> > for 64-bit little-endian platforms with +200 GiB/s memory bandwidth):
> >
> > ---8<---
> > Subject: kisskb: FAILED linux-next/m68k-allmodconfig/m68k-gcc8 Mon Mar 04,
> > 06:35
> > To: ge...@linux-m68k.org
> > Date: Mon, 04 Mar 2024 08:05:14 -
> >
> > FAILED linux-next/m68k-allmodconfig/m68k-gcc8 Mon Mar 04, 06:35
> >
> > http://kisskb.ellerman.id.au/kisskb/buildresult/15135537/
> >
> > Commit: Add linux-next specific files for 20240304
> > 67908bf6954b7635d33760ff6dfc189fc26ccc89
> > Compiler: m68k-linux-gcc (GCC) 8.5.0 / GNU ld (GNU Binutils) 2.36.1
> >
> > Possible errors
> > ---
> >
> > ERROR: modpost: "__udivdi3" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko]
> > undefined!
> > make[3]: *** [scripts/Makefile.modpost:145: Module.symvers] Error 1
> > make[2]: *** [Makefile:1871: modpost] Error 2
> > make[1]: *** [Makefile:240: __sub-make] Error 2
> > make: *** [Makefile:240: __sub-make] Error 2
> >
> > No warnings found in log.
> > ------->8---
>
> The driver is meant for a controller featured in an SoC with a Cortex-A8
> ARM CPU and less than a GiB/s memory bandwidth.
Good, so the hardware cannot possibly need 64-bit pixel clock values ;-)
BTW, doesn't the build fail on arm32, too?
> And I just sent a fix for that one, thanks for the report.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] drm/sun4i: hdmi: Fix u64 div on 32bit arch
Hi Maxime,
Thanks for your patch!
On Mon, Mar 4, 2024 at 10:12 AM Maxime Ripard wrote:
> Commit 358e76fd613a ("drm/sun4i: hdmi: Consolidate atomic_check and
> mode_valid") changed the clock rate from an unsigned long to an unsigned
> long long resulting in a a 64-bit division that might not be supported
> on all platforms.
Why was this changed to unsigned long long?
Can a valid pixel clock really not fit in 32-bit?
> The resulted in compilation being broken at least for m68k, xtensa and
> some arm configurations, at least.
>
> Fixes: 358e76fd613a ("drm/sun4i: hdmi: Consolidate atomic_check and
> mode_valid")
> Reported-by: Geert Uytterhoeven
> Reported-by: Naresh Kamboju
> Closes:
> https://lore.kernel.org/r/CA+G9fYvG9KE15PGNoLu+SBVyShe+u5HBLQ81+kK9Zop6u=y...@mail.gmail.com/
> Reported-by: kernel test robot
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202403011839.klixh4wc-...@intel.com/
> Signed-off-by: Maxime Ripard
> --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> @@ -163,11 +163,11 @@ static enum drm_mode_status
> sun4i_hdmi_connector_clock_valid(const struct drm_connector *connector,
> const struct drm_display_mode *mode,
> unsigned long long clock)
> {
> const struct sun4i_hdmi *hdmi =
> drm_connector_to_sun4i_hdmi(connector);
> - unsigned long diff = clock / 200; /* +-0.5% allowed by HDMI spec */
> + unsigned long diff = div_u64(clock, 200); /* +-0.5% allowed by HDMI
> spec */
I'd rather see clock changed back to unsigned long.
> long rounded_rate;
>
> if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> return MODE_BAD;
>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Sun, Mar 3, 2024 at 10:30 AM Geert Uytterhoeven wrote:
> On Sun, Mar 3, 2024 at 3:30 AM Randy Dunlap wrote:
> > On 3/2/24 14:10, Guenter Roeck wrote:
> > > While checkpatch is indeed of arguable value, I think it would help a
> > > lot not having to bother about the persistent _build_ failures on
> > > 32-bit systems. You mentioned the fancy drm CI system above, but they
> > > don't run tests and not even test builds on 32-bit targets, which has
> > > repeatedly caused (and currently does cause) build failures in drm
> > > code when trying to build, say, arm:allmodconfig in linux-next. Most
> > > trivial build failures in linux-next (and, yes, sometimes mainline)
> > > could be prevented with a simple generic CI.
> >
> > Yes, definitely. Thanks for bringing that up.
>
> +1
> Kisskb can send out email when builds get broken, and when they get
> fixed again. I receive such emails for the m68k builds.
Like this (yes, one more in DRM; sometimes I wonder if DRM is meant only
for 64-bit little-endian platforms with +200 GiB/s memory bandwidth):
---8<---
Subject: kisskb: FAILED linux-next/m68k-allmodconfig/m68k-gcc8 Mon Mar 04, 06:35
To: ge...@linux-m68k.org
Date: Mon, 04 Mar 2024 08:05:14 -
FAILED linux-next/m68k-allmodconfig/m68k-gcc8 Mon Mar 04, 06:35
http://kisskb.ellerman.id.au/kisskb/buildresult/15135537/
Commit: Add linux-next specific files for 20240304
67908bf6954b7635d33760ff6dfc189fc26ccc89
Compiler: m68k-linux-gcc (GCC) 8.5.0 / GNU ld (GNU Binutils) 2.36.1
Possible errors
---
ERROR: modpost: "__udivdi3" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
make[3]: *** [scripts/Makefile.modpost:145: Module.symvers] Error 1
make[2]: *** [Makefile:1871: modpost] Error 2
make[1]: *** [Makefile:240: __sub-make] Error 2
make: *** [Makefile:240: __sub-make] Error 2
No warnings found in log.
------->8---
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Sun, Mar 3, 2024 at 3:30 AM Randy Dunlap wrote:
> On 3/2/24 14:10, Guenter Roeck wrote:
> > On Thu, Feb 29, 2024 at 12:21 PM Linus Torvalds
> > wrote:
> >> On Thu, 29 Feb 2024 at 01:23, Nikolai Kondrashov wrote:
> >>>
> >>> However, I think a better approach would be *not* to add the
> >>> .gitlab-ci.yaml
> >>> file in the root of the source tree, but instead change the very same repo
> >>> setting to point to a particular entry YAML, *inside* the repo (somewhere
> >>> under "ci" directory) instead.
> >>
> >> I really don't want some kind of top-level CI for the base kernel project.
> >>
> >> We already have the situation that the drm people have their own ci
> >> model. II'm ok with that, partly because then at least the maintainers
> >> of that subsystem can agree on the rules for that one subsystem.
> >>
> >> I'm not at all interested in having something that people will then
> >> either fight about, or - more likely - ignore, at the top level
> >> because there isn't some global agreement about what the rules are.
> >>
> >> For example, even just running checkpatch is often a stylistic thing,
> >> and not everybody agrees about all the checkpatch warnings.
> >
> > While checkpatch is indeed of arguable value, I think it would help a
> > lot not having to bother about the persistent _build_ failures on
> > 32-bit systems. You mentioned the fancy drm CI system above, but they
> > don't run tests and not even test builds on 32-bit targets, which has
> > repeatedly caused (and currently does cause) build failures in drm
> > code when trying to build, say, arm:allmodconfig in linux-next. Most
> > trivial build failures in linux-next (and, yes, sometimes mainline)
> > could be prevented with a simple generic CI.
>
> Yes, definitely. Thanks for bringing that up.
+1
> > Sure, argue against checkpatch as much as you like, but the code
> > should at least _build_, and it should not be necessary for random
> > people to report build failures to the submitters.
>
> I do 110 randconfig builds nightly (10 each of 11 $ARCH/$BITS).
> That's about all the horsepower that I have. and I am not a CI. :)
>
> So I see quite a bit of what you are saying. It seems that Arnd is
> in the same boat.
You don't even have to do your own builds (although it does help),
and can look at e.g. http://kisskb.ellerman.id.au/kisskb/
Kisskb can send out email when builds get broken, and when they get
fixed again. I receive such emails for the m68k builds.
I have the feeling this is not used up to its full potential yet.
My initial plan with the "Build regressions/improvements in ..." emails
[1] was to fully automate this, and enable it for the other daily builds
(e.g. linux-next), too, but there are only so many hours in a day...
[1] https://lore.kernel.org/all/20240226081253.3688538-1-ge...@linux-m68k.org/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [DO NOT MERGE v6 34/37] sh: Add dtbs target support.
On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato
wrote:
> Signed-off-by: Yoshinori Sato
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [DO NOT MERGE v6 13/37] dt-bindings: clock: sh7750-cpg: Add renesas,sh7750-cpg header.
Hi Sato-san,
Thanks for your patch!
On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato
wrote:
> SH7750 CPG Clock output define.
Please improve the patch description.
> Signed-off-by: Yoshinori Sato
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,sh7750-cpg.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/renesas,sh7750-cpg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas SH7750/7751 Clock Pulse Generator (CPG)
> +
> +maintainers:
> + - Yoshinori Sato
> +
> +description:
> + The Clock Pulse Generator (CPG) generates core clocks for the SoC. It
> + includes PLLs, and variable ratio dividers.
> +
> + The CPG may also provide a Clock Domain for SoC devices, in combination
> with
> + the CPG Module Stop (MSTP) Clocks.
> +
> +properties:
> + compatible:
> +enum:
> + - renesas,sh7750-cpg # SH7750
> + - renesas,sh7750s-cpg# SH775S
> + - renesas,sh7750r-cpg# SH7750R
> + - renesas,sh7751-cpg # SH7751
> + - renesas,sh7751r-cpg# SH7751R
> +
> + reg: true
> +
> + reg-names: true
> +
> + clocks: true
clocks:
maxItems: 1
> +
> + clock-names: true
clock-names:
const: extal
> +examples:
> + - |
> +#include
> +cpg: clock-controller@ffc0 {
> +#clock-cells = <1>;
> +#power-domain-cells = <0>;
> +compatible = "renesas,sh7751r-cpg";
> +clocks = <>;
> +clock-names = "xtal";
"extal"
"xtal" is an output pin, connected to a crystal resonator.
"extal" is the clock input put (either crystal resonator or exteral
clock input.
> +reg = <0xffc0 20>, <0xfe0a 16>;
> +reg-names = "FRQCR", "CLKSTP00";
> +renesas,mode = <0>;
> +};
> diff --git a/include/dt-bindings/clock/sh7750-cpg.h
> b/include/dt-bindings/clock/sh7750-cpg.h
> new file mode 100644
> index ..17d5a8076aac
> --- /dev/null
> +++ b/include/dt-bindings/clock/sh7750-cpg.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> + *
> + * Copyright 2023 Yoshinori Sato
> + */
> +
> +#ifndef __DT_BINDINGS_CLOCK_SH7750_H__
> +#define __DT_BINDINGS_CLOCK_SH7750_H__
> +
> +#define SH7750_CPG_PLLOUT 0
> +
> +#define SH7750_CPG_FCK 1
PCK?
> +#define SH7750_CPG_BCK 2
> +#define SH7750_CPG_ICK 3
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [DO NOT MERGE v6 15/37] clk: renesas: Add SH7750/7751 CPG Driver
CLK_GATE_REG_8BIT |
> CLK_GATE_SET_TO_DISABLE,
> + >clklock);
> + if (IS_ERR(reg_hw)) {
> + ret = PTR_ERR(reg_hw);
> + goto error;
> + }
> + data->hws[num_clk++] = reg_hw;
> + }
> + if (cpg->feat & MSTP_CLKSTP) {
> + for (i = 0; i < ARRAY_SIZE(clkstpout); i++) {
> + if (i == 2 && !(cpg->feat & MSTP_CSTP2))
> + continue;
Set maximum loop counter upfront?
> + reg_hw = clk_hw_register_clkstp(node, clkstpout[i],
> + divout[0],
> cpg->clkstp00,
> + i, >clklock);
> + if (IS_ERR(reg_hw)) {
> + ret = PTR_ERR(reg_hw);
> + goto error;
> + }
> + data->hws[num_clk++] = reg_hw;
> + }
> + }
> + data->num = num_clk;
> + ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, data);
> + if (ret < 0)
> + goto error;
> + return 0;
> +
> +error:
> + pr_err("%pOF: failed to register clock (%d)\n",
> + node, ret);
> + for (num_clk--; num_clk >= 0; num_clk--)
> + kfree(data->hws[num_clk]);
> + kfree(data);
> + return ret;
> +}
> +
> +static struct cpg_priv *sh7750_cpg_setup(struct device_node *node, u32 feat)
> +{
> + unsigned int num_parents;
> + u32 mode;
> + struct cpg_priv *cpg;
> + int ret = 0;
> +
> + num_parents = of_clk_get_parent_count(node);
> + if (num_parents < 1) {
> + pr_err("%s: no parent found", node->name);
> + return ERR_PTR(-ENODEV);
> + }
Do you need num_parents?
> +
> + of_property_read_u32_index(node, "renesas,mode", 0, );
mode may be used uninitialized, if "renesas,mode" is missing.
> + if (mode >= 7) {
> + pr_err("%s: Invalid clock mode setting (%u)\n",
> + node->name, mode);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + cpg = kzalloc(sizeof(struct cpg_priv), GFP_KERNEL);
> + if (!cpg)
> + return ERR_PTR(-ENOMEM);
> +
> + cpg->frqcr = of_iomap(node, 0);
> + if (cpg->frqcr == NULL) {
> + pr_err("%pOF: failed to map divide register", node);
> + ret = -ENODEV;
> + goto cpg_free;
> + }
> +
> + if (feat & MSTP_CLKSTP) {
> + cpg->clkstp00 = of_iomap(node, 1);
> + if (cpg->clkstp00 == NULL) {
> + pr_err("%pOF: failed to map clkstp00 register", node);
> + ret = -ENODEV;
> + goto unmap_frqcr;
> + }
> + }
> + cpg->feat = feat;
> + cpg->mode = mode;
> +
> + ret = register_pll(node, cpg);
> + if (ret < 0)
> + goto unmap_clkstp00;
> +
> + ret = register_div(node, cpg);
> + if (ret < 0)
> + goto unmap_clkstp00;
> +
Perhaps "cpg_data = cpg;" here, and return an error code instead? ...
> + return cpg;
> +
> +unmap_clkstp00:
> + iounmap(cpg->clkstp00);
> +unmap_frqcr:
> + iounmap(cpg->frqcr);
> +cpg_free:
> + kfree(cpg);
> + return ERR_PTR(ret);
> +}
> +
> +static void __init sh7750_cpg_init(struct device_node *node)
> +{
> + cpg_data = sh7750_cpg_setup(node, cpg_feature[CPG_SH7750]);
> + if (IS_ERR(cpg_data))
> + cpg_data = NULL;
... then all cpg_data handling can be removed here...
> +}
> +static int sh7750_cpg_probe(struct platform_device *pdev)
> +{
> + u32 feature;
> +
> + if (cpg_data)
> + return 0;
> + feature = *(u32 *)of_device_get_match_data(>dev);
> + cpg_data = sh7750_cpg_setup(pdev->dev.of_node, feature);
> + if (IS_ERR(cpg_data))
> + return PTR_ERR(cpg_data);
> + return 0;
... and this can be simplified to
return sh7750_cpg_setup(...);
> +}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
