RIP: 0010:g84_gr_tlb_flush+0x2eb/0x300 [nouveau]
Hi there, Could someone please comment on the following hard-lock (*). Staring at the code, I see `nvkm_rd32` calls are enclosed in a timeout detection, except one. drivers/gpu/drm/nouveau/nvkm/engine/gr/g84.c#L171 ... nvkm_msec(device, 2000, if (!(nvkm_rd32(device, 0x100c80) & 0x0001)) break; ); ... Would it make sense to also enclose this in the do/while loop ? Full ref: * https://gitlab.freedesktop.org/xorg/driver/xf86-video-nouveau/-/issues/536 Thanks (*) Oct 20 08:44:13 vostrodell kernel: [55669.783436] [ cut here ] Oct 20 08:44:13 vostrodell kernel: [55669.783446] nouveau :01:00.0: timeout Oct 20 08:44:13 vostrodell kernel: [55669.783527] WARNING: CPU: 1 PID: 7812 at drivers/gpu/drm/nouveau/nvkm/engine/gr/g84.c:171 g84_gr_tlb_flush+0x2eb/0x300 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.783527] Modules linked in: cdc_acm(OE) fuse md4 sha512_ssse3 sha512_generic cmac nls_utf8 cifs dns_resolver fscache libdes intel_rapl_msr intel_rapl_common x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm ath9k irqbypass ath9k_common ath9k_hw crc32_pclmul ath snd_hda_codec_conexant ghash_clmulni_intel snd_hda_codec_generic snd_hda_codec_hdmi ledtrig_audio mac80211 snd_hda_intel snd_intel_dspcfg aesni_intel libaes crypto_simd snd_hda_codec snd_hda_core cryptd snd_hwdep cfg80211 glue_helper dcdbas snd_pcm rapl iTCO_wdt snd_timer intel_pmc_bxt mei_me intel_cstate rfkill iTCO_vendor_support snd intel_uncore dell_smm_hwmon mei serio_raw soundcore watchdog libarc4 at24 sg pcspkr evdev binfmt_misc ip_tables x_tables autofs4 ext4 crc16 mbcache jbd2 crc32c_generic sd_mod sr_mod t10_pi cdrom crc_t10dif crct10dif_generic hid_generic usbhid hid uas ata_generic usb_storage nouveau mxm_wmi wmi ata_piix crct10dif_pclmul crct10dif_common video i2c_algo_bit libata ttm crc32c_intel Oct 20 08:44:13 vostrodell kernel: [55669.783551] drm_kms_helper psmouse scsi_mod i2c_i801 i2c_smbus cec lpc_ich ehci_pci ehci_hcd drm usbcore r8169 realtek libphy usb_common button [last unloaded: cdc_acm] Oct 20 08:44:13 vostrodell kernel: [55669.783558] CPU: 1 PID: 7812 Comm: kworker/1:0 Tainted: G OE 5.8.0-0.bpo.2-amd64 #1 Debian 5.8.10-1~bpo10+1 Oct 20 08:44:13 vostrodell kernel: [55669.783559] Hardware name: Dell Inc. Vostro 260 /0GDG8Y , BIOS A10 02/22/2013 Oct 20 08:44:13 vostrodell kernel: [55669.783595] Workqueue: events nouveau_cli_work [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.783631] RIP: 0010:g84_gr_tlb_flush+0x2eb/0x300 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.783633] Code: 8b 40 10 48 8b 78 10 4c 8b 67 50 4d 85 e4 75 03 4c 8b 27 e8 87 e7 ab e7 4c 89 e2 48 c7 c7 6c 43 63 c0 48 89 c6 e8 30 af 58 e7 <0f> 0b e9 44 ff ff ff e8 d9 d7 cf e7 66 0f 1f 84 00 00 00 00 00 66 Oct 20 08:44:13 vostrodell kernel: [55669.783634] RSP: 0018:ae76c66db998 EFLAGS: 00010082 Oct 20 08:44:13 vostrodell kernel: [55669.783634] RAX: RBX: 9dc2450f8c00 RCX: 0027 Oct 20 08:44:13 vostrodell kernel: [55669.783635] RDX: 0027 RSI: 9dc277498ac0 RDI: 9dc277498ac8 Oct 20 08:44:13 vostrodell kernel: [55669.783636] RBP: R08: 04df R09: 0004 Oct 20 08:44:13 vostrodell kernel: [55669.783636] R10: R11: 0001 R12: 9dc26fc42290 Oct 20 08:44:13 vostrodell kernel: [55669.783637] R13: 32a15d0363a0 R14: 9dc277560580 R15: 0001 Oct 20 08:44:13 vostrodell kernel: [55669.783638] FS: () GS:9dc27748() knlGS: Oct 20 08:44:13 vostrodell kernel: [55669.783639] CS: 0010 DS: ES: CR0: 80050033 Oct 20 08:44:13 vostrodell kernel: [55669.783639] CR2: 7f72949e5000 CR3: 00020480a001 CR4: 000606e0 Oct 20 08:44:13 vostrodell kernel: [55669.783640] Call Trace: Oct 20 08:44:13 vostrodell kernel: [55669.783648] ? enqueue_task_fair+0x8e/0x690 Oct 20 08:44:13 vostrodell kernel: [55669.783683] ? nv04_timer_read+0x42/0x60 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.831715] nv50_vmm_flush+0x1f1/0x220 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.831719] ? refcount_dec_and_mutex_lock+0x26/0x60 Oct 20 08:44:13 vostrodell kernel: [55669.831751] nvkm_vmm_iter.constprop.10+0x364/0x820 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.831783] ? nvkm_vmm_map_choose+0xa0/0xa0 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.831815] ? nv44_vmm_new+0xe0/0xe0 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.831847] nvkm_vmm_ptes_unmap_put+0x2c/0x40 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.831885] ? nvkm_vmm_map_choose+0xa0/0xa0 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.831922] ? nv44_vmm_new+0xe0/0xe0 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.831968] nvkm_vmm_put_locked+0x203/0x240 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.832000] nvkm_uvmm_mthd+0x249/0x710 [nouveau] Oct 20 08:44:13 vostrodell kernel: [55669.832022] nvkm_ioctl+
Re: [PATCH] video: fbdev: radeon: Fix memleak in radeonfb_pci_register
On Tue, Aug 25, 2020 at 9:27 AM Dinghao Liu wrote:
>
> When radeon_kick_out_firmware_fb() fails, info should be
> freed just like the subsequent error paths.
>
> Fixes: 069ee21a82344 ("fbdev: Fix loading of module radeonfb on PowerMac")
> Signed-off-by: Dinghao Liu
> ---
> drivers/video/fbdev/aty/radeon_base.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/aty/radeon_base.c
> b/drivers/video/fbdev/aty/radeon_base.c
> index 3fe509cb9b87..13bd2bd5c043 100644
> --- a/drivers/video/fbdev/aty/radeon_base.c
> +++ b/drivers/video/fbdev/aty/radeon_base.c
> @@ -2307,7 +2307,7 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
>
> ret = radeon_kick_out_firmware_fb(pdev);
> if (ret)
> - return ret;
> + goto err_release_fb;
Good catch ! Thanks
Reviewed-by: Mathieu Malaterre
> /* request the mem regions */
> ret = pci_request_region(pdev, 0, "radeonfb framebuffer");
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/radeon: disable AGP by default
On Wed, May 13, 2020 at 1:21 PM Christian König wrote: > > Always use the PCI GART instead. Reviewed-by: Mathieu Malaterre > Signed-off-by: Christian König > --- > drivers/gpu/drm/radeon/radeon_drv.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c > b/drivers/gpu/drm/radeon/radeon_drv.c > index bbb0883e8ce6..a71f13116d6b 100644 > --- a/drivers/gpu/drm/radeon/radeon_drv.c > +++ b/drivers/gpu/drm/radeon/radeon_drv.c > @@ -171,12 +171,7 @@ int radeon_no_wb; > int radeon_modeset = -1; > int radeon_dynclks = -1; > int radeon_r4xx_atom = 0; > -#ifdef __powerpc__ > -/* Default to PCI on PowerPC (fdo #95017) */ > int radeon_agpmode = -1; > -#else > -int radeon_agpmode = 0; > -#endif > int radeon_vram_limit = 0; > int radeon_gart_size = -1; /* auto */ > int radeon_benchmarking = 0; > -- > 2.17.1 > -- Mathieu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: radeon: add a missing break in evergreen_cs_handle_reg
On Thu, Jan 17, 2019 at 9:40 PM Mathieu Malaterre wrote:
>
> In commit dd220a00e8bd ("drm/radeon/kms: add support for streamout v7")
> case statements were added without a terminating break statement. This
> commit adds the missing break. This was discovered during a compilation
> with W=1.
>
> This commit removes the following warning:
>
> drivers/gpu/drm/radeon/evergreen_cs.c:1301:11: warning: this statement may
> fall through [-Wimplicit-fallthrough=]
>
> Suggested-by: Alex Deucher
> Fixes: dd220a00e8bd ("drm/radeon/kms: add support for streamout v7")
> Signed-off-by: Mathieu Malaterre
> ---
> v2: Add missing break statement, instead of marking it as fall through
Replaced by:
cc5034a5d293 drm/radeon/evergreen_cs: fix missing break in switch statement
> drivers/gpu/drm/radeon/evergreen_cs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c
> b/drivers/gpu/drm/radeon/evergreen_cs.c
> index f471537c852f..1e14c6921454 100644
> --- a/drivers/gpu/drm/radeon/evergreen_cs.c
> +++ b/drivers/gpu/drm/radeon/evergreen_cs.c
> @@ -1299,6 +1299,7 @@ static int evergreen_cs_handle_reg(struct
> radeon_cs_parser *p, u32 reg, u32 idx)
> return -EINVAL;
> }
> ib[idx] += (u32)((reloc->gpu_offset >> 8) & 0x);
> + break;
> case CB_TARGET_MASK:
> track->cb_target_mask = radeon_get_ib_value(p, idx);
> track->cb_dirty = true;
> --
> 2.19.2
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] video: fbdev: Fix Warning comparing pointer to 0 reported by coccicheck
On Mon, Jun 3, 2019 at 1:21 PM wrote: > > From: Shobhit Kukreti > > Fixed Warning Comparing Pointer to 0. Changed return value to -ENOMEM to > report kzalloc failure > > drivers/video/fbdev/controlfb.c: WARNING comparing pointer to 0 > drivers/video/fbdev/controlfb.c: WARNING comparing pointer to 0 > drivers/video/fbdev/controlfb.c: WARNING comparing pointer to 0 > > Signed-off-by: Shobhit Kukreti > --- > Changes in v2: > - Modified commit message to report change in return type > > drivers/video/fbdev/controlfb.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c > index 7af8db2..07907c5 100644 > --- a/drivers/video/fbdev/controlfb.c > +++ b/drivers/video/fbdev/controlfb.c > @@ -182,7 +182,7 @@ int init_module(void) > int ret = -ENXIO; > > dp = of_find_node_by_name(NULL, "control"); > - if (dp != 0 && !control_of_init(dp)) > + if (dp != NULL && !control_of_init(dp)) > ret = 0; > of_node_put(dp); > > @@ -580,7 +580,7 @@ static int __init control_init(void) > control_setup(option); > > dp = of_find_node_by_name(NULL, "control"); > - if (dp != 0 && !control_of_init(dp)) > + if (dp != NULL && !control_of_init(dp)) > ret = 0; > of_node_put(dp); > > @@ -683,8 +683,8 @@ static int __init control_of_init(struct device_node *dp) > return -ENXIO; > } > p = kzalloc(sizeof(*p), GFP_KERNEL); > - if (p == 0) > - return -ENXIO; > + if (p == NULL) nit: I would have use `!p` (same for the others above). Maybe checkpatch with --strict would warn for those (can't remember from top of my head). Anyway: Reviewed-by: Mathieu Malaterre > + return -ENOMEM; > control_fb = p; /* save it for cleanups */ > > /* Map in frame buffer and registers */ > -- > 2.7.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: fbdev: Fix Warning comparing pointer to 0 reported by coccicheck
On Sat, Jun 1, 2019 at 5:39 PM Shobhit Kukreti wrote: > > drivers/video/fbdev/controlfb.c: WARNING comparing pointer to 0 > drivers/video/fbdev/controlfb.c: WARNING comparing pointer to 0 > drivers/video/fbdev/controlfb.c: WARNING comparing pointer to 0 > > Signed-off-by: Shobhit Kukreti > --- > drivers/video/fbdev/controlfb.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c > index 7af8db2..07907c5 100644 > --- a/drivers/video/fbdev/controlfb.c > +++ b/drivers/video/fbdev/controlfb.c > @@ -182,7 +182,7 @@ int init_module(void) > int ret = -ENXIO; > > dp = of_find_node_by_name(NULL, "control"); > - if (dp != 0 && !control_of_init(dp)) > + if (dp != NULL && !control_of_init(dp)) > ret = 0; > of_node_put(dp); > > @@ -580,7 +580,7 @@ static int __init control_init(void) > control_setup(option); > > dp = of_find_node_by_name(NULL, "control"); > - if (dp != 0 && !control_of_init(dp)) > + if (dp != NULL && !control_of_init(dp)) > ret = 0; > of_node_put(dp); > > @@ -683,8 +683,8 @@ static int __init control_of_init(struct device_node *dp) > return -ENXIO; > } > p = kzalloc(sizeof(*p), GFP_KERNEL); > - if (p == 0) > - return -ENXIO; > + if (p == NULL) > + return -ENOMEM; IMHO the above should be described in the commit message. > control_fb = p; /* save it for cleanups */ > > /* Map in frame buffer and registers */ > -- > 2.7.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 09/11] drm/fbdevdrm: Add primary plane
On Wed, Mar 27, 2019 at 10:37 AM Thomas Zimmermann wrote:
>
> Hi
>
> Am 26.03.19 um 14:33 schrieb Mathieu Malaterre:
> >
> > ...
> > ../drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c:174:2: note: in expansion
> > of macro 'do_div'
> > do_div(width, cpp);
> > ^~
> > In file included from ./arch/powerpc/include/generated/asm/div64.h:1,
> > from ../include/linux/kernel.h:18,
> > from ../include/linux/list.h:9,
> > from ../include/linux/rculist.h:10,
> > from ../include/linux/pid.h:5,
> > from ../include/linux/sched.h:14,
> > from ../drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c:15:
> > ../include/asm-generic/div64.h:239:22: error: passing argument 1 of
> > '__div64_32' from incompatible pointer type
> > [-Werror=incompatible-pointer-types]
> >__rem = __div64_32(&(n), __base); \
> > ^~~~
> > ../drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c:174:2: note: in expansion
> > of macro 'do_div'
> > do_div(width, cpp);
> > ^~
> > ../include/asm-generic/div64.h:213:38: note: expected 'uint64_t *'
> > {aka 'long long unsigned int *'} but argument is of type 'unsigned int
> > *'
> > extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
> > ...
>
> I didn't see this error in 64-bit builds either. Could you send me your
> kernel config? Thanks!
https://github.com/malaterre/linux/blob/g4/arch/powerpc/configs/g4_defconfig
I am using Debian/sid with the default cross compiler for powerpc:
$ make -j8 O=g4 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
> Best regards
> Thomas
>
>
> >
> >> +
> >> + if (width > (__u32)-1)
> >> + return -EINVAL; /* would overflow fb_var->xres_virtual */
> >> +
> >> + pitch = fb->pitches[0];
> >> + lines = vram_size;
> >> + do_div(lines, pitch);
> >> +
> >> + if (lines > (__u32)-1)
> >> + return -EINVAL; /* would overflow fb_var->yres_virtual */
> >> +
> >> + fb_var->xres_virtual = width;
> >> + fb_var->yres_virtual = lines;
> >> +
> >> + ret = fbdevdrm_update_fb_var_screeninfo_from_format(
> >> + fb_var, fb->format[0].format);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + return 0;
> >> +}
> >> diff --git a/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.h
> >> b/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.h
> >> index f88a86a83858..925eea78e3f0 100644
> >> --- a/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.h
> >> +++ b/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.h
> >> @@ -13,8 +13,11 @@
> >> #ifndef FBDEVDRM_MODES_H
> >> #define FBDEVDRM_MODES_H
> >>
> >> +#include
> >> +
> >> struct drm_device;
> >> struct drm_display_mode;
> >> +struct drm_framebuffer;
> >> struct fb_videomode;
> >> struct fb_var_screeninfo;
> >>
> >> @@ -43,4 +46,8 @@ void
> >> fbdevdrm_init_fb_var_screeninfo_from_mode(struct fb_var_screeninfo *var,
> >> const struct drm_display_mode
> >> *mode);
> >>
> >> +int fbdevdrm_update_fb_var_screeninfo_from_framebuffer(
> >> + struct fb_var_screeninfo *fb_var, struct drm_framebuffer *fb,
> >> + size_t vram_size);
> >> +
> >> #endif
> >> diff --git a/drivers/gpu/drm/fbdevdrm/fbdevdrm_modeset.c
> >> b/drivers/gpu/drm/fbdevdrm/fbdevdrm_modeset.c
> >> index 585f3478f190..3473b85acbf1 100644
> >> --- a/drivers/gpu/drm/fbdevdrm/fbdevdrm_modeset.c
> >> +++ b/drivers/gpu/drm/fbdevdrm/fbdevdrm_modeset.c
> >> @@ -20,6 +20,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include "fbdevdrm_primary.h"
> >>
> >> /*
> >> * CRTC
> >> @@ -376,7 +377,13 @@ int fbdevdrm_modeset_init(struct fbdevdrm_modeset
> >> *modeset,
> >> * connect them with each other.
> >> */
> >>
> >> - ret = drm_crtc_init_with_planes(dev, &modeset->crtc, NULL, NULL,
> >> + ret = fbdevdrm_init_primary_plane_from_fb_info(
> >> + &modeset->primary_plane, dev, 0, fb_info);
> >> + if (ret)
> >> + goto er
Re: [PATCH 09/11] drm/fbdevdrm: Add primary plane
On Tue, Mar 26, 2019 at 10:18 AM Thomas Zimmermann wrote:
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/fbdevdrm/Makefile | 1 +
> drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c | 42 ++
> drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.h | 7 +
> drivers/gpu/drm/fbdevdrm/fbdevdrm_modeset.c | 9 +-
> drivers/gpu/drm/fbdevdrm/fbdevdrm_modeset.h | 2 +
> drivers/gpu/drm/fbdevdrm/fbdevdrm_primary.c | 498
> drivers/gpu/drm/fbdevdrm/fbdevdrm_primary.h | 27 ++
> 7 files changed, 585 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_primary.c
> create mode 100644 drivers/gpu/drm/fbdevdrm/fbdevdrm_primary.h
>
> diff --git a/drivers/gpu/drm/fbdevdrm/Makefile
> b/drivers/gpu/drm/fbdevdrm/Makefile
> index 2ca906a3258b..5507152d8187 100644
> --- a/drivers/gpu/drm/fbdevdrm/Makefile
> +++ b/drivers/gpu/drm/fbdevdrm/Makefile
> @@ -5,6 +5,7 @@ fbdevdrm-y := fbdevdrm_bo.o \
> fbdevdrm_format.o \
> fbdevdrm_modes.o \
> fbdevdrm_modeset.o \
> + fbdevdrm_primary.o \
> fbdevdrm_ttm.o
>
> obj-$(CONFIG_DRM_FBDEVDRM) += fbdevdrm.o
> diff --git a/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c
> b/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c
> index bd3ad691e7ce..8dea7ef369dc 100644
> --- a/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c
> +++ b/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c
> @@ -11,8 +11,12 @@
> */
>
> #include "fbdevdrm_modes.h"
> +#include
> +#include /* for TASK_COMM_LEN in */
> +#include
> #include
> #include
> +#include "fbdevdrm_format.h"
>
> void drm_mode_update_from_fb_videomode(struct drm_display_mode *mode,
>const struct fb_videomode *fb_mode)
> @@ -151,3 +155,41 @@ fbdevdrm_init_fb_var_screeninfo_from_mode(struct
> fb_var_screeninfo *fb_var,
> memset(fb_var, 0, sizeof(*fb_var));
> fbdevdrm_update_fb_var_screeninfo_from_mode(fb_var, mode);
> }
> +
> +int fbdevdrm_update_fb_var_screeninfo_from_framebuffer(
> + struct fb_var_screeninfo *fb_var, struct drm_framebuffer *fb,
> + size_t vram_size)
> +{
> + unsigned int width, pitch;
> + uint64_t cpp, lines;
> + int ret;
> +
> + /* Our virtual screen covers all the graphics memory (sans some
> +* trailing bytes). This allows for setting the scanout buffer's
> +* address with fb_pan_display().
> +*/
> +
> + width = fb->pitches[0];
> + cpp = drm_format_plane_cpp(fb->format[0].format, 0);
> + do_div(width, cpp);
A simple compile/test here leads to:
...
../drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c:174:2: note: in expansion
of macro 'do_div'
do_div(width, cpp);
^~
In file included from ./arch/powerpc/include/generated/asm/div64.h:1,
from ../include/linux/kernel.h:18,
from ../include/linux/list.h:9,
from ../include/linux/rculist.h:10,
from ../include/linux/pid.h:5,
from ../include/linux/sched.h:14,
from ../drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c:15:
../include/asm-generic/div64.h:239:22: error: passing argument 1 of
'__div64_32' from incompatible pointer type
[-Werror=incompatible-pointer-types]
__rem = __div64_32(&(n), __base); \
^~~~
../drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.c:174:2: note: in expansion
of macro 'do_div'
do_div(width, cpp);
^~
../include/asm-generic/div64.h:213:38: note: expected 'uint64_t *'
{aka 'long long unsigned int *'} but argument is of type 'unsigned int
*'
extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
...
> +
> + if (width > (__u32)-1)
> + return -EINVAL; /* would overflow fb_var->xres_virtual */
> +
> + pitch = fb->pitches[0];
> + lines = vram_size;
> + do_div(lines, pitch);
> +
> + if (lines > (__u32)-1)
> + return -EINVAL; /* would overflow fb_var->yres_virtual */
> +
> + fb_var->xres_virtual = width;
> + fb_var->yres_virtual = lines;
> +
> + ret = fbdevdrm_update_fb_var_screeninfo_from_format(
> + fb_var, fb->format[0].format);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.h
> b/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.h
> index f88a86a83858..925eea78e3f0 100644
> --- a/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.h
> +++ b/drivers/gpu/drm/fbdevdrm/fbdevdrm_modes.h
> @@ -13,8 +13,11 @@
> #ifndef FBDEVDRM_MODES_H
> #define FBDEVDRM_MODES_H
>
> +#include
> +
> struct drm_device;
> struct drm_display_mode;
> +struct drm_framebuffer;
> struct fb_videomode;
> struct fb_var_screeninfo;
>
> @@ -43,4 +46,8 @@ void
> fbdevdrm_init_fb_var_screeninfo_from_mode(struct fb_var_screeninfo *var,
> const struct drm_display_mode
> *mode);
>
> +int fbdevdrm_update_fb_var_screeninfo_from
Re: [PATCH] video/hdmi: Change strncpy() into memcpy() in hdmi_spd_infoframe_init
On Fri, Jan 18, 2019 at 8:51 PM Joe Perches wrote:
>
> On Fri, 2019-01-18 at 20:32 +0100, Mathieu Malaterre wrote:
> > Using strncpy() is less than perfect since the destination buffers do not
> > need to be NUL terminated. Replace strncpy() with memcpy() to address a
> > warning triggered by gcc using W=1 and optimize the code a bit.
> >
> > This commit removes the following warnings:
> >
> > drivers/video/hdmi.c:234:2: warning: 'strncpy' specified bound 8 equals
> > destination size [-Wstringop-truncation]
> > drivers/video/hdmi.c:235:2: warning: 'strncpy' specified bound 16 equals
> > destination size [-Wstringop-truncation]
> []
> > diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> []
> > @@ -231,8 +231,8 @@ int hdmi_spd_infoframe_init(struct hdmi_spd_infoframe
> > *frame,
> > frame->version = 1;
> > frame->length = HDMI_SPD_INFOFRAME_SIZE;
> >
> > - strncpy(frame->vendor, vendor, sizeof(frame->vendor));
> > - strncpy(frame->product, product, sizeof(frame->product));
> > + memcpy(frame->vendor, vendor, sizeof(frame->vendor));
> > + memcpy(frame->product, product, sizeof(frame->product));
>
> This is not good.
>
> vendor can be any location and shorter than sizeof(frame->vendor)
> so this can copy from invalid memory locations.
Ah right. I did not realize that and know I see the call with "Intel",
will re-spin.
> You probably want strscpy.
Right.
> This is called with at least "mediatek" and "broadcom", so perhaps
> it's better still to change the struct vendor size to something a
> bit larger. Maybe change vendor[8] to vendor[16];
Looks like 8 bytes is required for call like hdmi_spd_infoframe_unpack()
> include/linux/hdmi.h:struct hdmi_spd_infoframe {
> include/linux/hdmi.h- enum hdmi_infoframe_type type;
> include/linux/hdmi.h- unsigned char version;
> include/linux/hdmi.h- unsigned char length;
> include/linux/hdmi.h- char vendor[8];
> include/linux/hdmi.h- char product[16];
> include/linux/hdmi.h- enum hdmi_spd_sdi sdi;
> include/linux/hdmi.h-};
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] video/hdmi: Change strncpy() into memcpy() in hdmi_spd_infoframe_init
Using strncpy() is less than perfect since the destination buffers do not need to be NUL terminated. Replace strncpy() with memcpy() to address a warning triggered by gcc using W=1 and optimize the code a bit. This commit removes the following warnings: drivers/video/hdmi.c:234:2: warning: 'strncpy' specified bound 8 equals destination size [-Wstringop-truncation] drivers/video/hdmi.c:235:2: warning: 'strncpy' specified bound 16 equals destination size [-Wstringop-truncation] Signed-off-by: Mathieu Malaterre --- drivers/video/hdmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index 799ae49774f5..553c39ac8f9e 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -231,8 +231,8 @@ int hdmi_spd_infoframe_init(struct hdmi_spd_infoframe *frame, frame->version = 1; frame->length = HDMI_SPD_INFOFRAME_SIZE; - strncpy(frame->vendor, vendor, sizeof(frame->vendor)); - strncpy(frame->product, product, sizeof(frame->product)); + memcpy(frame->vendor, vendor, sizeof(frame->vendor)); + memcpy(frame->product, product, sizeof(frame->product)); return 0; } -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm: radeon: add a missing break in evergreen_cs_handle_reg
In commit dd220a00e8bd ("drm/radeon/kms: add support for streamout v7")
case statements were added without a terminating break statement. This
commit adds the missing break. This was discovered during a compilation
with W=1.
This commit removes the following warning:
drivers/gpu/drm/radeon/evergreen_cs.c:1301:11: warning: this statement may
fall through [-Wimplicit-fallthrough=]
Suggested-by: Alex Deucher
Fixes: dd220a00e8bd ("drm/radeon/kms: add support for streamout v7")
Signed-off-by: Mathieu Malaterre
---
v2: Add missing break statement, instead of marking it as fall through
drivers/gpu/drm/radeon/evergreen_cs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c
b/drivers/gpu/drm/radeon/evergreen_cs.c
index f471537c852f..1e14c6921454 100644
--- a/drivers/gpu/drm/radeon/evergreen_cs.c
+++ b/drivers/gpu/drm/radeon/evergreen_cs.c
@@ -1299,6 +1299,7 @@ static int evergreen_cs_handle_reg(struct
radeon_cs_parser *p, u32 reg, u32 idx)
return -EINVAL;
}
ib[idx] += (u32)((reloc->gpu_offset >> 8) & 0x);
+ break;
case CB_TARGET_MASK:
track->cb_target_mask = radeon_get_ib_value(p, idx);
track->cb_dirty = true;
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/dp: annotate implicit fall throughs
There is a plan to build the kernel with -Wimplicit-fallthrough and
these places in the code produced warnings (W=1). Fix them up.
This commit remove the following warnings:
include/linux/compiler.h:77:22: warning: this statement may fall through
[-Wimplicit-fallthrough=]
include/asm-generic/bug.h:134:2: note: in expansion of macro 'unlikely'
drivers/gpu/drm/drm_dp_helper.c:155:3: note: in expansion of macro 'WARN'
include/linux/compiler.h:77:22: warning: this statement may fall through
[-Wimplicit-fallthrough=]
include/asm-generic/bug.h:134:2: note: in expansion of macro 'unlikely'
drivers/gpu/drm/drm_dp_helper.c:173:3: note: in expansion of macro 'WARN'
drivers/gpu/drm/drm_dp_helper.c:547:3: warning: this statement may fall
through [-Wimplicit-fallthrough=]
Signed-off-by: Mathieu Malaterre
---
drivers/gpu/drm/drm_dp_helper.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 516e82d0ed50..26835d174939 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -154,6 +154,7 @@ u8 drm_dp_link_rate_to_bw_code(int link_rate)
default:
WARN(1, "unknown DP link rate %d, using %x\n", link_rate,
DP_LINK_BW_1_62);
+ /* fall through */
case 162000:
return DP_LINK_BW_1_62;
case 27:
@@ -171,6 +172,7 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw)
switch (link_bw) {
default:
WARN(1, "unknown DP link BW code %x, using 162000\n", link_bw);
+ /* fall through */
case DP_LINK_BW_1_62:
return 162000;
case DP_LINK_BW_2_7:
@@ -552,6 +554,7 @@ int drm_dp_downstream_max_bpc(const u8
dpcd[DP_RECEIVER_CAP_SIZE],
case DP_DS_16BPC:
return 16;
}
+ /* fall through */
default:
return 0;
}
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] video: offb: annotate implicit fall throughs
There is a plan to build the kernel with -Wimplicit-fallthrough and these places in the code produced warnings (W=1). Fix them up. This commit remove the following warnings: drivers/video/fbdev/offb.c:211:5: warning: this statement may fall through [-Wimplicit-fallthrough=] drivers/video/fbdev/offb.c:142:3: warning: this statement may fall through [-Wimplicit-fallthrough=] Signed-off-by: Mathieu Malaterre --- drivers/video/fbdev/offb.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c index 31f769d67195..e0f0c5351306 100644 --- a/drivers/video/fbdev/offb.c +++ b/drivers/video/fbdev/offb.c @@ -141,6 +141,7 @@ static int offb_setcolreg(u_int regno, u_int red, u_int green, u_int blue, /* Clear PALETTE_ACCESS_CNTL in DAC_CNTL */ out_le32(par->cmap_adr + 0x58, in_le32(par->cmap_adr + 0x58) & ~0x20); + /* fall through */ case cmap_r128: /* Set palette index & data */ out_8(par->cmap_adr + 0xb0, regno); @@ -210,6 +211,7 @@ static int offb_blank(int blank, struct fb_info *info) /* Clear PALETTE_ACCESS_CNTL in DAC_CNTL */ out_le32(par->cmap_adr + 0x58, in_le32(par->cmap_adr + 0x58) & ~0x20); + /* fall through */ case cmap_r128: /* Set palette index & data */ out_8(par->cmap_adr + 0xb0, i); -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: radeon: annotate implicit fall through
There is a plan to build the kernel with -Wimplicit-fallthrough and this place in the code produced a warning (W=1). This commit remove the following warning: drivers/gpu/drm/radeon/evergreen_cs.c:1301:11: warning: this statement may fall through [-Wimplicit-fallthrough=] Signed-off-by: Mathieu Malaterre --- drivers/gpu/drm/radeon/evergreen_cs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c b/drivers/gpu/drm/radeon/evergreen_cs.c index f471537c852f..fbf346185790 100644 --- a/drivers/gpu/drm/radeon/evergreen_cs.c +++ b/drivers/gpu/drm/radeon/evergreen_cs.c @@ -1299,6 +1299,7 @@ static int evergreen_cs_handle_reg(struct radeon_cs_parser *p, u32 reg, u32 idx) return -EINVAL; } ib[idx] += (u32)((reloc->gpu_offset >> 8) & 0x); + /* fall through */ case CB_TARGET_MASK: track->cb_target_mask = radeon_get_ib_value(p, idx); track->cb_dirty = true; -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: offb: Fix OF node name handling
On Mon, Jan 7, 2019 at 6:31 PM Rob Herring wrote:
>
> Commit 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
> device_node.name") changed how the OF FB driver handles the OF node
> name. This missed the case where the node name is passed to
> offb_init_palette_hacks(). This results in a NULL ptr dereference
> in strncmp and breaks any system except ones using bootx with no display
> node.
>
> Fix this by making offb_init_palette_hacks() use the OF node pointer and
> use of_node_name_prefix() helper function instead for node name
> comparisons. This helps in moving all OF node name accesses to helper
> functions in preparation to remove struct device_node.name pointer.
>
> Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
> device_node.name")
Looks good to me:
...
[0.00] Linux version 5.0.0-rc1+ (ma...@debian.org) (gcc
version 6.3.0 20170516 (Debian 6.3.0-18)) #21 Mon Jan 7 21:03:53 CET
2019
...
So here is my :
Tested-by: Mathieu Malaterre
Thanks
> Reported-by: Mathieu Malaterre
> Cc: sta...@vger.kernel.org # v4.19+
> Cc: Bartlomiej Zolnierkiewicz
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-fb...@vger.kernel.org
> Signed-off-by: Rob Herring
> ---
> drivers/video/fbdev/offb.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
> index 31f769d67195..057d3cdef92e 100644
> --- a/drivers/video/fbdev/offb.c
> +++ b/drivers/video/fbdev/offb.c
> @@ -318,28 +318,28 @@ static void __iomem *offb_map_reg(struct device_node
> *np, int index,
> }
>
> static void offb_init_palette_hacks(struct fb_info *info, struct device_node
> *dp,
> - const char *name, unsigned long address)
> + unsigned long address)
> {
> struct offb_par *par = (struct offb_par *) info->par;
>
> - if (dp && !strncmp(name, "ATY,Rage128", 11)) {
> + if (of_node_name_prefix(dp, "ATY,Rage128")) {
> par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
> if (par->cmap_adr)
> par->cmap_type = cmap_r128;
> - } else if (dp && (!strncmp(name, "ATY,RageM3pA", 12)
> - || !strncmp(name, "ATY,RageM3p12A", 14))) {
> + } else if (of_node_name_prefix(dp, "ATY,RageM3pA") ||
> + of_node_name_prefix(dp, "ATY,RageM3p12A")) {
> par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
> if (par->cmap_adr)
> par->cmap_type = cmap_M3A;
> - } else if (dp && !strncmp(name, "ATY,RageM3pB", 12)) {
> + } else if (of_node_name_prefix(dp, "ATY,RageM3pB")) {
> par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
> if (par->cmap_adr)
> par->cmap_type = cmap_M3B;
> - } else if (dp && !strncmp(name, "ATY,Rage6", 9)) {
> + } else if (of_node_name_prefix(dp, "ATY,Rage6")) {
> par->cmap_adr = offb_map_reg(dp, 1, 0, 0x1fff);
> if (par->cmap_adr)
> par->cmap_type = cmap_radeon;
> - } else if (!strncmp(name, "ATY,", 4)) {
> + } else if (of_node_name_prefix(dp, "ATY,")) {
> unsigned long base = address & 0xff00UL;
> par->cmap_adr =
> ioremap(base + 0x7ff000, 0x1000) + 0xcc0;
> @@ -350,7 +350,7 @@ static void offb_init_palette_hacks(struct fb_info *info,
> struct device_node *dp
> par->cmap_adr = offb_map_reg(dp, 0, 0x6000, 0x1000);
> if (par->cmap_adr)
> par->cmap_type = cmap_gxt2000;
> - } else if (dp && !strncmp(name, "vga,Display-", 12)) {
> + } else if (of_node_name_prefix(dp, "vga,Display-")) {
> /* Look for AVIVO initialized by SLOF */
> struct device_node *pciparent = of_get_parent(dp);
> const u32 *vid, *did;
> @@ -438,7 +438,7 @@ static void __init offb_init_fb(const char *name,
>
> par->cmap_type = cmap_unknown;
> if (depth == 8)
> - offb_init_palette_hacks(info, dp, name, address);
> + offb_init_palette_hacks(info, dp, address);
> else
> fix->visual = FB_VISUAL_TRUECOLOR;
>
> --
> 2.19.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: offb: Fix OF node name handling
Hi Rob,
On Mon, Jan 7, 2019 at 9:11 PM Mathieu Malaterre wrote:
>
> On Mon, Jan 7, 2019 at 6:31 PM Rob Herring wrote:
> >
> > Commit 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
> > device_node.name") changed how the OF FB driver handles the OF node
> > name. This missed the case where the node name is passed to
> > offb_init_palette_hacks(). This results in a NULL ptr dereference
> > in strncmp and breaks any system except ones using bootx with no display
> > node.
> >
> > Fix this by making offb_init_palette_hacks() use the OF node pointer and
> > use of_node_name_prefix() helper function instead for node name
> > comparisons. This helps in moving all OF node name accesses to helper
> > functions in preparation to remove struct device_node.name pointer.
> >
> > Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
> > device_node.name")
>
> Looks good to me:
>
> ...
> [0.00] Linux version 5.0.0-rc1+ (ma...@debian.org) (gcc
> version 6.3.0 20170516 (Debian 6.3.0-18)) #21 Mon Jan 7 21:03:53 CET
> 2019
> ...
>
> So here is my :
>
> Tested-by: Mathieu Malaterre
Just for curiosity, why would you keep:
if (strcmp(dp->name, "valkyrie") == 0)
> > Reported-by: Mathieu Malaterre
> > Cc: sta...@vger.kernel.org # v4.19+
> > Cc: Bartlomiej Zolnierkiewicz
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-fb...@vger.kernel.org
> > Signed-off-by: Rob Herring
> > ---
> > drivers/video/fbdev/offb.c | 18 +-
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
> > index 31f769d67195..057d3cdef92e 100644
> > --- a/drivers/video/fbdev/offb.c
> > +++ b/drivers/video/fbdev/offb.c
> > @@ -318,28 +318,28 @@ static void __iomem *offb_map_reg(struct device_node
> > *np, int index,
> > }
> >
> > static void offb_init_palette_hacks(struct fb_info *info, struct
> > device_node *dp,
> > - const char *name, unsigned long address)
> > + unsigned long address)
> > {
> > struct offb_par *par = (struct offb_par *) info->par;
> >
> > - if (dp && !strncmp(name, "ATY,Rage128", 11)) {
> > + if (of_node_name_prefix(dp, "ATY,Rage128")) {
> > par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
> > if (par->cmap_adr)
> > par->cmap_type = cmap_r128;
> > - } else if (dp && (!strncmp(name, "ATY,RageM3pA", 12)
> > - || !strncmp(name, "ATY,RageM3p12A", 14))) {
> > + } else if (of_node_name_prefix(dp, "ATY,RageM3pA") ||
> > + of_node_name_prefix(dp, "ATY,RageM3p12A")) {
> > par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
> > if (par->cmap_adr)
> > par->cmap_type = cmap_M3A;
> > - } else if (dp && !strncmp(name, "ATY,RageM3pB", 12)) {
> > + } else if (of_node_name_prefix(dp, "ATY,RageM3pB")) {
> > par->cmap_adr = offb_map_reg(dp, 2, 0, 0x1fff);
> > if (par->cmap_adr)
> > par->cmap_type = cmap_M3B;
> > - } else if (dp && !strncmp(name, "ATY,Rage6", 9)) {
> > + } else if (of_node_name_prefix(dp, "ATY,Rage6")) {
> > par->cmap_adr = offb_map_reg(dp, 1, 0, 0x1fff);
> > if (par->cmap_adr)
> > par->cmap_type = cmap_radeon;
> > - } else if (!strncmp(name, "ATY,", 4)) {
> > + } else if (of_node_name_prefix(dp, "ATY,")) {
> > unsigned long base = address & 0xff00UL;
> > par->cmap_adr =
> > ioremap(base + 0x7ff000, 0x1000) + 0xcc0;
> > @@ -350,7 +350,7 @@ static void offb_init_palette_hacks(struct fb_info
> > *info, struct device_node *dp
> > par->cmap_adr = offb_map_reg(dp, 0, 0x6000, 0x1000);
> > if (par->cmap_adr)
> > par->cmap_type = cmap_gxt2000;
> > - } else if (dp && !strncmp(name, "vga,Display-", 12)) {
> > + } else if (of_node_name_prefix(dp, "vga,Display-")) {
> > /* Look for AVIVO initialized by SLOF */
> > struct device_node *pciparent = of_get_parent(dp);
> > const u32 *vid, *did;
> > @@ -438,7 +438,7 @@ static void __init offb_init_fb(const char *name,
> >
> > par->cmap_type = cmap_unknown;
> > if (depth == 8)
> > - offb_init_palette_hacks(info, dp, name, address);
> > + offb_init_palette_hacks(info, dp, address);
> > else
> > fix->visual = FB_VISUAL_TRUECOLOR;
> >
> > --
> > 2.19.1
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: offb: Pass actual name in offb_init_palette_hacks
Bartlomiej,
Do you need an Acked-by from Rob, or can you take it in the next round
of fixes for v4.20 ?
Just to repeat myself, previous code would call
offb_init_palette_hacks(), which in turn would do:
if (dp && !strncmp(name, "ATY,Rage128", 11)) {
with name=NULL.
Thanks
On Thu, Dec 20, 2018 at 5:10 PM Mathieu Malaterre wrote:
>
> Rob, any comment ?
>
> On Fri, Dec 7, 2018 at 1:59 PM Mathieu Malaterre wrote:
> >
> > This is a partial revert of commit 5c63e407aaab ("fbdev: Convert to
> > using %pOFn instead of device_node.name"). This is the minimal work to
> > get a Mac Mini G4 back to a bootable state. The function
> > offb_init_palette_hacks would need to handle the case where `name` has
> > been set to NULL.
> >
> > Cc: Rob Herring
> > Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
> > device_node.name")
> > Cc: sta...@vger.kernel.org # v4.19+
> > Signed-off-by: Mathieu Malaterre
> > ---
> > drivers/video/fbdev/offb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
> > index 31f769d67195..6e75851f8142 100644
> > --- a/drivers/video/fbdev/offb.c
> > +++ b/drivers/video/fbdev/offb.c
> > @@ -648,7 +648,7 @@ static void __init offb_init_nodriver(struct
> > device_node *dp, int no_real_node)
> > /* kludge for valkyrie */
> > if (strcmp(dp->name, "valkyrie") == 0)
> > address += 0x1000;
> > - offb_init_fb(no_real_node ? "bootx" : NULL,
> > + offb_init_fb(no_real_node ? "bootx" : dp->name,
> > width, height, depth, pitch, address,
> > foreign_endian, no_real_node ? NULL : dp);
> > }
> > --
> > 2.19.2
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: offb: Pass actual name in offb_init_palette_hacks
Patch is far from perfect but was tested also by: Elimar Riesebieter
riese...@lxtec.de
Le jeu. 20 déc. 2018 17:10, Mathieu Malaterre a écrit :
> Rob, any comment ?
>
> On Fri, Dec 7, 2018 at 1:59 PM Mathieu Malaterre wrote:
> >
> > This is a partial revert of commit 5c63e407aaab ("fbdev: Convert to
> > using %pOFn instead of device_node.name"). This is the minimal work to
> > get a Mac Mini G4 back to a bootable state. The function
> > offb_init_palette_hacks would need to handle the case where `name` has
> > been set to NULL.
> >
> > Cc: Rob Herring
> > Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
> device_node.name")
> > Cc: sta...@vger.kernel.org # v4.19+
> > Signed-off-by: Mathieu Malaterre
> > ---
> > drivers/video/fbdev/offb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
> > index 31f769d67195..6e75851f8142 100644
> > --- a/drivers/video/fbdev/offb.c
> > +++ b/drivers/video/fbdev/offb.c
> > @@ -648,7 +648,7 @@ static void __init offb_init_nodriver(struct
> device_node *dp, int no_real_node)
> > /* kludge for valkyrie */
> > if (strcmp(dp->name, "valkyrie") == 0)
> > address += 0x1000;
> > - offb_init_fb(no_real_node ? "bootx" : NULL,
> > + offb_init_fb(no_real_node ? "bootx" : dp->name,
> > width, height, depth, pitch, address,
> > foreign_endian, no_real_node ? NULL : dp);
> > }
> > --
> > 2.19.2
> >
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: offb: Pass actual name in offb_init_palette_hacks
Rob, any comment ?
On Fri, Dec 7, 2018 at 1:59 PM Mathieu Malaterre wrote:
>
> This is a partial revert of commit 5c63e407aaab ("fbdev: Convert to
> using %pOFn instead of device_node.name"). This is the minimal work to
> get a Mac Mini G4 back to a bootable state. The function
> offb_init_palette_hacks would need to handle the case where `name` has
> been set to NULL.
>
> Cc: Rob Herring
> Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
> device_node.name")
> Cc: sta...@vger.kernel.org # v4.19+
> Signed-off-by: Mathieu Malaterre
> ---
> drivers/video/fbdev/offb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
> index 31f769d67195..6e75851f8142 100644
> --- a/drivers/video/fbdev/offb.c
> +++ b/drivers/video/fbdev/offb.c
> @@ -648,7 +648,7 @@ static void __init offb_init_nodriver(struct device_node
> *dp, int no_real_node)
> /* kludge for valkyrie */
> if (strcmp(dp->name, "valkyrie") == 0)
> address += 0x1000;
> - offb_init_fb(no_real_node ? "bootx" : NULL,
> + offb_init_fb(no_real_node ? "bootx" : dp->name,
> width, height, depth, pitch, address,
> foreign_endian, no_real_node ? NULL : dp);
> }
> --
> 2.19.2
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] video: offb: Pass actual name in offb_init_palette_hacks
This is a partial revert of commit 5c63e407aaab ("fbdev: Convert to
using %pOFn instead of device_node.name"). This is the minimal work to
get a Mac Mini G4 back to a bootable state. The function
offb_init_palette_hacks would need to handle the case where `name` has
been set to NULL.
Cc: Rob Herring
Fixes: 5c63e407aaab ("fbdev: Convert to using %pOFn instead of
device_node.name")
Cc: sta...@vger.kernel.org # v4.19+
Signed-off-by: Mathieu Malaterre
---
drivers/video/fbdev/offb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
index 31f769d67195..6e75851f8142 100644
--- a/drivers/video/fbdev/offb.c
+++ b/drivers/video/fbdev/offb.c
@@ -648,7 +648,7 @@ static void __init offb_init_nodriver(struct device_node
*dp, int no_real_node)
/* kludge for valkyrie */
if (strcmp(dp->name, "valkyrie") == 0)
address += 0x1000;
- offb_init_fb(no_real_node ? "bootx" : NULL,
+ offb_init_fb(no_real_node ? "bootx" : dp->name,
width, height, depth, pitch, address,
foreign_endian, no_real_node ? NULL : dp);
}
--
2.19.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: change function signature to pass full range
On Fri, Apr 13, 2018 at 9:59 AM Huang Rui wrote:
> On Thu, Apr 12, 2018 at 09:33:33PM +0200, Mathieu Malaterre wrote:
> > In function ‘radeon_process_i2c_ch’ a comparison of a u8 value against
> > 255 is done. Since it is always false, change the signature of this
> > function to use an `int` instead, which match the type used in caller:
> > `radeon_atom_hw_i2c_xfer`.
> >
> > Fix the following warning triggered with W=1:
> >
> > CC [M] drivers/gpu/drm/radeon/atombios_i2c.o
> > drivers/gpu/drm/radeon/atombios_i2c.c: In function
> ‘radeon_process_i2c_ch’:
> > drivers/gpu/drm/radeon/atombios_i2c.c:71:11: warning: comparison is
> always false due to limited range of data type [-Wtype-limits]
> > if (num > ATOM_MAX_HW_I2C_READ) {
> >^
> >
> > Signed-off-by: Mathieu Malaterre
>
> Reviewed-by: Huang Rui
>
>
Any chance to be included in the next pull request ? thanks
> > ---
> > drivers/gpu/drm/radeon/atombios_i2c.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/atombios_i2c.c
> b/drivers/gpu/drm/radeon/atombios_i2c.c
> > index 4157780585a0..9022e9af11a0 100644
> > --- a/drivers/gpu/drm/radeon/atombios_i2c.c
> > +++ b/drivers/gpu/drm/radeon/atombios_i2c.c
> > @@ -35,7 +35,7 @@
> >
> > static int radeon_process_i2c_ch(struct radeon_i2c_chan *chan,
> >u8 slave_addr, u8 flags,
> > - u8 *buf, u8 num)
> > + u8 *buf, int num)
> > {
> > struct drm_device *dev = chan->dev;
> > struct radeon_device *rdev = dev->dev_private;
> > --
> > 2.11.0
> >
> > ___
> > amd-gfx mailing list
> > amd-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: 答复: Tracking: radeon 0000:00:10.0: ring 0 stalled for more than 10240msec
Hi all, On Wed, May 2, 2018 at 4:39 AM, Qu, Jim wrote: > Hi , > > If you are sure that the HW worked fine before. I think you should: > > 1. Be sure that HW works fine now. > 2. recall the driver to the point at where it works well, and then replace > them one by one to confirm component which causes the issue. > 3. try to update the last VBIOS to adapt new driver. > > Thanks > JimQu > > > 发件人: amd-gfx 代表 Christian König > > 发送时间: 2018年4月30日 1:16:14 > 收件人: Mathieu Malaterre; Deucher, Alexander > 抄送: David Airlie; Zhou, David(ChunMing); dri-devel; > amd-...@lists.freedesktop.org; LKML > 主题: Re: Tracking: radeon :00:10.0: ring 0 stalled for more than 10240msec > > Am 23.04.2018 um 20:50 schrieb Mathieu Malaterre: >> Hi there, >> >> I am pretty sure I was able to run kodi on an old Mac Mini G4 (big >> endian) with AMD RV280. Today it is failing to start with: > > Well, that is rather old hardware. I suggest to make sure first that the > hw isn't broken in some way. > >> How should I go and debug this (other than plain git-bisect) ? > > You first need to figure out what's the failing component. Either Mesa, > DDX or the Kernel are possible candidates. > > Another possibility is that you updated kodi and kodi is now doing > something the hw doesn't like. That was my mistake, I forgot about AGP vs PCI on radeon. Should be fixed in next release: https://patchwork.kernel.org/patch/10360887/ > Regards, > Christian. > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: Change the default to PCI on PowerPC
For reference: https://lists.freedesktop.org/archives/mesa-dev/2016-May/115453.html 2018-04-24 21:55 GMT+02:00 Mathieu Malaterre : > AGP mode is unstable on PowerPC. Symptoms are generally of the form: > > [ 1228.795711] radeon :00:10.0: ring 0 stalled for more than 10240msec > > Reviewed-by: Christian König > Signed-off-by: Mathieu Malaterre > --- > drivers/gpu/drm/radeon/radeon_drv.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c > b/drivers/gpu/drm/radeon/radeon_drv.c > index b28288a781ef..2a7977a23b31 100644 > --- a/drivers/gpu/drm/radeon/radeon_drv.c > +++ b/drivers/gpu/drm/radeon/radeon_drv.c > @@ -168,7 +168,12 @@ int radeon_no_wb; > int radeon_modeset = -1; > int radeon_dynclks = -1; > int radeon_r4xx_atom = 0; > +#ifdef __powerpc__ > +/* Default to PCI on PowerPC (fdo #95017) */ > +int radeon_agpmode = -1; > +#else > int radeon_agpmode = 0; > +#endif > int radeon_vram_limit = 0; > int radeon_gart_size = -1; /* auto */ > int radeon_benchmarking = 0; > -- > 2.11.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: Change the default to PCI on PowerPC
AGP mode is unstable on PowerPC. Symptoms are generally of the form: [ 1228.795711] radeon :00:10.0: ring 0 stalled for more than 10240msec Reviewed-by: Christian König Signed-off-by: Mathieu Malaterre --- drivers/gpu/drm/radeon/radeon_drv.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index b28288a781ef..2a7977a23b31 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -168,7 +168,12 @@ int radeon_no_wb; int radeon_modeset = -1; int radeon_dynclks = -1; int radeon_r4xx_atom = 0; +#ifdef __powerpc__ +/* Default to PCI on PowerPC (fdo #95017) */ +int radeon_agpmode = -1; +#else int radeon_agpmode = 0; +#endif int radeon_vram_limit = 0; int radeon_gart_size = -1; /* auto */ int radeon_benchmarking = 0; -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Tracking: radeon 0000:00:10.0: ring 0 stalled for more than 10240msec
Hi there, I am pretty sure I was able to run kodi on an old Mac Mini G4 (big endian) with AMD RV280. Today it is failing to start with: [ 162.971551] radeon :00:10.0: ring 0 stalled for more than 10240msec [ 162.971568] radeon :00:10.0: GPU lockup (current fence id 0x01d6 last fence id 0x01d7 on ring 0) [ 163.482863] radeon :00:10.0: ring 0 stalled for more than 10752msec [ 163.482880] radeon :00:10.0: GPU lockup (current fence id 0x01d6 last fence id 0x01d7 on ring 0) [ 163.994225] radeon :00:10.0: ring 0 stalled for more than 11264msec [ 163.994241] radeon :00:10.0: GPU lockup (current fence id 0x01d6 last fence id 0x01d7 on ring 0) [ 164.505598] radeon :00:10.0: ring 0 stalled for more than 11776msec [ 164.505614] radeon :00:10.0: GPU lockup (current fence id 0x01d6 last fence id 0x01d7 on ring 0) [ 165.016996] radeon :00:10.0: ring 0 stalled for more than 12288msec [ 165.017013] radeon :00:10.0: GPU lockup (current fence id 0x01d6 last fence id 0x01d7 on ring 0) [ 165.528429] radeon :00:10.0: ring 0 stalled for more than 12800msec [ 165.528446] radeon :00:10.0: GPU lockup (current fence id 0x01d6 last fence id 0x01d7 on ring 0) [ 166.039865] radeon :00:10.0: ring 0 stalled for more than 13312msec [ 166.039882] radeon :00:10.0: GPU lockup (current fence id 0x01d6 last fence id 0x01d7 on ring 0) [ 166.551351] radeon :00:10.0: ring 0 stalled for more than 13824msec [ 166.551368] radeon :00:10.0: GPU lockup (current fence id 0x01d6 last fence id 0x01d7 on ring 0) [ 167.062819] radeon :00:10.0: ring 0 stalled for more than 14336msec [ 167.062836] radeon :00:10.0: GPU lockup (current fence id 0x01d6 last fence id 0x01d7 on ring 0) [ 167.574331] radeon :00:10.0: ring 0 stalled for more than 14848msec [ 167.574348] radeon :00:10.0: GPU lockup (current fence id 0x01d6 last fence id 0x01d7 on ring 0) [ 167.798244] [TTM] Buffer eviction failed [ 167.940488] radeon: wait for empty RBBM fifo failed! Bad things might happen. [ 168.076053] Failed to wait GUI idle while programming pipes. Bad things might happen. [ 168.092258] radeon :00:10.0: Saved 91 dwords of commands on ring 0. [ 168.092380] radeon :00:10.0: (r100_asic_reset:2560) RBBM_STATUS=0x83F96100 [ 168.589895] radeon :00:10.0: (r100_asic_reset:2581) RBBM_STATUS=0x80010140 [ 169.083456] radeon :00:10.0: (r100_asic_reset:2589) RBBM_STATUS=0x0140 [ 169.083482] radeon :00:10.0: GPU reset succeed [ 169.083487] radeon :00:10.0: GPU reset succeeded, trying to resume [ 169.083550] radeon :00:10.0: WB disabled [ 169.083561] radeon :00:10.0: fence driver on ring 0 use gpu addr 0x and cpu addr 0x883a5378 [ 169.083612] [drm] radeon: ring at 0x1000 [ 169.228838] [drm:r100_ring_test [radeon]] *ERROR* radeon: ring test failed (scratch(0x15E8)=0xCAFEDEAD) [ 169.228910] [drm:r100_cp_init [radeon]] *ERROR* radeon: cp isn't working (-22). [ 169.228919] radeon :00:10.0: failed initializing CP (-22). How should I go and debug this (other than plain git-bisect) ? For reference: # modprobe radeon ... [ 100.369890] [drm] radeon kernel modesetting enabled. [ 100.377816] checking generic (9c008000 5a000) vs hw (9800 800) [ 100.377824] fb: switching to radeondrmfb from OFfb ATY,RockHo [ 100.382566] Console: switching to colour dummy device 80x25 [ 100.386224] radeon :00:10.0: enabling device (0006 -> 0007) [ 100.389596] [drm] initializing kernel modesetting (RV280 0x1002:0x5962 0x1002:0x5962 0x01). [ 100.389783] radeon :00:10.0: Invalid PCI ROM header signature: expecting 0xaa55, got 0x [ 100.389813] radeon :00:10.0: Invalid PCI ROM header signature: expecting 0xaa55, got 0x [ 100.390218] [drm:radeon_get_bios [radeon]] *ERROR* Unable to locate a BIOS ROM [ 100.390247] [drm] Using device-tree clock info [ 100.390286] agpgart-uninorth :00:0b.0: putting AGP V2 device into 4x mode [ 100.390300] radeon :00:10.0: putting AGP V2 device into 4x mode [ 100.390345] radeon :00:10.0: GTT: 256M 0x - 0x0FFF [ 100.390355] [drm] Generation 2 PCI interface, using max accessible memory [ 100.390368] radeon :00:10.0: VRAM: 128M 0x9800 - 0x9FFF (32M used) [ 100.390406] [drm] Detected VRAM RAM=128M, BAR=128M [ 100.390415] [drm] RAM width 64bits DDR [ 100.405414] [TTM] Zone kernel: Available graphics memory: 254062 kiB [ 100.405444] [TTM] Initializing pool allocator [ 100.405542] [drm] radeon: 32M of VRAM memory ready [ 100.405554] [drm] radeon: 256M of GTT memory ready. [ 100.406161] radeon :00:10.0: WB disabled [ 100.406187] radeon :00:10.0: fence driver on ring 0 use gp
[PATCH] drm/radeon: change function signature to pass full range
In function ‘radeon_process_i2c_ch’ a comparison of a u8 value against
255 is done. Since it is always false, change the signature of this
function to use an `int` instead, which match the type used in caller:
`radeon_atom_hw_i2c_xfer`.
Fix the following warning triggered with W=1:
CC [M] drivers/gpu/drm/radeon/atombios_i2c.o
drivers/gpu/drm/radeon/atombios_i2c.c: In function ‘radeon_process_i2c_ch’:
drivers/gpu/drm/radeon/atombios_i2c.c:71:11: warning: comparison is always
false due to limited range of data type [-Wtype-limits]
if (num > ATOM_MAX_HW_I2C_READ) {
^
Signed-off-by: Mathieu Malaterre
---
drivers/gpu/drm/radeon/atombios_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/atombios_i2c.c
b/drivers/gpu/drm/radeon/atombios_i2c.c
index 4157780585a0..9022e9af11a0 100644
--- a/drivers/gpu/drm/radeon/atombios_i2c.c
+++ b/drivers/gpu/drm/radeon/atombios_i2c.c
@@ -35,7 +35,7 @@
static int radeon_process_i2c_ch(struct radeon_i2c_chan *chan,
u8 slave_addr, u8 flags,
-u8 *buf, u8 num)
+u8 *buf, int num)
{
struct drm_device *dev = chan->dev;
struct radeon_device *rdev = dev->dev_private;
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5] Fix loading of module radeonfb on PowerMac
When the linux kernel is build with (typical kernel ship with Debian
installer):
CONFIG_FB=y
CONFIG_FB_OF=y
CONFIG_VT_HW_CONSOLE_BINDING=y
CONFIG_FB_RADEON=m
The offb driver takes precedence over module radeonfb. It is then
impossible to load the module, error reported is:
[ 96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007)
[ 96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem
0x9800-0x9fff pref]
[ 96.551531] radeonfb (:00:10.0): cannot request region 0.
[ 96.551545] radeonfb: probe of :00:10.0 failed with error -16
This patch reproduce the behavior of the module radeon, so as to make it
possible to load radeonfb when offb is first loaded, see
commit a56f7428d753 ("drm/radeon: Add early unregister of firmware fb's").
Signed-off-by: Mathieu Malaterre
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id=119741
Suggested-by: Lennart Sorensen
Cc: Bartlomiej Zolnierkiewicz
Cc: Benjamin Herrenschmidt
Cc: Tomi Valkeinen
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since
the conflicts in region is due to OFfb explicitly refers to it.
v4: Use drm_fb_helper_remove_conflicting_framebuffers from drm_fb_helper
v5: do not use drm_fb_helper_remove_conflicting_framebuffers from drm_fb_helper
remove ifdef hacks since not needed once framebuffer is properly decremented
drivers/video/fbdev/aty/radeon_base.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/video/fbdev/aty/radeon_base.c
b/drivers/video/fbdev/aty/radeon_base.c
index 87608c0b2351..e8594bbaea60 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -2255,6 +2255,23 @@ static const struct bin_attribute edid2_attr = {
.read = radeon_show_edid2,
};
+static int radeon_kick_out_firmware_fb(struct pci_dev *pdev)
+{
+ struct apertures_struct *ap;
+
+ ap = alloc_apertures(1);
+ if (!ap)
+ return -ENOMEM;
+
+ ap->ranges[0].base = pci_resource_start(pdev, 0);
+ ap->ranges[0].size = pci_resource_len(pdev, 0);
+
+ remove_conflicting_framebuffers(ap, KBUILD_MODNAME, false);
+
+ kfree(ap);
+
+ return 0;
+}
static int radeonfb_pci_register(struct pci_dev *pdev,
const struct pci_device_id *ent)
@@ -2308,6 +2325,10 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
rinfo->fb_base_phys = pci_resource_start (pdev, 0);
rinfo->mmio_base_phys = pci_resource_start (pdev, 2);
+ ret = radeon_kick_out_firmware_fb(pdev);
+ if (ret)
+ return ret;
+
/* request the mem regions */
ret = pci_request_region(pdev, 0, "radeonfb framebuffer");
if (ret < 0) {
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] Fix loading of module radeonfb on PowerMac
On Tue, Feb 13, 2018 at 1:05 PM, Bartlomiej Zolnierkiewicz wrote: > On Saturday, February 10, 2018 01:48:55 PM Mathieu Malaterre wrote: >> Hi, >> >> On Thu, Feb 8, 2018 at 2:28 PM, Bartlomiej Zolnierkiewicz >> wrote: >> > On Wednesday, January 31, 2018 08:51:23 PM Mathieu Malaterre wrote: >> >> Bartlomiej, >> >> >> >> On Wed, Jan 31, 2018 at 12:57 PM, Bartlomiej Zolnierkiewicz >> >> wrote: >> >> > On Tuesday, January 30, 2018 02:14:10 PM Mathieu Malaterre wrote: >> >> >> Bartlomiej, >> >> >> >> >> >> On Wed, Jan 3, 2018 at 3:47 PM, Bartlomiej Zolnierkiewicz >> >> >> wrote: >> >> >> > >> >> >> > On Thursday, December 21, 2017 11:07:56 PM Mathieu Malaterre wrote: >> >> >> >> When the linux kernel is build with (typical kernel ship with Debian >> >> >> >> installer): >> >> >> >> >> >> >> >> CONFIG_FB_OF=y >> >> >> >> CONFIG_VT_HW_CONSOLE_BINDING=y >> >> >> >> CONFIG_FB_RADEON=m >> >> >> >> >> >> >> >> The offb driver takes precedence over module radeonfb. It is then >> >> >> >> impossible to load the module, error reported is: >> >> >> >> >> >> >> >> [ 96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007) >> >> >> >> [ 96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem >> >> >> >> 0x9800-0x9fff pref] >> >> >> >> [ 96.551531] radeonfb (:00:10.0): cannot request region 0. >> >> >> >> [ 96.551545] radeonfb: probe of :00:10.0 failed with error -16 >> >> >> >> >> >> >> >> This patch reproduce the behavior of the module radeon, so as to >> >> >> >> make it >> >> >> >> possible to load radeonfb when offb is first loaded. >> >> >> >> >> >> >> >> It should be noticed that `offb_destroy` is never called which >> >> >> >> explain the >> >> >> >> need to skip error detection on the radeon side. >> >> >> > >> >> >> > This still needs to be explained more, from my last mail: >> >> >> > >> >> >> > "The last put_fb_info() on fb_info should call ->fb_destroy >> >> >> > (offb_destroy in our case) and remove_conflicting_framebuffers() >> >> >> > is calling put_fb_info() so there is some extra reference on >> >> >> > fb_info somewhere preventing it from going away. >> >> >> > >> >> >> > Please look into fixing this." >> >> >> >> >> >> I am not familiar with the fb stuff internals but here is what I see: >> >> >> >> >> >> # modprobe radeonfb >> >> >> >> >> >> leads to: >> >> >> >> >> >> [ 52.058546] bus: 'pci': add driver radeonfb >> >> >> [ 52.058588] bus: 'pci': driver_probe_device: matched device >> >> >> :00:10.0 with driver radeonfb >> >> >> [ 52.058595] bus: 'pci': really_probe: probing driver radeonfb with >> >> >> device :00:10.0 >> >> >> [ 52.058608] devices_kset: Moving :00:10.0 to end of list >> >> >> [ 52.058613] radeonfb_pci_register BEGIN >> >> >> [ 52.058634] radeonfb :00:10.0: enabling device (0006 -> 0007) >> >> >> >> >> >> [ 52.058666] checking generic (9c008000 96000) vs hw (9800 >> >> >> 800) >> >> >> [ 52.058667] fb: switching to radeonfb from OFfb ATY,RockHo >> >> >> [ 52.058844] Console: switching to colour dummy device 80x25 >> >> >> [ 52.058860] device: 'fb0': device_unregister >> >> >> [ 52.058956] PM: Removing info for No Bus:fb0 >> >> >> [ 52.059014] device: 'fb0': device_create_release >> >> >> >> >> >> >> >> >> [ 52.059048] device: 'vtcon1': device_unregister >> >> >> [ 52.059076] PM: Removing info for No Bus:vtcon1 >> >> >> [ 52.059091] device: 'vtcon1': device
[PATCH] video: offb: Deallocate the color map
The function offb_destroy did not deallocate the color map leaving some memory around after destruction. Call the color map deallocate function to remove the memory leak. Handle another case where color map should have been deallocated during an error code path. Fix memory leaks reported by kmemleak: # dmesg ... [ 1884.719941] kmemleak: 3 new suspected memory leaks (see /sys/kernel/debug/kmemleak) # cat /sys/kernel/debug/kmemleak unreferenced object 0xde3d9000 (size 512): comm "swapper", pid 1, jiffies 4294892827 (age 1906.784s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 aa aa aa aa aa aa aa aa 55 55 55 55 55 55 55 55 ff ff ff ff ff ff ff ff backtrace: [] fb_alloc_cmap_gfp+0x54/0x15c [] offb_init_nodriver+0x8e8/0xa3c [] offb_init+0xd0/0x164 [<322f82a3>] do_one_initcall+0x4c/0x178 [] kernel_init_freeable+0x138/0x1cc [<2a17fa0e>] kernel_init+0x24/0x118 [<4079749a>] ret_from_kernel_thread+0x5c/0x64 unreferenced object 0xde3d9200 (size 512): comm "swapper", pid 1, jiffies 4294892827 (age 1906.784s) hex dump (first 32 bytes): 00 00 00 00 aa aa aa aa 00 00 00 00 55 55 aa aa UU.. 55 55 55 55 ff ff ff ff 55 55 55 55 ff ff ff ff backtrace: [<4bf3594d>] fb_alloc_cmap_gfp+0x6c/0x15c [] offb_init_nodriver+0x8e8/0xa3c [] offb_init+0xd0/0x164 [<322f82a3>] do_one_initcall+0x4c/0x178 [] kernel_init_freeable+0x138/0x1cc [<2a17fa0e>] kernel_init+0x24/0x118 [<4079749a>] ret_from_kernel_thread+0x5c/0x64 unreferenced object 0xde3d9600 (size 512): comm "swapper", pid 1, jiffies 4294892827 (age 1906.784s) hex dump (first 32 bytes): 00 00 aa aa 00 00 aa aa 00 00 aa aa 00 00 aa aa 55 55 ff ff 55 55 ff ff 55 55 ff ff 55 55 ff ff UU..UU..UU..UU.. backtrace: [<23a3ea03>] fb_alloc_cmap_gfp+0x84/0x15c [] offb_init_nodriver+0x8e8/0xa3c [] offb_init+0xd0/0x164 [<322f82a3>] do_one_initcall+0x4c/0x178 [] kernel_init_freeable+0x138/0x1cc [<2a17fa0e>] kernel_init+0x24/0x118 [<4079749a>] ret_from_kernel_thread+0x5c/0x64 Signed-off-by: Mathieu Malaterre --- drivers/video/fbdev/offb.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c index 6f65d132eba1..30786445de04 100644 --- a/drivers/video/fbdev/offb.c +++ b/drivers/video/fbdev/offb.c @@ -282,6 +282,7 @@ static void offb_destroy(struct fb_info *info) if (info->screen_base) iounmap(info->screen_base); release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size); + fb_dealloc_cmap(&info->cmap); framebuffer_release(info); } @@ -520,6 +521,7 @@ static void __init offb_init_fb(const char *name, return; out_err: + fb_dealloc_cmap(&info->cmap); iounmap(info->screen_base); out_aper: iounmap(par->cmap_adr); -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] Fix loading of module radeonfb on PowerMac
Hi, On Thu, Feb 8, 2018 at 2:28 PM, Bartlomiej Zolnierkiewicz wrote: > On Wednesday, January 31, 2018 08:51:23 PM Mathieu Malaterre wrote: >> Bartlomiej, >> >> On Wed, Jan 31, 2018 at 12:57 PM, Bartlomiej Zolnierkiewicz >> wrote: >> > On Tuesday, January 30, 2018 02:14:10 PM Mathieu Malaterre wrote: >> >> Bartlomiej, >> >> >> >> On Wed, Jan 3, 2018 at 3:47 PM, Bartlomiej Zolnierkiewicz >> >> wrote: >> >> > >> >> > On Thursday, December 21, 2017 11:07:56 PM Mathieu Malaterre wrote: >> >> >> When the linux kernel is build with (typical kernel ship with Debian >> >> >> installer): >> >> >> >> >> >> CONFIG_FB_OF=y >> >> >> CONFIG_VT_HW_CONSOLE_BINDING=y >> >> >> CONFIG_FB_RADEON=m >> >> >> >> >> >> The offb driver takes precedence over module radeonfb. It is then >> >> >> impossible to load the module, error reported is: >> >> >> >> >> >> [ 96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007) >> >> >> [ 96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem >> >> >> 0x9800-0x9fff pref] >> >> >> [ 96.551531] radeonfb (:00:10.0): cannot request region 0. >> >> >> [ 96.551545] radeonfb: probe of :00:10.0 failed with error -16 >> >> >> >> >> >> This patch reproduce the behavior of the module radeon, so as to make >> >> >> it >> >> >> possible to load radeonfb when offb is first loaded. >> >> >> >> >> >> It should be noticed that `offb_destroy` is never called which explain >> >> >> the >> >> >> need to skip error detection on the radeon side. >> >> > >> >> > This still needs to be explained more, from my last mail: >> >> > >> >> > "The last put_fb_info() on fb_info should call ->fb_destroy >> >> > (offb_destroy in our case) and remove_conflicting_framebuffers() >> >> > is calling put_fb_info() so there is some extra reference on >> >> > fb_info somewhere preventing it from going away. >> >> > >> >> > Please look into fixing this." >> >> >> >> I am not familiar with the fb stuff internals but here is what I see: >> >> >> >> # modprobe radeonfb >> >> >> >> leads to: >> >> >> >> [ 52.058546] bus: 'pci': add driver radeonfb >> >> [ 52.058588] bus: 'pci': driver_probe_device: matched device >> >> :00:10.0 with driver radeonfb >> >> [ 52.058595] bus: 'pci': really_probe: probing driver radeonfb with >> >> device :00:10.0 >> >> [ 52.058608] devices_kset: Moving :00:10.0 to end of list >> >> [ 52.058613] radeonfb_pci_register BEGIN >> >> [ 52.058634] radeonfb :00:10.0: enabling device (0006 -> 0007) >> >> >> >> [ 52.058666] checking generic (9c008000 96000) vs hw (9800 800) >> >> [ 52.058667] fb: switching to radeonfb from OFfb ATY,RockHo >> >> [ 52.058844] Console: switching to colour dummy device 80x25 >> >> [ 52.058860] device: 'fb0': device_unregister >> >> [ 52.058956] PM: Removing info for No Bus:fb0 >> >> [ 52.059014] device: 'fb0': device_create_release >> >> >> >> >> >> [ 52.059048] device: 'vtcon1': device_unregister >> >> [ 52.059076] PM: Removing info for No Bus:vtcon1 >> >> [ 52.059091] device: 'vtcon1': device_create_release >> >> [ 52.059107] radeonfb :00:10.0: BAR 0: can't reserve [mem >> >> 0x9800-0x9fff pref] >> >> [ 52.256151] aper_base: 9800 MC_FB_LOC to: 9bff9800, MC_AGP_LOC >> >> to: a000 >> >> [ 52.256157] radeonfb (:00:10.0): Found 32768k of DDR 64 bits >> >> wide videoram >> >> >> >> I can confirm that offb_destroy is never called (not sure exactly >> >> why), but in any case the call to radeon_kick_out_firmware_fb happen >> >> much earlier, at least before the put_fb_info. >> > >> > It is okay, put_fb_info() is called indirectly by >> > radeon_kick_out_firmware_fb() >> > >> > radeon_kick_out_firmware_fb() >> > remove_
warning: "EDID_LENGTH" redefined (was Re: [PATCH v4] Fix loading of module radeonfb on PowerMac)
Hi Dave, Since I am not very familiar with the drm vs fbdev architecture, could you please suggest a patch for the following [*]. Reference: f453ba0460742 (Dave Airlie 2008-11-07 14:05:41 -0800 32) #define EDID_LENGTH 128 Thanks [*] On Sat, Feb 3, 2018 at 3:39 AM, kbuild test robot wrote: > Hi Mathieu, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on linus/master] > [also build test WARNING on v4.15 next-20180202] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Mathieu-Malaterre/Fix-loading-of-module-radeonfb-on-PowerMac/20180203-085907 > config: x86_64-randconfig-x009-201804 (attached as .config) > compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025 > reproduce: > # save the attached .config to linux build tree > make ARCH=x86_64 > > All warnings (new ones prefixed by >>): > >In file included from drivers/video/fbdev/aty/radeon_base.c:91:0: >>> drivers/video/fbdev/aty/../edid.h:21:0: warning: "EDID_LENGTH" redefined > #define EDID_LENGTH0x80 > >In file included from include/drm/drm_crtc.h:44:0, > from include/drm/drm_fb_helper.h:35, > from drivers/video/fbdev/aty/radeon_base.c:73: >include/drm/drm_edid.h:32:0: note: this is the location of the previous > definition > #define EDID_LENGTH 128 > >Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls64 >Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u64 >Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order >Cyclomatic Complexity 1 include/linux/string.h:strnlen >Cyclomatic Complexity 4 include/linux/string.h:strlen >Cyclomatic Complexity 6 include/linux/string.h:strlcpy >Cyclomatic Complexity 4 include/linux/string.h:memcpy >Cyclomatic Complexity 1 > arch/x86/include/asm/paravirt.h:arch_local_irq_disable >Cyclomatic Complexity 1 > arch/x86/include/asm/paravirt.h:arch_local_irq_enable >Cyclomatic Complexity 1 include/linux/spinlock.h:spinlock_check >Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock_irqrestore >Cyclomatic Complexity 1 include/linux/jiffies.h:_msecs_to_jiffies >Cyclomatic Complexity 3 include/linux/jiffies.h:msecs_to_jiffies >Cyclomatic Complexity 1 arch/x86/include/asm/io.h:readb >Cyclomatic Complexity 1 arch/x86/include/asm/io.h:readw >Cyclomatic Complexity 1 arch/x86/include/asm/io.h:readl >Cyclomatic Complexity 1 arch/x86/include/asm/io.h:writeb >Cyclomatic Complexity 1 arch/x86/include/asm/io.h:writel >Cyclomatic Complexity 1 arch/x86/include/asm/io.h:ioremap >Cyclomatic Complexity 1 include/linux/kobject.h:kobject_name >Cyclomatic Complexity 2 include/linux/device.h:dev_name >Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata >Cyclomatic Complexity 1 include/linux/device.h:dev_set_drvdata >Cyclomatic Complexity 1 include/linux/io.h:arch_phys_wc_add >Cyclomatic Complexity 1 include/linux/io.h:arch_phys_wc_del >Cyclomatic Complexity 68 include/linux/slab.h:kmalloc_large >Cyclomatic Complexity 3 include/linux/slab.h:kmalloc >Cyclomatic Complexity 1 include/linux/slab.h:kzalloc >Cyclomatic Complexity 1 include/linux/pci.h:pci_get_drvdata >Cyclomatic Complexity 1 include/linux/pci.h:pci_set_drvdata >Cyclomatic Complexity 1 include/linux/pci.h:pci_name >Cyclomatic Complexity 2 include/linux/fb.h:alloc_apertures >Cyclomatic Complexity 2 > drivers/video/fbdev/aty/radeonfb.h:radeon_pll_errata_after_index >Cyclomatic Complexity 2 > drivers/video/fbdev/aty/radeonfb.h:radeon_pll_errata_after_data >Cyclomatic Complexity 1 drivers/video/fbdev/aty/radeonfb.h:round_div >Cyclomatic Complexity 3 drivers/video/fbdev/aty/radeonfb.h:var_to_depth >Cyclomatic Complexity 5 > drivers/video/fbdev/aty/radeonfb.h:radeon_get_dstbpp >Cyclomatic Complexity 1 drivers/video/fbdev/aty/radeonfb.h:radeonfb_bl_init >Cyclomatic Complexity 1 drivers/video/fbdev/aty/radeonfb.h:radeonfb_bl_exit >Cyclomatic Complexity 1 > include/drm/drm_fb_helper.h:drm_fb_helper_remove_conflicting_framebuffers >Cyclomatic Complexity 21 > drivers/video/fbdev/aty/radeon_base.c:radeon_calc_pll_regs >Cyclomatic Complexity 1 drivers/video/fbdev/aty/radeon_base.c:radeonfb_exit >Cyclomatic Complexity 6 > drivers/video/fbdev/aty/radeon_base.c:radeon_find_mem_vbios >Cyclomatic Complexity 4 > drivers/video/fbdev/aty/radeon_base.c:radeon_kick_out_firmware_fb >Cyclomatic Complexity 5 > drivers/video/fbdev/aty/radeon_base.c:radeonfb_pci_unregister >
Re: [PATCH v2] Fixing arbitrary kernel leak in case FBIOGETCMAP_SPARC in sbusfb_ioctl_helper().
Hi Peter, On Wed, Jan 31, 2018 at 3:57 PM, Peter Malone wrote: > Fixing arbitrary kernel leak in case FBIOGETCMAP_SPARC in > sbusfb_ioctl_helper(). > > 'index' is defined as an int in sbusfb_ioctl_helper(). > We retrieve this from the user: > if (get_user(index, &c->index) || > __get_user(count, &c->count) || > __get_user(ured, &c->red) || > __get_user(ugreen, &c->green) || > __get_user(ublue, &c->blue)) >return -EFAULT; > > and then we use 'index' in the following way: > red = cmap->red[index + i] >> 8; > green = cmap->green[index + i] >> 8; > blue = cmap->blue[index + i] >> 8; > > This is a classic information leak vulnerability. 'index' should be > an unsigned int, given its usage above. > > This patch is straight-forward; it changes 'index' to unsigned int > in two switch-cases: FBIOGETCMAP_SPARC && FBIOPUTCMAP_SPARC. > > Signed-off-by: Peter Malone > --- much better :) > v2: fixed formatting > > drivers/video/fbdev/sbuslib.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/fbdev/sbuslib.c b/drivers/video/fbdev/sbuslib.c > index af6fc97f4ba4..a436d44f1b7f 100644 > --- a/drivers/video/fbdev/sbuslib.c > +++ b/drivers/video/fbdev/sbuslib.c > @@ -122,7 +122,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long > arg, > unsigned char __user *ured; > unsigned char __user *ugreen; > unsigned char __user *ublue; > - int index, count, i; > + unsigned int index, count, i; > > if (get_user(index, &c->index) || > __get_user(count, &c->count) || > @@ -161,7 +161,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long > arg, > unsigned char __user *ugreen; > unsigned char __user *ublue; > struct fb_cmap *cmap = &info->cmap; > - int index, count, i; > + unsigned int index, count, i; > u8 red, green, blue; > > if (get_user(index, &c->index) || > -- > 2.14.3 > By just looking at the code and commit message: Acked-by: Mathieu Malaterre ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] Fix loading of module radeonfb on PowerMac
Bartlomiej,
On Wed, Jan 31, 2018 at 12:57 PM, Bartlomiej Zolnierkiewicz
wrote:
> On Tuesday, January 30, 2018 02:14:10 PM Mathieu Malaterre wrote:
>> Bartlomiej,
>>
>> On Wed, Jan 3, 2018 at 3:47 PM, Bartlomiej Zolnierkiewicz
>> wrote:
>> >
>> > On Thursday, December 21, 2017 11:07:56 PM Mathieu Malaterre wrote:
>> >> When the linux kernel is build with (typical kernel ship with Debian
>> >> installer):
>> >>
>> >> CONFIG_FB_OF=y
>> >> CONFIG_VT_HW_CONSOLE_BINDING=y
>> >> CONFIG_FB_RADEON=m
>> >>
>> >> The offb driver takes precedence over module radeonfb. It is then
>> >> impossible to load the module, error reported is:
>> >>
>> >> [ 96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007)
>> >> [ 96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem
>> >> 0x9800-0x9fff pref]
>> >> [ 96.551531] radeonfb (:00:10.0): cannot request region 0.
>> >> [ 96.551545] radeonfb: probe of :00:10.0 failed with error -16
>> >>
>> >> This patch reproduce the behavior of the module radeon, so as to make it
>> >> possible to load radeonfb when offb is first loaded.
>> >>
>> >> It should be noticed that `offb_destroy` is never called which explain the
>> >> need to skip error detection on the radeon side.
>> >
>> > This still needs to be explained more, from my last mail:
>> >
>> > "The last put_fb_info() on fb_info should call ->fb_destroy
>> > (offb_destroy in our case) and remove_conflicting_framebuffers()
>> > is calling put_fb_info() so there is some extra reference on
>> > fb_info somewhere preventing it from going away.
>> >
>> > Please look into fixing this."
>>
>> I am not familiar with the fb stuff internals but here is what I see:
>>
>> # modprobe radeonfb
>>
>> leads to:
>>
>> [ 52.058546] bus: 'pci': add driver radeonfb
>> [ 52.058588] bus: 'pci': driver_probe_device: matched device
>> :00:10.0 with driver radeonfb
>> [ 52.058595] bus: 'pci': really_probe: probing driver radeonfb with
>> device :00:10.0
>> [ 52.058608] devices_kset: Moving :00:10.0 to end of list
>> [ 52.058613] radeonfb_pci_register BEGIN
>> [ 52.058634] radeonfb :00:10.0: enabling device (0006 -> 0007)
>>
>> [ 52.058666] checking generic (9c008000 96000) vs hw (9800 800)
>> [ 52.058667] fb: switching to radeonfb from OFfb ATY,RockHo
>> [ 52.058844] Console: switching to colour dummy device 80x25
>> [ 52.058860] device: 'fb0': device_unregister
>> [ 52.058956] PM: Removing info for No Bus:fb0
>> [ 52.059014] device: 'fb0': device_create_release
>>
>>
>> [ 52.059048] device: 'vtcon1': device_unregister
>> [ 52.059076] PM: Removing info for No Bus:vtcon1
>> [ 52.059091] device: 'vtcon1': device_create_release
>> [ 52.059107] radeonfb :00:10.0: BAR 0: can't reserve [mem
>> 0x9800-0x9fff pref]
>> [ 52.256151] aper_base: 9800 MC_FB_LOC to: 9bff9800, MC_AGP_LOC
>> to: a000
>> [ 52.256157] radeonfb (:00:10.0): Found 32768k of DDR 64 bits
>> wide videoram
>>
>> I can confirm that offb_destroy is never called (not sure exactly
>> why), but in any case the call to radeon_kick_out_firmware_fb happen
>> much earlier, at least before the put_fb_info.
>
> It is okay, put_fb_info() is called indirectly by
> radeon_kick_out_firmware_fb()
>
> radeon_kick_out_firmware_fb()
> remove_conflicting_framebuffers()
> do_remove_conflicting_framebuffers()
> do_unregister_framebuffer()
> put_fb_info()
>
> offb_destroy() is not called because there is an extra reference on old
> fb_info (->count == 2):
>
> static void put_fb_info(struct fb_info *fb_info)
> {
> if (!atomic_dec_and_test(&fb_info->count))
> return;
> if (fb_info->fbops->fb_destroy)
> fb_info->fbops->fb_destroy(fb_info);
> }
>
> The question is why there is an extra reference, probably user-space
> is still holding the fb_info reference obtained in fb_open() call and
> fb_release() is never called. Besides not calling fbops->fb_destroy()
> this also causes missing call of fbops->fb_release() (in fb_release())
> which some fb drivers are implementing (but
[PATCH v4] Fix loading of module radeonfb on PowerMac
When the linux kernel is build with (typical kernel ship with Debian
installer):
CONFIG_FB_OF=y
CONFIG_VT_HW_CONSOLE_BINDING=y
CONFIG_FB_RADEON=m
The offb driver takes precedence over module radeonfb. It is then
impossible to load the module, error reported is:
[ 96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007)
[ 96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem
0x9800-0x9fff pref]
[ 96.551531] radeonfb (:00:10.0): cannot request region 0.
[ 96.551545] radeonfb: probe of :00:10.0 failed with error -16
This patch reproduce the behavior of the module radeon, so as to make it
possible to load radeonfb when offb is first loaded, see commit a56f7428d753
("drm/radeon: Add early unregister of firmware fb's").
It should be noticed that `offb_destroy` is never called which explain the
need to skip error detection on the radeon side.
Signed-off-by: Mathieu Malaterre
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id=119741
Suggested-by: Lennart Sorensen
Cc: Bartlomiej Zolnierkiewicz
Cc: Benjamin Herrenschmidt
Cc: Tomi Valkeinen
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since
the conflicts in region is due to OFfb explicitly refers to it.
v4: Use drm_fb_helper_remove_conflicting_framebuffers from drm_fb_helper
drivers/video/fbdev/aty/radeon_base.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/video/fbdev/aty/radeon_base.c
b/drivers/video/fbdev/aty/radeon_base.c
index 4d77daeecf99..ae669f424537 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -70,6 +70,7 @@
#include
#include
#include
+#include
#include
#include
@@ -2259,6 +2260,23 @@ static const struct bin_attribute edid2_attr = {
.read = radeon_show_edid2,
};
+static int radeon_kick_out_firmware_fb(struct pci_dev *pdev)
+{
+ struct apertures_struct *ap;
+
+ ap = alloc_apertures(1);
+ if (!ap)
+ return -ENOMEM;
+
+ ap->ranges[0].base = pci_resource_start(pdev, 0);
+ ap->ranges[0].size = pci_resource_len(pdev, 0);
+
+ drm_fb_helper_remove_conflicting_framebuffers(ap, KBUILD_MODNAME,
false);
+
+ kfree(ap);
+
+ return 0;
+}
static int radeonfb_pci_register(struct pci_dev *pdev,
const struct pci_device_id *ent)
@@ -2312,19 +2330,27 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
rinfo->fb_base_phys = pci_resource_start (pdev, 0);
rinfo->mmio_base_phys = pci_resource_start (pdev, 2);
+ ret = radeon_kick_out_firmware_fb(pdev);
+ if (ret)
+ return ret;
+
/* request the mem regions */
ret = pci_request_region(pdev, 0, "radeonfb framebuffer");
if (ret < 0) {
+#ifndef CONFIG_FB_OF
printk( KERN_ERR "radeonfb (%s): cannot request region 0.\n",
pci_name(rinfo->pdev));
goto err_release_fb;
+#endif
}
ret = pci_request_region(pdev, 2, "radeonfb mmio");
if (ret < 0) {
+#ifndef CONFIG_FB_OF
printk( KERN_ERR "radeonfb (%s): cannot request region 2.\n",
pci_name(rinfo->pdev));
goto err_release_pci0;
+#endif
}
/* map the regions */
@@ -2509,10 +2535,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
iounmap(rinfo->mmio_base);
err_release_pci2:
pci_release_region(pdev, 2);
+#ifndef CONFIG_FB_OF
err_release_pci0:
pci_release_region(pdev, 0);
err_release_fb:
framebuffer_release(info);
+#endif
err_disable:
err_out:
return ret;
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] Fix loading of module radeonfb on PowerMac
Bartlomiej,
On Wed, Jan 3, 2018 at 3:47 PM, Bartlomiej Zolnierkiewicz
wrote:
>
> On Thursday, December 21, 2017 11:07:56 PM Mathieu Malaterre wrote:
>> When the linux kernel is build with (typical kernel ship with Debian
>> installer):
>>
>> CONFIG_FB_OF=y
>> CONFIG_VT_HW_CONSOLE_BINDING=y
>> CONFIG_FB_RADEON=m
>>
>> The offb driver takes precedence over module radeonfb. It is then
>> impossible to load the module, error reported is:
>>
>> [ 96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007)
>> [ 96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem
>> 0x9800-0x9fff pref]
>> [ 96.551531] radeonfb (:00:10.0): cannot request region 0.
>> [ 96.551545] radeonfb: probe of :00:10.0 failed with error -16
>>
>> This patch reproduce the behavior of the module radeon, so as to make it
>> possible to load radeonfb when offb is first loaded.
>>
>> It should be noticed that `offb_destroy` is never called which explain the
>> need to skip error detection on the radeon side.
>
> This still needs to be explained more, from my last mail:
>
> "The last put_fb_info() on fb_info should call ->fb_destroy
> (offb_destroy in our case) and remove_conflicting_framebuffers()
> is calling put_fb_info() so there is some extra reference on
> fb_info somewhere preventing it from going away.
>
> Please look into fixing this."
I am not familiar with the fb stuff internals but here is what I see:
# modprobe radeonfb
leads to:
[ 52.058546] bus: 'pci': add driver radeonfb
[ 52.058588] bus: 'pci': driver_probe_device: matched device
:00:10.0 with driver radeonfb
[ 52.058595] bus: 'pci': really_probe: probing driver radeonfb with
device :00:10.0
[ 52.058608] devices_kset: Moving :00:10.0 to end of list
[ 52.058613] radeonfb_pci_register BEGIN
[ 52.058634] radeonfb :00:10.0: enabling device (0006 -> 0007)
[ 52.058666] checking generic (9c008000 96000) vs hw (9800 800)
[ 52.058667] fb: switching to radeonfb from OFfb ATY,RockHo
[ 52.058844] Console: switching to colour dummy device 80x25
[ 52.058860] device: 'fb0': device_unregister
[ 52.058956] PM: Removing info for No Bus:fb0
[ 52.059014] device: 'fb0': device_create_release
[ 52.059048] device: 'vtcon1': device_unregister
[ 52.059076] PM: Removing info for No Bus:vtcon1
[ 52.059091] device: 'vtcon1': device_create_release
[ 52.059107] radeonfb :00:10.0: BAR 0: can't reserve [mem
0x9800-0x9fff pref]
[ 52.256151] aper_base: 9800 MC_FB_LOC to: 9bff9800, MC_AGP_LOC
to: a000
[ 52.256157] radeonfb (:00:10.0): Found 32768k of DDR 64 bits
wide videoram
I can confirm that offb_destroy is never called (not sure exactly
why), but in any case the call to radeon_kick_out_firmware_fb happen
much earlier, at least before the put_fb_info.
Could you describe a bit more the chain of calls you were thinking of ?
>> Signed-off-by: Mathieu Malaterre
>> Link: https://bugs.debian.org/826629#57
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=119741
>> Suggested-by: Lennart Sorensen
>> ---
>> v2: Only fails when CONFIG_PCC is not set
>> v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since
>> the conflicts in region is due to OFfb explicitly refers to it.
>
> It seems that there may still be configurations when this is
> incorrect -> when offb drives primary (non-radeon) card and radeonfb
> drives secondary (radeon) card..
>
>> drivers/video/fbdev/aty/radeon_base.c | 26 ++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/video/fbdev/aty/radeon_base.c
>> b/drivers/video/fbdev/aty/radeon_base.c
>> index 4d77daeecf99..221879196531 100644
>> --- a/drivers/video/fbdev/aty/radeon_base.c
>> +++ b/drivers/video/fbdev/aty/radeon_base.c
>> @@ -2259,6 +2259,22 @@ static const struct bin_attribute edid2_attr = {
>> .read = radeon_show_edid2,
>> };
>>
>> +static int radeon_kick_out_firmware_fb(struct pci_dev *pdev)
>> +{
>> + struct apertures_struct *ap;
>> +
>> + ap = alloc_apertures(1);
>> + if (!ap)
>> + return -ENOMEM;
>> +
>> + ap->ranges[0].base = pci_resource_start(pdev, 0);
>> + ap->ranges[0].size = pci_resource_len(pdev, 0);
>> +
>> + remove_conflicting_framebuffers(ap, KBUILD_MODNAME, false);
>> + kfree(ap);
>> +
>> + return 0;
>> +}
>>
>> static int radeonfb_pci_register(struct pci_dev *pdev,
>>const struct pci_devi
Re: [PATCH v3] Fix loading of module radeonfb on PowerMac
Hi Bartlomiej, On Wed, Jan 3, 2018 at 4:23 PM, Lennart Sorensen wrote: > On Wed, Jan 03, 2018 at 03:47:35PM +0100, Bartlomiej Zolnierkiewicz wrote: >> On Thursday, December 21, 2017 11:07:56 PM Mathieu Malaterre wrote: >> > When the linux kernel is build with (typical kernel ship with Debian >> > installer): >> > >> > CONFIG_FB_OF=y >> > CONFIG_VT_HW_CONSOLE_BINDING=y >> > CONFIG_FB_RADEON=m >> > >> > The offb driver takes precedence over module radeonfb. It is then >> > impossible to load the module, error reported is: >> > >> > [ 96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007) >> > [ 96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem >> > 0x9800-0x9fff pref] >> > [ 96.551531] radeonfb (:00:10.0): cannot request region 0. >> > [ 96.551545] radeonfb: probe of :00:10.0 failed with error -16 >> > >> > This patch reproduce the behavior of the module radeon, so as to make it >> > possible to load radeonfb when offb is first loaded. >> > >> > It should be noticed that `offb_destroy` is never called which explain the >> > need to skip error detection on the radeon side. >> >> This still needs to be explained more, from my last mail: >> >> "The last put_fb_info() on fb_info should call ->fb_destroy >> (offb_destroy in our case) and remove_conflicting_framebuffers() >> is calling put_fb_info() so there is some extra reference on >> fb_info somewhere preventing it from going away. >> >> Please look into fixing this." > > My impression of that problem is that the offb driver is in use because > it is running the console, and until the radeonfb driver is loaded and > ready to take over, you can't stop using the offb driver, but you can't > currently load the radeonfb because offb is holding resources it wants. > > Maybe I have misunderstood what the code is doing, but that seems to be > the problem. > > On an x86 PC you could drop one fb and go to text mode then start another > fb driver. PPC doesn't have that option since there is no text mode. For reference my patch was inspired by commit: https://github.com/torvalds/linux/commit/a56f7428d7534f162fbb089c5c79012bf38a7c29 While doing the search, I discover my patch is incorrect, I need to integrate change from: https://github.com/torvalds/linux/commit/44adece57e2604cec8527a499b48e4d584ab53b8#diff-767fb253c0135686111755f394d64199 So I'll submit a v4 anyway. Thanks all, ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] Fix loading of module radeonfb on PowerMac
When the linux kernel is build with (typical kernel ship with Debian
installer):
CONFIG_FB_OF=y
CONFIG_VT_HW_CONSOLE_BINDING=y
CONFIG_FB_RADEON=m
The offb driver takes precedence over module radeonfb. It is then
impossible to load the module, error reported is:
[ 96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007)
[ 96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem
0x9800-0x9fff pref]
[ 96.551531] radeonfb (:00:10.0): cannot request region 0.
[ 96.551545] radeonfb: probe of :00:10.0 failed with error -16
This patch reproduce the behavior of the module radeon, so as to make it
possible to load radeonfb when offb is first loaded.
It should be noticed that `offb_destroy` is never called which explain the
need to skip error detection on the radeon side.
Signed-off-by: Mathieu Malaterre
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id=119741
Suggested-by: Lennart Sorensen
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since
the conflicts in region is due to OFfb explicitly refers to it.
drivers/video/fbdev/aty/radeon_base.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/video/fbdev/aty/radeon_base.c
b/drivers/video/fbdev/aty/radeon_base.c
index 4d77daeecf99..221879196531 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -2259,6 +2259,22 @@ static const struct bin_attribute edid2_attr = {
.read = radeon_show_edid2,
};
+static int radeon_kick_out_firmware_fb(struct pci_dev *pdev)
+{
+ struct apertures_struct *ap;
+
+ ap = alloc_apertures(1);
+ if (!ap)
+ return -ENOMEM;
+
+ ap->ranges[0].base = pci_resource_start(pdev, 0);
+ ap->ranges[0].size = pci_resource_len(pdev, 0);
+
+ remove_conflicting_framebuffers(ap, KBUILD_MODNAME, false);
+ kfree(ap);
+
+ return 0;
+}
static int radeonfb_pci_register(struct pci_dev *pdev,
const struct pci_device_id *ent)
@@ -2312,19 +2328,27 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
rinfo->fb_base_phys = pci_resource_start (pdev, 0);
rinfo->mmio_base_phys = pci_resource_start (pdev, 2);
+ ret = radeon_kick_out_firmware_fb(pdev);
+ if (ret)
+ return ret;
+
/* request the mem regions */
ret = pci_request_region(pdev, 0, "radeonfb framebuffer");
if (ret < 0) {
+#ifndef CONFIG_FB_OF
printk( KERN_ERR "radeonfb (%s): cannot request region 0.\n",
pci_name(rinfo->pdev));
goto err_release_fb;
+#endif
}
ret = pci_request_region(pdev, 2, "radeonfb mmio");
if (ret < 0) {
+#ifndef CONFIG_FB_OF
printk( KERN_ERR "radeonfb (%s): cannot request region 2.\n",
pci_name(rinfo->pdev));
goto err_release_pci0;
+#endif
}
/* map the regions */
@@ -2509,10 +2533,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
iounmap(rinfo->mmio_base);
err_release_pci2:
pci_release_region(pdev, 2);
+#ifndef CONFIG_FB_OF
err_release_pci0:
pci_release_region(pdev, 0);
err_release_fb:
framebuffer_release(info);
+#endif
err_disable:
err_out:
return ret;
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH radeon] Change the default to PCI on PowerPC
Is there a chance this patch could be considered for inclusion ? It has been reviewed and it is well known that PCI generally work best by default. It will solve a long standing issue when user experience Debian on a Powermac and quickly get a non-working (freezing) X session. Thanks again for consideration. 2016-05-13 8:29 GMT+02:00 Mathieu Malaterre : > AGP mode is unstable on PowerPC > > Signed-off-by: Mathieu Malaterre > Reviewed-by: Christian König > --- > drivers/gpu/drm/radeon/radeon_drv.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c > b/drivers/gpu/drm/radeon/radeon_drv.c > index ccd4ad4..402cf85 100644 > --- a/drivers/gpu/drm/radeon/radeon_drv.c > +++ b/drivers/gpu/drm/radeon/radeon_drv.c > @@ -170,7 +170,12 @@ int radeon_no_wb; > int radeon_modeset = -1; > int radeon_dynclks = -1; > int radeon_r4xx_atom = 0; > +#ifdef __powerpc__ > +/* Default to PCI on PowerPC (fdo #95017) */ > +int radeon_agpmode = -1; > +#else > int radeon_agpmode = 0; > +#endif > int radeon_vram_limit = 0; > int radeon_gart_size = -1; /* auto */ > int radeon_benchmarking = 0; > -- > 2.1.4 > -- Mathieu
[PATCH radeon] Change the default to PCI on PowerPC
ping ? 2016-05-13 8:29 GMT+02:00 Mathieu Malaterre : > AGP mode is unstable on PowerPC > > Signed-off-by: Mathieu Malaterre > Reviewed-by: Christian König > --- > drivers/gpu/drm/radeon/radeon_drv.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c > b/drivers/gpu/drm/radeon/radeon_drv.c > index ccd4ad4..402cf85 100644 > --- a/drivers/gpu/drm/radeon/radeon_drv.c > +++ b/drivers/gpu/drm/radeon/radeon_drv.c > @@ -170,7 +170,12 @@ int radeon_no_wb; > int radeon_modeset = -1; > int radeon_dynclks = -1; > int radeon_r4xx_atom = 0; > +#ifdef __powerpc__ > +/* Default to PCI on PowerPC (fdo #95017) */ > +int radeon_agpmode = -1; > +#else > int radeon_agpmode = 0; > +#endif > int radeon_vram_limit = 0; > int radeon_gart_size = -1; /* auto */ > int radeon_benchmarking = 0; > -- > 2.1.4 > -- Mathieu
[Mesa-dev] radeonfb: BAR 0: can't reserve [mem 0x98000000-0x9fffffff pref]
On Tue, Jun 7, 2016 at 8:05 AM, Mathieu Malaterre wrote: > Hi Alex, > > On Mon, Jun 6, 2016 at 7:20 PM, Alex Deucher wrote: >> On Mon, Jun 6, 2016 at 1:16 PM, Marek Olšák wrote: >>> [+ dri-devel] >>> >>> On Mon, Jun 6, 2016 at 8:42 AM, Mathieu Malaterre >>> wrote: >>>> Hi, >>>> >>>> Before reporting a possible invalid bug report. Does anyone knows why >>>> radeaonfb is not configured the same way radeon is ? For instance on a >>>> PowerPC machine, when Open Firmware Frame Buffer is used (OFfb), I >>>> cannot `modprobe radeonfb` (but I can load `radeon`). It fails with: >>>> >>>> [ 96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007) >>>> [ 96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem >>>> 0x9800-0x9fff pref] >>>> [ 96.551531] radeonfb (:00:10.0): cannot request region 0. >>>> [ 96.551545] radeonfb: probe of :00:10.0 failed with error -16 >>>> >>>> It seems (to me) that it should be possible to add something like this >>>> to `radeonfb`: >>>> >>>> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/radeon/radeon_drv.c#L353 >>>> >>>> Is the above correct ? >> >> Yes, probably. But is there any reason why you'd want to use radeonfb >> rather than radeon? > > I'll check with the debian-installer team if this is possible. Right > now the debian-installer is setup to use `radeonfb` (at least on > PowerPC) during the text-based installation. > > ref: https://bugs.debian.org/825840#77 [between a rock and a hard place] So it seems there would be a risk moving from radeonfb to radeon, esp. since Debian does not distribute (at least by default) the firmware during installation: https://bugs.debian.org/826629#12. I'll report the bug against radeonfb and see if I can provide a patch. -M
[Mesa-dev] radeonfb: BAR 0: can't reserve [mem 0x98000000-0x9fffffff pref]
Hi Alex, On Mon, Jun 6, 2016 at 7:20 PM, Alex Deucher wrote: > On Mon, Jun 6, 2016 at 1:16 PM, Marek Olšák wrote: >> [+ dri-devel] >> >> On Mon, Jun 6, 2016 at 8:42 AM, Mathieu Malaterre >> wrote: >>> Hi, >>> >>> Before reporting a possible invalid bug report. Does anyone knows why >>> radeaonfb is not configured the same way radeon is ? For instance on a >>> PowerPC machine, when Open Firmware Frame Buffer is used (OFfb), I >>> cannot `modprobe radeonfb` (but I can load `radeon`). It fails with: >>> >>> [ 96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007) >>> [ 96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem >>> 0x9800-0x9fff pref] >>> [ 96.551531] radeonfb (:00:10.0): cannot request region 0. >>> [ 96.551545] radeonfb: probe of :00:10.0 failed with error -16 >>> >>> It seems (to me) that it should be possible to add something like this >>> to `radeonfb`: >>> >>> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/radeon/radeon_drv.c#L353 >>> >>> Is the above correct ? > > Yes, probably. But is there any reason why you'd want to use radeonfb > rather than radeon? I'll check with the debian-installer team if this is possible. Right now the debian-installer is setup to use `radeonfb` (at least on PowerPC) during the text-based installation. ref: https://bugs.debian.org/825840#77 -M
PCI on PowerPC
Dear all, I've tested a patch against radeon_drc.c so that the default mode is now PCI on PowerPC arch. This is the result of the discussion on the following bug report: https://bugs.freedesktop.org/show_bug.cgi?id=95017 Thanks for your comments, -- Mathieu
[PATCH radeon] Change the default to PCI on PowerPC
AGP mode is unstable on PowerPC Signed-off-by: Mathieu Malaterre Reviewed-by: Christian König --- drivers/gpu/drm/radeon/radeon_drv.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index ccd4ad4..402cf85 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -170,7 +170,12 @@ int radeon_no_wb; int radeon_modeset = -1; int radeon_dynclks = -1; int radeon_r4xx_atom = 0; +#ifdef __powerpc__ +/* Default to PCI on PowerPC (fdo #95017) */ +int radeon_agpmode = -1; +#else int radeon_agpmode = 0; +#endif int radeon_vram_limit = 0; int radeon_gart_size = -1; /* auto */ int radeon_benchmarking = 0; -- 2.1.4
