Re: [PATCH 0/7] staging: vc04_services: Some dead code removal
Stefan Wahren writes: > Hi Tuomas, > >> Tuomas Tynkkynen hat am 4. Oktober 2018 um 11:37 >> geschrieben: >> >> >> Drop various pieces of dead code from here and there to get rid of >> the remaining users of VCHI_CONNECTION_T. After that we get to drop >> entire header files worth of unused code. >> >> I've tested on a Raspberry Pi Model B (bcm2835_defconfig) that >> snd-bcm2835 can still play analog audio just fine. >> > > thanks and i'm fine with your patch series: > > Acked-by: Stefan Wahren > > Unfortunately this would break compilation of the downstream vchi > drivers like vcsm [1]. Personally i don't want to maintain another > one, because i cannot see the gain of the resulting effort. > > [1] - > https://github.com/raspberrypi/linux/tree/rpi-4.14.y/drivers/char/broadcom/vc_sm I think the main concern would be if we removed things necessary for 6by9's new vcsm (the one that will let us do dma-buf sharing between media decode and DRM). On the other hand, git revert is a thing, so it's not like we actually lose anything. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/7] staging: vc04_services: Some dead code removal
Dan Carpenter writes: > On Sat, Oct 06, 2018 at 12:18:38PM +0200, Stefan Wahren wrote: >> Hi Tuomas, >> >> > Tuomas Tynkkynen hat am 4. Oktober 2018 um 11:37 >> > geschrieben: >> > >> > >> > Drop various pieces of dead code from here and there to get rid of >> > the remaining users of VCHI_CONNECTION_T. After that we get to drop >> > entire header files worth of unused code. >> > >> > I've tested on a Raspberry Pi Model B (bcm2835_defconfig) that >> > snd-bcm2835 can still play analog audio just fine. >> > >> >> thanks and i'm fine with your patch series: >> >> Acked-by: Stefan Wahren >> >> Unfortunately this would break compilation of the downstream vchi >> drivers like vcsm [1]. Personally i don't want to maintain another >> one, because i cannot see the gain of the resulting effort. >> >> [1] - >> https://github.com/raspberrypi/linux/tree/rpi-4.14.y/drivers/char/broadcom/vc_sm > > > I feel like everyone else already knows the answer but why don't we just > merge that code into staging? Dave's been working on a new VCSM service where the firmware can call back into Linux to allocate (instead of just having a permanent carveout of system memory that the firmware allocates from), and lets us make dma-bufs out of those buffers. That driver makes a no-copies v4l2 media decode driver possible, which would then let Kodi and similar projects switch from downstream kernels with closed graphics to upstream kernels with open graphics. Given that the new VCSM service is a rewrite, it's not clear to me that importing the old VCSM driver is a win. But maybe we should go raid https://github.com/6by9/linux/tree/rpi-4.14.y-codecs-push-pt2a and grab the new drivers. Upstreaming the VCHI audio driver to staging has clearly been a win for it, so maybe other eyes on the new v4l2 codec could help Dave along in stabilizing it. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2] staging: vchiq_2835_arm: Make cache-line-size a required DT property
Stefan Wahren writes:
> [add Eric]
>
>> Michael Zoran hat am 18. Februar 2017 um 12:59
>> geschrieben:
>>
>>
>> On Sat, 2017-02-18 at 03:22 -0800, Michael Zoran wrote:
>> > The original github source allowed for the cache-line-size property
>> > to be missing. Since recent firmwares also require this property,
>> > it makes sense to always require it in the driver as well.
>> >
>> > If the cache-line-size property is missing, then the driver probe
>> > should fail as no dev since the kernel and dt may be out of sync.
>> > The fix is to add a check for the return value of
>> > of_property_read_u32.
>> >
>> > Changes V2:
>> >1. Add error message if cache-line-size is missing.
>> >2. Simple check for non-zero return value from
>> > of_property_read_u32.
>> >
>> > Signed-off-by: Michael Zoran
>> > ---
>> > .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c|
>> > 8 +++-
>> > 1 file changed, 7 insertions(+), 1 deletion(-)
>> >
>> > diff --git
>> > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
>> > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
>> > index e6241fb5cfa6..3aeffcb9c87e 100644
>> > ---
>> > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
>> > +++
>> > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
>> > @@ -121,8 +121,14 @@ int vchiq_platform_init(struct platform_device
>> > *pdev, VCHIQ_STATE_T *state)
>> >if (err < 0)
>> >return err;
>> >
>> > - (void)of_property_read_u32(dev->of_node, "cache-line-size",
>> > + err = of_property_read_u32(dev->of_node, "cache-line-size",
>> > &g_cache_line_size);
>> > +
>> > + if (err) {
>> > + dev_err(dev, "Missing cache-line-size property\n");
>> > + return -ENODEV;
>> > + }
>> > +
>> >g_fragments_size = 2 * g_cache_line_size;
>> >
>> >/* Allocate space for the channels in coherent memory */
>>
>> If anybody is willing to add an Reviewed-by, Acked, or even tested-by
>> to this it would be great. I know gregk is a busy guy so it might make
>> sense to make a note that someone else looked at this and that I'm not
>> out in left field.
>>
>> It would be really great if this made it into 4.11. I'm a bit
>> concerned about someone accidently trying to mix this driver with an
>> ancient DT. Things would probably sort of work if that was done, but it
>> would result in difficult to debug errors.
>
> I think Greg is waiting for Eric's Ack, but it seems you didn't send him a
> copy.
>
> @Eric: Are you okay with this patch?
For staging, Greg has been taking patches without platform maintainer
ack. I think this is great -- the staging code needs *lots* of work,
and it generally doesn't need any platform knowledge.
As far as this patch, I don't know why we wouldn't just use
cache_line_size() instead of asking DT.
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RESEND PATCH V4] staging: vchiq_arm: Add compatibility wrappers for ioctls
32)
> +
> +static long
> +vchiq_compat_ioctl_await_completion(struct file *file,
> + unsigned int cmd,
> + unsigned long arg)
> +{
> + VCHIQ_AWAIT_COMPLETION_T *args;
> + VCHIQ_COMPLETION_DATA_T *completion;
> + VCHIQ_COMPLETION_DATA_T completiontemp;
> + struct vchiq_await_completion32 args32;
> + struct vchiq_completion_data32 completion32;
> + unsigned int *msgbufcount32;
> + compat_uptr_t msgbuf32;
> + void *msgbuf;
> + void **msgbufptr;
> + long ret;
> +
> + args = compat_alloc_user_space(sizeof(*args) +
> +sizeof(*completion) +
> +sizeof(*msgbufptr));
> + if (!args)
> + return -EFAULT;
> +
> + completion = (VCHIQ_COMPLETION_DATA_T *)(args + 1);
> + msgbufptr = (void __user **)(completion + 1);
> +
> + if (copy_from_user(&args32,
> +(struct vchiq_completion_data32 *)arg,
> +sizeof(args32)))
> + return -EFAULT;
> +
> + if (put_user(args32.count, &args->count) ||
> + put_user(compat_ptr(args32.buf), &args->buf) ||
> + put_user(args32.msgbufsize, &args->msgbufsize) ||
> + put_user(args32.msgbufcount, &args->msgbufcount) ||
> + put_user(compat_ptr(args32.msgbufs), &args->msgbufs))
> + return -EFAULT;
> +
> + if (!args32.count || !args32.buf || !args32.msgbufcount)
> + return vchiq_ioctl(file,
> +VCHIQ_IOC_AWAIT_COMPLETION,
> +(unsigned long)args);
> +
> + if (copy_from_user(&msgbuf32,
> +compat_ptr(args32.msgbufs) +
> +(sizeof(compat_uptr_t) *
> +(args32.msgbufcount - 1)),
> +sizeof(msgbuf32)))
> + return -EFAULT;
> +
> + msgbuf = compat_ptr(msgbuf32);
> +
> + if (copy_to_user(msgbufptr,
> + &msgbuf,
> + sizeof(msgbuf)))
> + return -EFAULT;
> +
> + if (copy_to_user(&args->msgbufs,
> + &msgbufptr,
> + sizeof(msgbufptr)))
> + return -EFAULT;
> +
> + if (put_user(1U, &args->count) ||
> + put_user(completion, &args->buf) ||
> + put_user(1U, &args->msgbufcount))
> + return -EFAULT;
Seems like instead of treating the user ioctl as having specified a
count of 1 / msgbufcount of 1, we should throw -EINVAL if they specified
something other than what we support for the compat path.
(this also means we don't need the weird bumping of msgbuf by
msgbufcount - 1 in the copy_from_user above, right?)
> +
> + ret = vchiq_ioctl(file,
> + VCHIQ_IOC_AWAIT_COMPLETION,
> + (unsigned long)args);
> +
> + if (ret <= 0)
> + return ret;
> +
> + if (copy_from_user(&completiontemp, completion, sizeof(*completion)))
> + return -EFAULT;
> +
> + completion32.reason = completiontemp.reason;
> + completion32.header = ptr_to_compat(completiontemp.header);
> + completion32.service_userdata =
> + ptr_to_compat(completiontemp.service_userdata);
> + completion32.bulk_userdata =
> + ptr_to_compat(completiontemp.bulk_userdata);
> +
> + if (copy_to_user(compat_ptr(args32.buf),
> + &completion32,
> + sizeof(completion32)))
> + return -EFAULT;
> +
> + args32.msgbufcount--;
> +
> + msgbufcount32 =
> + &((struct vchiq_await_completion32 __user *)arg)->msgbufcount;
There seem to be conditions in the real ioctl where msgbufcount doesn't
get decremented. Could we just get_user() the args->msgbufcount and
copy that back out?
With these 3 fixes,
Reviewed-by: Eric Anholt
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
Mauro Carvalho Chehab writes:
> Em Fri, 27 Jan 2017 13:54:57 -0800
> Eric Anholt escreveu:
>
>> Here's my first pass at importing the camera driver. There's a bunch
>> of TODO left to it, most of which is documented, and the rest being
>> standard checkpatch fare.
>>
>> Unfortunately, when I try modprobing it on my pi3, the USB network
>> device dies, consistently. I'm not sure what's going on here yet, but
>> I'm going to keep working on some debug of it. I've unfortunately
>> changed a lot of variables (pi3 vs pi2, upstream vs downstream, vchi's
>> updates while in staging, 4.9 vs 4.4), so I probably won't figure it
>> out today.
>>
>> Note that the "Update the driver to the current VCHI API" patch will
>> conflict with the outstanding "Add vchi_queue_kernel_message and
>> vchi_queue_user_message" series, but the fix should be pretty obvious
>> when that lands.
>>
>> I built this against 4.10-rc1, but a merge with staging-next was clean
>> and still built fine.
>
> I'm trying it, building from the linux-next branch of the staging
> tree. No joy.
>
> That's what happens when I modprobe it:
>
> [ 991.841549] bcm2835_v4l2: module is from the staging directory, the
> quality is unknown, you have been warned.
> [ 991.842931] vchiq_get_state: g_state.remote == NULL
> [ 991.843437] vchiq_get_state: g_state.remote == NULL
> [ 991.843940] vchiq_get_state: g_state.remote == NULL
> [ 991.84] vchiq_get_state: g_state.remote == NULL
> [ 991.844947] vchiq_get_state: g_state.remote == NULL
> [ 991.845451] vchiq_get_state: g_state.remote == NULL
> [ 991.845954] vchiq_get_state: g_state.remote == NULL
> [ 991.846457] vchiq_get_state: g_state.remote == NULL
> [ 991.846961] vchiq_get_state: g_state.remote == NULL
> [ 991.847464] vchiq_get_state: g_state.remote == NULL
> [ 991.847969] vchiq: vchiq_initialise: videocore not initialized
>
> [ 991.847973] mmal_vchiq: Failed to initialise VCHI instance (status=-1)
Yeah, this failure mode sucks. I'm guessing you don't have a VCHI node
in the DT? Patch attached.
I haven't followed up on getting the DT documented so that it can be
merged, and it sounds like Michael has some plans for changing how VCHI
and VCHI's consumers get attached to each other so that it's not
DT-based anyway.
From 9488974b836b1fba7d32af34d612151872f9ce0d Mon Sep 17 00:00:00 2001
From: Eric Anholt
Date: Mon, 3 Oct 2016 11:23:34 -0700
Subject: [PATCH] ARM: bcm2835: Add VCHIQ to the DT.
Signed-off-by: Eric Anholt
---
arch/arm/boot/dts/bcm2835-rpi.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi
b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index caf2707680c1..f5fb5c5aa07a 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -26,6 +26,14 @@
firmware = <&firmware>;
#power-domain-cells = <1>;
};
+
+ vchiq {
+ compatible = "brcm,bcm2835-vchiq";
+ reg = <0x7e00b840 0xf>;
+ interrupts = <0 2>;
+ cache-line-size = <32>;
+ firmware = <&firmware>;
+ };
};
};
--
2.11.0
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] bcm2835-v4l2: Fix buffer overflow problem
Greg KH writes: > On Wed, Mar 15, 2017 at 03:32:56PM +, Dave Stevenson wrote: >> You've got a reason. It's GPLv2 licenced code so I have no control >> over what happens to it. >> Everywhere I have worked, when a patch has issues it is better to "-1" >> to stop/delay the merge even (or particularly) on your own patches, >> but this is your playground so your rules. > > It's fine to say "don't apply, for _THIS REASON_". You didn't specify a > reason, which is why I complained. > >> Anyway, I'll go back to working with the downstream tree (pull request >> for the full fix there) and stop bothering you. > > Ah, fun, we will diverge even more in the future, not good. Any reason > why I shouldn't just delete this whole in-kernel tree if you are going > to spend time maintaining a different version? > > If you want to maintain this driver, in the kernel tree, by all means I > would love the help as I don't have hardware or the ability to test > anything. Having two different trees for the source guarantees that > there are going to be constant problems here, and that's not good for > anyone. If Dave doesn't do the upstreaming work for his future work, I'll be forced to. The rpi tree still hasn't diverged, yet (other than this patch and the related one being discussed). signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
Mauro Carvalho Chehab writes: > Em Wed, 15 Mar 2017 18:46:24 -0700 > Michael Zoran escreveu: > >> On Wed, 2017-03-15 at 22:08 -0300, Mauro Carvalho Chehab wrote: >> >> > No, I didn't. Thanks! Applied it but, unfortunately, didn't work. >> > Perhaps I'm missing some other patch. I'm compiling it from >> > the Greg's staging tree (branch staging-next): >> >https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging. >> > git/log/?h=staging-next >> > >> > Btw, as I'm running Raspbian, and didn't want to use compat32 bits, >> > I'm compiling the Kernel as an arm32 bits Kernel. >> > >> > I did a small trick to build the DTB on arm32: >> > >> >ln -sf ../../../arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts >> > arch/arm/boot/dts/bcm2837-rpi-3-b.dts >> >ln -sf ../../../arm64/boot/dts/broadcom/bcm2837.dtsi >> > arch/arm/boot/dts/bcm2837.dtsi >> >git checkout arch/arm/boot/dts/Makefile >> >sed "s,bcm2835-rpi-zero.dtb,bcm2835-rpi-zero.dtb bcm2837-rpi-3- >> > b.dtb," a && mv a arch/arm/boot/dts/Makefile >> > >> >> Two other hacks are currently needed to get the camera to work: >> >> 1. Add this to config.txt(This required to get the firmware to detect >> the camera) >> >> start_x=1 >> gpu_mem=128 > > I had this already. > >> >> 2. VC4 is incompatible with the firmware at this time, so you need >> to presently munge the build configuration. What you do is leave >> simplefb in the build config(I'm assuming you already have that), but >> you will need to remove VC4 from the config. >> >> The firmware currently adds a node for a simplefb for debugging >> purposes to show the boot log. Surprisingly, this is still good enough >> for basic usage and testing. > > That solved the issue. Thanks! It would be good to add a notice > about that at the TODO, not let it build if DRM_VC4. > > Please consider applying the enclosed path. The VC4 incompatibility (camera firmware's AWB ends up briefly using the GPU, without coordinating with the Linux driver) is supposed to be fixed in current firmware (https://github.com/raspberrypi/firmware/issues/760#issuecomment-287391025) signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service
Michael Zoran writes: >> > Since the API is completely documented, I see no reason we or >> > anybody >> > couldn't essentially rewrite the driver while it's in staging. I >> > just >> > think it would be best for everyone if the new version was a drop >> > in >> > replacement for the original version. Essential an enhancement >> > rather >> > then a competitor. >> >> I think my comments weren't fundamental changes, but you surely mean >> the devicetree ABI? I like to see this driver ASAP out of staging and >> i'm not interested to maintain 2 functional identical driver only to >> keep compability with the Foundation tree. Currently i'm afraid that >> we build up many drivers in staging, which need a complete rewrite >> later if they should come out of staging. It would be nice if we >> could avoid the situation we have with the thermal driver. >> >> Stefan > > The API I'm talking about here is the mailbox API that is used to talk > to the firmware. The numbers and structures to pass are documented. > Nothing prevents anybody from rewriting this driver and submitting it > to the appropriate subsystems. It's certainly small enough. > > If you really want working thermal or cpu speed drivers today, nothing > stops anybody from submitting the downstream drivers after doing some > minor touchups and submitting them to staging. That would at least get > things working while people argue about what the correct DT nodes > should be. > > I would also like to point out that the RPI 3 has been out for over a > year and nobody has been able to get working video out of it through > VC4 on a mainline tree. At least until now. So I'm not sure the best > way to go is for the expander driver to go under the GPIO subtree. Excuse me? Display works fine on my Pi3. VC4 uses DDC to probe for connection when the GPIO line isn't present in the DT. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Eric Anholt offically announces support of VC4 without access to expander on the RPI 3
Michael Zoran writes: > On Mon, 2017-03-20 at 10:22 -0700, Eric Anholt wrote: >> Michael Zoran writes: >> >> > > > Since the API is completely documented, I see no reason we or >> > > > anybody >> > > > couldn't essentially rewrite the driver while it's in >> > > > staging. I >> > > > just >> > > > think it would be best for everyone if the new version was a >> > > > drop >> > > > in >> > > > replacement for the original version. Essential an enhancement >> > > > rather >> > > > then a competitor. >> > > >> > > I think my comments weren't fundamental changes, but you surely >> > > mean >> > > the devicetree ABI? I like to see this driver ASAP out of staging >> > > and >> > > i'm not interested to maintain 2 functional identical driver only >> > > to >> > > keep compability with the Foundation tree. Currently i'm afraid >> > > that >> > > we build up many drivers in staging, which need a complete >> > > rewrite >> > > later if they should come out of staging. It would be nice if we >> > > could avoid the situation we have with the thermal driver. >> > > >> > > Stefan >> > >> > The API I'm talking about here is the mailbox API that is used to >> > talk >> > to the firmware. The numbers and structures to pass are >> > documented. >> > Nothing prevents anybody from rewriting this driver and submitting >> > it >> > to the appropriate subsystems. It's certainly small enough. >> > >> > If you really want working thermal or cpu speed drivers today, >> > nothing >> > stops anybody from submitting the downstream drivers after doing >> > some >> > minor touchups and submitting them to staging. That would at least >> > get >> > things working while people argue about what the correct DT nodes >> > should be. >> > >> > I would also like to point out that the RPI 3 has been out for over >> > a >> > year and nobody has been able to get working video out of it >> > through >> > VC4 on a mainline tree. At least until now. So I'm not sure the >> > best >> > way to go is for the expander driver to go under the GPIO subtree. >> >> Excuse me? Display works fine on my Pi3. VC4 uses DDC to probe for >> connection when the GPIO line isn't present in the DT. > > Just a FYI, Eric Anholt has offically announced support for VC4 for > HDMI on mainline Linus build without any support from the expander on > the RPI 3. > > Sounds like this particular driver isn't needed then, correct? That's the HDMI audio that just landed. HDMI has been working on the pi3 since 9d44a8d530e8cc97d71ffcbc0ff3b5553c62. In the absence of a GPIO line for hotplug detect, we use DDC, which is slower and throws an error in dmesg when the probe happens but HDMI is disconnected. As such, having a GPIO driver would improve things for people. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Eric Anholt offically announces support of VC4 without access to expander on the RPI 3
Michael Zoran writes: > On Tue, 2017-03-21 at 10:34 -0700, Eric Anholt wrote: >> Michael Zoran writes: >> >> > On Mon, 2017-03-20 at 10:22 -0700, Eric Anholt wrote: >> > > Michael Zoran writes: >> > > >> > > > > > Since the API is completely documented, I see no reason we >> > > > > > or >> > > > > > anybody >> > > > > > couldn't essentially rewrite the driver while it's in >> > > > > > staging. I >> > > > > > just >> > > > > > think it would be best for everyone if the new version was >> > > > > > a >> > > > > > drop >> > > > > > in >> > > > > > replacement for the original version. Essential an >> > > > > > enhancement >> > > > > > rather >> > > > > > then a competitor. >> > > > > >> > > > > I think my comments weren't fundamental changes, but you >> > > > > surely >> > > > > mean >> > > > > the devicetree ABI? I like to see this driver ASAP out of >> > > > > staging >> > > > > and >> > > > > i'm not interested to maintain 2 functional identical driver >> > > > > only >> > > > > to >> > > > > keep compability with the Foundation tree. Currently i'm >> > > > > afraid >> > > > > that >> > > > > we build up many drivers in staging, which need a complete >> > > > > rewrite >> > > > > later if they should come out of staging. It would be nice if >> > > > > we >> > > > > could avoid the situation we have with the thermal driver. >> > > > > >> > > > > Stefan >> > > > >> > > > The API I'm talking about here is the mailbox API that is used >> > > > to >> > > > talk >> > > > to the firmware. The numbers and structures to pass are >> > > > documented. >> > > > Nothing prevents anybody from rewriting this driver and >> > > > submitting >> > > > it >> > > > to the appropriate subsystems. It's certainly small enough. >> > > > >> > > > If you really want working thermal or cpu speed drivers today, >> > > > nothing >> > > > stops anybody from submitting the downstream drivers after >> > > > doing >> > > > some >> > > > minor touchups and submitting them to staging. That would at >> > > > least >> > > > get >> > > > things working while people argue about what the correct DT >> > > > nodes >> > > > should be. >> > > > >> > > > I would also like to point out that the RPI 3 has been out for >> > > > over >> > > > a >> > > > year and nobody has been able to get working video out of it >> > > > through >> > > > VC4 on a mainline tree. At least until now. So I'm not sure >> > > > the >> > > > best >> > > > way to go is for the expander driver to go under the GPIO >> > > > subtree. >> > > >> > > Excuse me? Display works fine on my Pi3. VC4 uses DDC to probe >> > > for >> > > connection when the GPIO line isn't present in the DT. >> > >> > Just a FYI, Eric Anholt has offically announced support for VC4 for >> > HDMI on mainline Linus build without any support from the expander >> > on >> > the RPI 3. >> > >> > Sounds like this particular driver isn't needed then, correct? >> >> That's the HDMI audio that just landed. HDMI has been working on the >> pi3 since 9d44a8d530e8cc97d71ffcbc0ff3b5553c62. >> >> In the absence of a GPIO line for hotplug detect, we use DDC, which >> is >> slower and throws an error in dmesg when the probe happens but HDMI >> is >> disconnected. As such, having a GPIO driver would improve things for >> people. > > > Would a non DT based solution be outside the realm of possibilities? I > wrote a very simple library that emulates vcgencmd from the kernel. > One of the commands for vcgencmd is to query the HDMI hotplug status. > It's sort of semi documentated on the web. The vcgencmd library works, > but I haven't tested if the virtualized hotplug info from the command > output is correct in all cases. > > What I'm thinking is I could package this in a single file library that > would get put in upstream, but it would only support 1 command only. To > query the virtual hotplug status. > > The only thing is you would need to take a dependency from VC4 to > vchiq/vc04_services. It would get VC4 working better on upstream, but > it would mean taking a dependency from VC4 to vc04_services and the > stagging tree(with all that imples). > > I think this may be a better solution then arguing with the DT folks on > this expander driver that isn't being directly controlled. > > What do you think? Not interested. We should expose the full GPIO expander using the firmware's GPIO expander interfaces, which is a more generally useful solution. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging/bcm2835-camera: Set ourselves up as a platform driver.
This allows bcm2835-camera to automatically probe after VCHI has
loaded, rather than only successfully probing if the arbitrary probe
order chooses us after VCHI.
Signed-off-by: Eric Anholt
---
.../staging/vc04_services/bcm2835-camera/TODO | 11 ---
.../bcm2835-camera/bcm2835-camera.c| 18 ++
2 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/TODO
b/drivers/staging/vc04_services/bcm2835-camera/TODO
index 0ab9e88d769a..cefce72d814f 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/TODO
+++ b/drivers/staging/vc04_services/bcm2835-camera/TODO
@@ -21,14 +21,3 @@ less copy it needed to do.
The bulk_receive() does some manual cache flushing that are 32-bit ARM
only, which we should convert to proper cross-platform APIs.
-4) Convert to be a platform driver.
-
-Right now when the module probes, it tries to initialize VCHI and
-errors out if it wasn't ready yet. If bcm2835-v4l2 was built in, then
-VCHI generally isn't ready because it depends on both the firmware and
-mailbox drivers having already loaded.
-
-We should have VCHI create a platform device once it's initialized,
-and have this driver bind to it, so that we automatically load the
-v4l2 module after VCHI loads.
-
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index d2262275a870..aac876c35dea 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "mmal-common.h"
#include "mmal-encodings.h"
@@ -1803,7 +1804,7 @@ static struct v4l2_format default_v4l2_format = {
.fmt.pix.sizeimage = 1024 * 768,
};
-static int __init bm2835_mmal_init(void)
+static int __init bcm2835_mmal_probe(struct platform_device *pdev)
{
int ret;
struct bm2835_mmal_dev *dev;
@@ -1923,7 +1924,7 @@ static int __init bm2835_mmal_init(void)
return ret;
}
-static void __exit bm2835_mmal_exit(void)
+static int bcm2835_mmal_remove(struct platform_device *pdev)
{
int camera;
struct vchiq_mmal_instance *instance = gdev[0]->instance;
@@ -1933,7 +1934,16 @@ static void __exit bm2835_mmal_exit(void)
gdev[camera] = NULL;
}
vchiq_mmal_finalise(instance);
+
+ return 0;
}
-module_init(bm2835_mmal_init);
-module_exit(bm2835_mmal_exit);
+static struct platform_driver bcm2835_camera_driver = {
+ .probe = bcm2835_mmal_probe,
+ .remove = bcm2835_mmal_remove,
+ .driver = {
+ .name = "bcm2835-camera",
+ },
+};
+
+module_platform_driver(bcm2835_camera_driver)
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging/vc04_services: Register a platform device for the camera driver.
We had the camera driver set up in a module_init function, but that
meant that the camera driver would fail to load if it was initialized
before VCHI. This enforces that it loads after we've successfully set
up.
Signed-off-by: Eric Anholt
---
I'm going to try to get Dave Stevenson's new v4l2 codec driver merged
to staging (my primary motivation for getting vchi merged in the first
place!), and since his series touches the camera driver I needed to
probe the camera successfully in order to test it.
.../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 3cd6177a7373..aaa264f3b598 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -168,6 +168,7 @@ static VCHIQ_STATE_T g_state;
static struct class *vchiq_class;
static struct device *vchiq_dev;
static DEFINE_SPINLOCK(msg_queue_spinlock);
+static struct platform_device *bcm2835_camera;
static const char *const ioctl_names[] = {
"CONNECT",
@@ -3638,6 +3639,10 @@ static int vchiq_probe(struct platform_device *pdev)
VCHIQ_VERSION, VCHIQ_VERSION_MIN,
MAJOR(vchiq_devid), MINOR(vchiq_devid));
+ bcm2835_camera = platform_device_register_data(&pdev->dev,
+ "bcm2835-camera", -1,
+ NULL, 0);
+
return 0;
failed_debugfs_init:
@@ -3655,6 +3660,7 @@ static int vchiq_probe(struct platform_device *pdev)
static int vchiq_remove(struct platform_device *pdev)
{
+ platform_device_unregister(bcm2835_camera);
vchiq_debugfs_deinit();
device_destroy(vchiq_class, vchiq_devid);
class_destroy(vchiq_class);
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/15] staging/bcm2835-camera: Set ourselves up as a platform driver.
This allows bcm2835-camera to automatically probe after VCHI has
loaded, rather than only successfully probing if the arbitrary probe
order chooses us after VCHI.
Signed-off-by: Eric Anholt
---
.../staging/vc04_services/bcm2835-camera/TODO | 11 ---
.../bcm2835-camera/bcm2835-camera.c| 18 ++
2 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/TODO
b/drivers/staging/vc04_services/bcm2835-camera/TODO
index 0ab9e88d769a..cefce72d814f 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/TODO
+++ b/drivers/staging/vc04_services/bcm2835-camera/TODO
@@ -21,14 +21,3 @@ less copy it needed to do.
The bulk_receive() does some manual cache flushing that are 32-bit ARM
only, which we should convert to proper cross-platform APIs.
-4) Convert to be a platform driver.
-
-Right now when the module probes, it tries to initialize VCHI and
-errors out if it wasn't ready yet. If bcm2835-v4l2 was built in, then
-VCHI generally isn't ready because it depends on both the firmware and
-mailbox drivers having already loaded.
-
-We should have VCHI create a platform device once it's initialized,
-and have this driver bind to it, so that we automatically load the
-v4l2 module after VCHI loads.
-
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index d2262275a870..aac876c35dea 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "mmal-common.h"
#include "mmal-encodings.h"
@@ -1803,7 +1804,7 @@ static struct v4l2_format default_v4l2_format = {
.fmt.pix.sizeimage = 1024 * 768,
};
-static int __init bm2835_mmal_init(void)
+static int __init bcm2835_mmal_probe(struct platform_device *pdev)
{
int ret;
struct bm2835_mmal_dev *dev;
@@ -1923,7 +1924,7 @@ static int __init bm2835_mmal_init(void)
return ret;
}
-static void __exit bm2835_mmal_exit(void)
+static int bcm2835_mmal_remove(struct platform_device *pdev)
{
int camera;
struct vchiq_mmal_instance *instance = gdev[0]->instance;
@@ -1933,7 +1934,16 @@ static void __exit bm2835_mmal_exit(void)
gdev[camera] = NULL;
}
vchiq_mmal_finalise(instance);
+
+ return 0;
}
-module_init(bm2835_mmal_init);
-module_exit(bm2835_mmal_exit);
+static struct platform_driver bcm2835_camera_driver = {
+ .probe = bcm2835_mmal_probe,
+ .remove = bcm2835_mmal_remove,
+ .driver = {
+ .name = "bcm2835-camera",
+ },
+};
+
+module_platform_driver(bcm2835_camera_driver)
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/15] staging: bcm2835-camera: Add multiple include protection
From: Dave Stevenson
mmal-parameters.h didn't have the normal
...
protection to stop it being included multiple times. Add it.
Signed-off-by: Dave Stevenson
Signed-off-by: Eric Anholt
---
.../staging/vc04_services/bcm2835-camera/mmal-parameters.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
b/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
index 1607bc4c0347..3dc50593a665 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
@@ -18,6 +18,9 @@
* @{
*/
+#ifndef __MMAL_PARAMETERS_H
+#define __MMAL_PARAMETERS_H
+
/** Common parameter ID group, used with many types of component. */
#define MMAL_PARAMETER_GROUP_COMMON(0<<16)
/** Camera-specific parameter ID group. */
@@ -682,3 +685,5 @@ struct mmal_parameter_camera_info_t {
struct mmal_parameter_camera_info_flash_t
flashes[MMAL_PARAMETER_CAMERA_INFO_MAX_FLASHES];
};
+
+#endif
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/15] staging: bcm2835-camera: Remove V4L2/MMAL buffer remapping
From: Dave Stevenson
The MMAL and V4L2 buffers had been disassociated, and linked on
demand. Seeing as both are finite and low in number, and we now have
the same number of each, link them for the duration. This removes the
complexity of maintaining lists as the struct mmal_buffer context
comes back from the VPU, so we can directly link back to the relevant
V4L2 buffer.
Signed-off-by: Dave Stevenson
Signed-off-by: Eric Anholt
---
.../bcm2835-camera/bcm2835-camera.c | 7 +-
.../vc04_services/bcm2835-camera/mmal-vchiq.c | 109 --
2 files changed, 29 insertions(+), 87 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 8553b677eb08..c5ca56414139 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -299,8 +299,8 @@ static int buffer_prepare(struct vb2_buffer *vb)
struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
unsigned long size;
- v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, "%s: dev:%p\n",
-__func__, dev);
+ v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, "%s: dev:%p, vb %p\n",
+__func__, dev, vb);
BUG_ON(!dev->capture.port);
BUG_ON(!dev->capture.fmt);
@@ -492,7 +492,8 @@ static void buffer_queue(struct vb2_buffer *vb)
int ret;
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
-"%s: dev:%p buf:%p\n", __func__, dev, buf);
+"%s: dev:%p buf:%p, idx %u\n",
+__func__, dev, buf, vb2->vb2_buf.index);
ret = vchiq_mmal_submit_buffer(dev->instance, dev->capture.port, buf);
if (ret < 0)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 0f1961aeb223..3a3b843fc122 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -326,16 +326,12 @@ static int bulk_receive(struct vchiq_mmal_instance
*instance,
struct mmal_msg_context *msg_context)
{
unsigned long rd_len;
- unsigned long flags = 0;
int ret;
rd_len = msg->u.buffer_from_host.buffer_header.length;
- /* take buffer from queue */
- spin_lock_irqsave(&msg_context->u.bulk.port->slock, flags);
- if (list_empty(&msg_context->u.bulk.port->buffers)) {
- spin_unlock_irqrestore(&msg_context->u.bulk.port->slock, flags);
- pr_err("buffer list empty trying to submit bulk receive\n");
+ if (!msg_context->u.bulk.buffer) {
+ pr_err("bulk.buffer not configured - error in
buffer_from_host\n");
/* todo: this is a serious error, we should never have
* committed a buffer_to_host operation to the mmal
@@ -350,13 +346,6 @@ static int bulk_receive(struct vchiq_mmal_instance
*instance,
return -EINVAL;
}
- msg_context->u.bulk.buffer =
- list_entry(msg_context->u.bulk.port->buffers.next,
- struct mmal_buffer, list);
- list_del(&msg_context->u.bulk.buffer->list);
-
- spin_unlock_irqrestore(&msg_context->u.bulk.port->slock, flags);
-
/* ensure we do not overrun the available buffer */
if (rd_len > msg_context->u.bulk.buffer->buffer_size) {
rd_len = msg_context->u.bulk.buffer->buffer_size;
@@ -419,31 +408,6 @@ static int inline_receive(struct vchiq_mmal_instance
*instance,
struct mmal_msg *msg,
struct mmal_msg_context *msg_context)
{
- unsigned long flags = 0;
-
- /* take buffer from queue */
- spin_lock_irqsave(&msg_context->u.bulk.port->slock, flags);
- if (list_empty(&msg_context->u.bulk.port->buffers)) {
- spin_unlock_irqrestore(&msg_context->u.bulk.port->slock, flags);
- pr_err("buffer list empty trying to receive inline\n");
-
- /* todo: this is a serious error, we should never have
-* committed a buffer_to_host operation to the mmal
-* port without the buffer to back it up (with
-* underflow handling) and there is no obvious way to
-* deal with this. Less bad than the bulk case as we
-* can just drop this on the floor but...unhelpful
-*/
- return -EINVAL;
- }
-
- msg_context->u.bulk.buffer =
- list_entry(msg_context->u.bulk.port->buffers.next,
-
[PATCH 11/15] staging: bcm2835-camera: Fix comment typos.
From: Dave Stevenson
Fix a typo flagged by checkpatch, and another in the same line.
Signed-off-by: Dave Stevenson
Signed-off-by: Eric Anholt
---
drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h
b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h
index dd4b4ce72081..3b3ed79cadd9 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h
@@ -37,7 +37,7 @@ enum mmal_port_type {
*
* most elements are informational only, the pointer values for
* interogation messages are generally provided as additional
- * strucures within the message. When used to set values only teh
+ * structures within the message. When used to set values only the
* buffer_num, buffer_size and userdata parameters are writable.
*/
struct mmal_port {
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 14/15] staging: bcm2835: Remove dead code related to framerate.
Fixes a compiler warning about a set-but-not-used variable. I think
this was just leftover dead code from before set_framerate_params(),
since that also sets up some mmal_parameter_rational structs for fps.
Signed-off-by: Eric Anholt
---
.../staging/vc04_services/bcm2835-camera/bcm2835-camera.c| 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 2007088ab504..879c0b0ed958 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -1397,7 +1397,6 @@ static int vidioc_s_parm(struct file *file, void *priv,
{
struct bm2835_mmal_dev *dev = video_drvdata(file);
struct v4l2_fract tpf;
- struct mmal_parameter_rational fps_param;
if (parm->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
@@ -1414,10 +1413,6 @@ static int vidioc_s_parm(struct file *file, void *priv,
parm->parm.capture.readbuffers = 1;
parm->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
- fps_param.num = 0; /* Select variable fps, and then use
-* FPS_RANGE to select the actual limits.
-*/
- fps_param.den = 1;
set_framerate_params(dev);
return 0;
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 13/15] staging: bcm2835-camera: Fix warnings about string ops on v4l2 uapi.
The v4l2 uapi uses u8[] for strings, so cast those to char * to avoid compiler warnings about unsigned vs signed with sprintf() and friends. Signed-off-by: Eric Anholt --- .../vc04_services/bcm2835-camera/bcm2835-camera.c| 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c index ad53bce5c786..2007088ab504 100644 --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c @@ -693,7 +693,7 @@ static int vidioc_enum_fmt_vid_overlay(struct file *file, void *priv, fmt = &formats[f->index]; - strlcpy(f->description, fmt->name, sizeof(f->description)); + strlcpy((char *)f->description, fmt->name, sizeof(f->description)); f->pixelformat = fmt->fourcc; f->flags = fmt->flags; @@ -851,7 +851,7 @@ static int vidioc_enum_input(struct file *file, void *priv, return -EINVAL; inp->type = V4L2_INPUT_TYPE_CAMERA; - sprintf(inp->name, "Camera %u", inp->index); + sprintf((char *)inp->name, "Camera %u", inp->index); return 0; } @@ -879,11 +879,11 @@ static int vidioc_querycap(struct file *file, void *priv, vchiq_mmal_version(dev->instance, &major, &minor); - strcpy(cap->driver, "bm2835 mmal"); - snprintf(cap->card, sizeof(cap->card), "mmal service %d.%d", + strcpy((char *)cap->driver, "bm2835 mmal"); + snprintf((char *)cap->card, sizeof(cap->card), "mmal service %d.%d", major, minor); - snprintf(cap->bus_info, sizeof(cap->bus_info), + snprintf((char *)cap->bus_info, sizeof(cap->bus_info), "platform:%s", dev->v4l2_dev.name); cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OVERLAY | V4L2_CAP_STREAMING | V4L2_CAP_READWRITE; @@ -902,7 +902,7 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv, fmt = &formats[f->index]; - strlcpy(f->description, fmt->name, sizeof(f->description)); + strlcpy((char *)f->description, fmt->name, sizeof(f->description)); f->pixelformat = fmt->fourcc; f->flags = fmt->flags; -- 2.17.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/15] staging: bcm2835-camera: Remove bulk_mutex as it is not required
From: Dave Stevenson
There is no requirement to serialise bulk transfers as that is all
done in VCHI, and if a second MMAL_MSG_TYPE_BUFFER_TO_HOST happened
before the VCHI_CALLBACK_BULK_RECEIVED, then the service_callback
thread is deadlocked.
Remove the bulk_mutex so that multiple receives can be scheduled at a
time.
Signed-off-by: Dave Stevenson
Signed-off-by: Eric Anholt
---
.../vc04_services/bcm2835-camera/mmal-vchiq.c | 48 +--
1 file changed, 1 insertion(+), 47 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 037c68b83df9..d6950226551f 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -162,9 +162,6 @@ struct vchiq_mmal_instance {
/* ensure serialised access to service */
struct mutex vchiq_mutex;
- /* ensure serialised access to bulk operations */
- struct mutex bulk_mutex;
-
/* vmalloc page to receive scratch bulk xfers into */
void *bulk_scratch;
@@ -332,13 +329,6 @@ static int bulk_receive(struct vchiq_mmal_instance
*instance,
unsigned long flags = 0;
int ret;
- /* bulk mutex stops other bulk operations while we have a
-* receive in progress - released in callback
-*/
- ret = mutex_lock_interruptible(&instance->bulk_mutex);
- if (ret != 0)
- return ret;
-
rd_len = msg->u.buffer_from_host.buffer_header.length;
/* take buffer from queue */
@@ -357,8 +347,6 @@ static int bulk_receive(struct vchiq_mmal_instance
*instance,
* waiting bulk receive?
*/
- mutex_unlock(&instance->bulk_mutex);
-
return -EINVAL;
}
@@ -399,11 +387,6 @@ static int bulk_receive(struct vchiq_mmal_instance
*instance,
vchi_service_release(instance->handle);
- if (ret != 0) {
- /* callback will not be clearing the mutex */
- mutex_unlock(&instance->bulk_mutex);
- }
-
return ret;
}
@@ -413,13 +396,6 @@ static int dummy_bulk_receive(struct vchiq_mmal_instance
*instance,
{
int ret;
- /* bulk mutex stops other bulk operations while we have a
-* receive in progress - released in callback
-*/
- ret = mutex_lock_interruptible(&instance->bulk_mutex);
- if (ret != 0)
- return ret;
-
/* zero length indicates this was a dummy transfer */
msg_context->u.bulk.buffer_used = 0;
@@ -435,11 +411,6 @@ static int dummy_bulk_receive(struct vchiq_mmal_instance
*instance,
vchi_service_release(instance->handle);
- if (ret != 0) {
- /* callback will not be clearing the mutex */
- mutex_unlock(&instance->bulk_mutex);
- }
-
return ret;
}
@@ -494,18 +465,11 @@ buffer_from_host(struct vchiq_mmal_instance *instance,
pr_debug("instance:%p buffer:%p\n", instance->handle, buf);
- /* bulk mutex stops other bulk operations while we
-* have a receive in progress
-*/
- if (mutex_lock_interruptible(&instance->bulk_mutex))
- return -EINTR;
-
/* get context */
if (!buf->msg_context) {
pr_err("%s: msg_context not allocated, buf %p\n", __func__,
buf);
- ret = -EINVAL;
- goto unlock;
+ return -EINVAL;
}
msg_context = buf->msg_context;
@@ -559,9 +523,6 @@ buffer_from_host(struct vchiq_mmal_instance *instance,
vchi_service_release(instance->handle);
-unlock:
- mutex_unlock(&instance->bulk_mutex);
-
return ret;
}
@@ -685,9 +646,6 @@ static void buffer_to_host_cb(struct vchiq_mmal_instance
*instance,
static void bulk_receive_cb(struct vchiq_mmal_instance *instance,
struct mmal_msg_context *msg_context)
{
- /* bulk receive operation complete */
- mutex_unlock(&msg_context->u.bulk.instance->bulk_mutex);
-
/* replace the buffer header */
port_buffer_from_host(msg_context->u.bulk.instance,
msg_context->u.bulk.port);
@@ -703,9 +661,6 @@ static void bulk_abort_cb(struct vchiq_mmal_instance
*instance,
{
pr_err("%s: bulk ABORTED msg_context:%p\n", __func__, msg_context);
- /* bulk receive operation complete */
- mutex_unlock(&msg_context->u.bulk.instance->bulk_mutex);
-
/* replace the buffer header */
port_buffer_from_host(msg_context->u.bulk.instance,
msg_context->u.bulk.port);
@@ -2042,7 +1997,6 @@ int vchiq_mmal_init(struct vchiq_mmal_instance
**out_instance)
re
[PATCH 10/15] staging: bcm2835-camera: Replace BUG_ON with return error
From: Dave Stevenson
The error conditions don't warrant taking the kernel down, so remove
BUG_ON.
Signed-off-by: Dave Stevenson
Signed-off-by: Eric Anholt
---
.../staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index c5ca56414139..bd6bf3d991ef 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -302,8 +302,8 @@ static int buffer_prepare(struct vb2_buffer *vb)
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, "%s: dev:%p, vb %p\n",
__func__, dev, vb);
- BUG_ON(!dev->capture.port);
- BUG_ON(!dev->capture.fmt);
+ if (!dev->capture.port || !dev->capture.fmt)
+ return -ENODEV;
size = dev->capture.stride * dev->capture.height;
if (vb2_plane_size(vb, 0) < size) {
@@ -1017,7 +1017,8 @@ static int mmal_setup_components(struct bm2835_mmal_dev
*dev,
struct mmal_fmt *mfmt = get_format(f);
u32 remove_padding;
- BUG_ON(!mfmt);
+ if (!mfmt)
+ return -EINVAL;
if (dev->capture.encode_component) {
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/15] staging: bcm2835-camera: Allocate context once per buffer
From: Dave Stevenson
The struct mmal_msg_context was being allocated for every message
being sent to the VPU, and freed when it came back. Whilst that is
required behaviour for some messages (mainly the synchronous ones), it
is wasteful for the video buffers that make up the majority of the
traffic.
Add to the buffer_init/cleanup hooks that it allocates/frees the
msg_context required.
v2: changes by anholt from the downstream tree: clean up indentation,
pass an error value through, forward-declare the struct so we have
less void *
Signed-off-by: Dave Stevenson
Signed-off-by: Eric Anholt
---
.../bcm2835-camera/bcm2835-camera.c | 30 +--
.../bcm2835-camera/mmal-common.h | 4 ++
.../vc04_services/bcm2835-camera/mmal-vchiq.c | 38 ++-
.../vc04_services/bcm2835-camera/mmal-vchiq.h | 3 ++
4 files changed, 62 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 7b32c3a93873..dc1c2775bc0b 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -278,6 +278,20 @@ static int queue_setup(struct vb2_queue *vq,
return 0;
}
+static int buffer_init(struct vb2_buffer *vb)
+{
+ struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
+ struct vb2_v4l2_buffer *vb2 = to_vb2_v4l2_buffer(vb);
+ struct mmal_buffer *buf = container_of(vb2, struct mmal_buffer, vb);
+
+ v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, "%s: dev:%p, vb %p\n",
+__func__, dev, vb);
+ buf->buffer = vb2_plane_vaddr(&buf->vb.vb2_buf, 0);
+ buf->buffer_size = vb2_plane_size(&buf->vb.vb2_buf, 0);
+
+ return mmal_vchi_buffer_init(dev->instance, buf);
+}
+
static int buffer_prepare(struct vb2_buffer *vb)
{
struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
@@ -300,6 +314,17 @@ static int buffer_prepare(struct vb2_buffer *vb)
return 0;
}
+static void buffer_cleanup(struct vb2_buffer *vb)
+{
+ struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
+ struct vb2_v4l2_buffer *vb2 = to_vb2_v4l2_buffer(vb);
+ struct mmal_buffer *buf = container_of(vb2, struct mmal_buffer, vb);
+
+ v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, "%s: dev:%p, vb %p\n",
+__func__, dev, vb);
+ mmal_vchi_buffer_cleanup(buf);
+}
+
static inline bool is_capturing(struct bm2835_mmal_dev *dev)
{
return dev->capture.camera_port ==
@@ -467,9 +492,6 @@ static void buffer_queue(struct vb2_buffer *vb)
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
"%s: dev:%p buf:%p\n", __func__, dev, buf);
- buf->buffer = vb2_plane_vaddr(&buf->vb.vb2_buf, 0);
- buf->buffer_size = vb2_plane_size(&buf->vb.vb2_buf, 0);
-
ret = vchiq_mmal_submit_buffer(dev->instance, dev->capture.port, buf);
if (ret < 0)
v4l2_err(&dev->v4l2_dev, "%s: error submitting buffer\n",
@@ -632,7 +654,9 @@ static void bm2835_mmal_unlock(struct vb2_queue *vq)
static const struct vb2_ops bm2835_mmal_video_qops = {
.queue_setup = queue_setup,
+ .buf_init = buffer_init,
.buf_prepare = buffer_prepare,
+ .buf_cleanup = buffer_cleanup,
.buf_queue = buffer_queue,
.start_streaming = start_streaming,
.stop_streaming = stop_streaming,
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-common.h
b/drivers/staging/vc04_services/bcm2835-camera/mmal-common.h
index e68ca1bf7222..fe079d054174 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-common.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-common.h
@@ -19,6 +19,8 @@
/** Special value signalling that time is not known */
#define MMAL_TIME_UNKNOWN (1LL<<63)
+struct mmal_msg_context;
+
/* mapping between v4l and mmal video modes */
struct mmal_fmt {
char *name;
@@ -43,6 +45,8 @@ struct mmal_buffer {
void *buffer; /* buffer pointer */
unsigned long buffer_size; /* size of allocated buffer */
+
+ struct mmal_msg_context *msg_context;
};
/* */
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index a91ef6ea29ce..037c68b83df9 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -321,8 +321,6 @@ static void buffer_work_cb(struct work_struct *work)
msg_context->u.bulk.dts,
msg_context->u.bulk.pts);
- /* release message context */
- release_msg_contex
[PATCH 12/15] staging: bcm2835-camera: Fix indentation of tables
From: Dave Stevenson
As requested by Mauro Carvalho Chehab in review.
Signed-off-by: Dave Stevenson
Signed-off-by: Eric Anholt
---
.../bcm2835-camera/bcm2835-camera.c | 289 +-
1 file changed, 139 insertions(+), 150 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index bd6bf3d991ef..ad53bce5c786 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -79,145 +79,132 @@ static const struct v4l2_fract
/* video formats */
static struct mmal_fmt formats[] = {
{
-.name = "4:2:0, planar, YUV",
-.fourcc = V4L2_PIX_FMT_YUV420,
-.flags = 0,
-.mmal = MMAL_ENCODING_I420,
-.depth = 12,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 1,
-.remove_padding = 1,
-},
- {
-.name = "4:2:2, packed, YUYV",
-.fourcc = V4L2_PIX_FMT_YUYV,
-.flags = 0,
-.mmal = MMAL_ENCODING_YUYV,
-.depth = 16,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 2,
-.remove_padding = 0,
-},
- {
-.name = "RGB24 (LE)",
-.fourcc = V4L2_PIX_FMT_RGB24,
-.flags = 0,
-.mmal = MMAL_ENCODING_RGB24,
-.depth = 24,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 3,
-.remove_padding = 0,
-},
- {
-.name = "JPEG",
-.fourcc = V4L2_PIX_FMT_JPEG,
-.flags = V4L2_FMT_FLAG_COMPRESSED,
-.mmal = MMAL_ENCODING_JPEG,
-.depth = 8,
-.mmal_component = MMAL_COMPONENT_IMAGE_ENCODE,
-.ybbp = 0,
-.remove_padding = 0,
-},
- {
-.name = "H264",
-.fourcc = V4L2_PIX_FMT_H264,
-.flags = V4L2_FMT_FLAG_COMPRESSED,
-.mmal = MMAL_ENCODING_H264,
-.depth = 8,
-.mmal_component = MMAL_COMPONENT_VIDEO_ENCODE,
-.ybbp = 0,
-.remove_padding = 0,
-},
- {
-.name = "MJPEG",
-.fourcc = V4L2_PIX_FMT_MJPEG,
-.flags = V4L2_FMT_FLAG_COMPRESSED,
-.mmal = MMAL_ENCODING_MJPEG,
-.depth = 8,
-.mmal_component = MMAL_COMPONENT_VIDEO_ENCODE,
-.ybbp = 0,
-.remove_padding = 0,
-},
- {
-.name = "4:2:2, packed, YVYU",
-.fourcc = V4L2_PIX_FMT_YVYU,
-.flags = 0,
-.mmal = MMAL_ENCODING_YVYU,
-.depth = 16,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 2,
-.remove_padding = 0,
-},
- {
-.name = "4:2:2, packed, VYUY",
-.fourcc = V4L2_PIX_FMT_VYUY,
-.flags = 0,
-.mmal = MMAL_ENCODING_VYUY,
-.depth = 16,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 2,
-.remove_padding = 0,
-},
- {
-.name = "4:2:2, packed, UYVY",
-.fourcc = V4L2_PIX_FMT_UYVY,
-.flags = 0,
-.mmal = MMAL_ENCODING_UYVY,
-.depth = 16,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 2,
-.remove_padding = 0,
-},
- {
-.name = "4:2:0, planar, NV12",
-.fourcc = V4L2_PIX_FMT_NV12,
-.flags = 0,
-.mmal = MMAL_ENCODING_NV12,
-.depth = 12,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 1,
-.remove_padding = 1,
-},
- {
-.name = "RGB24 (BE)",
-.fourcc = V4L2_PIX_FMT_BGR24,
-.flags = 0,
-.mmal = MMAL_ENCODING_BGR24,
-.depth = 24,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 3,
-.remove_padding = 0,
-},
- {
-.name = "4:2:0, planar, YVU",
-.fourcc = V4L2_PIX_FMT_YVU420,
-.flags = 0,
-.mmal = MMAL_ENCODING_YV12,
-.depth = 12,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 1,
-.remove_padding = 1,
-},
- {
-.name = "4:2:0, planar, NV21",
-.fourcc = V4L2_PIX_FMT_NV21,
-.flags = 0,
-.mmal = MMAL_ENCODING_NV21,
-.depth = 12,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 1,
-.remove_padding = 1,
-},
- {
-.name = "RGB32 (BE)",
-.fourcc = V4L2_PIX_FMT_BGR32,
-.flags = 0,
-.mmal = MMAL_ENCODING_BGRA,
-.depth = 32,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 4,
-.remove_padding = 0,
-},
+ .name = "4:2:0, planar, YUV",
+ .fourcc = V4L2_PIX_FMT_YUV420,
+ .flags = 0,
+ .mmal = MMAL_ENCODING_I420,
+ .depth = 12,
+ .mmal_
[PATCH 06/15] staging: bcm2835-camera: Match MMAL buffer count to V4L2.
From: Dave Stevenson
For historical reasons, the number of buffers passed to the VPU over
MMAL did not match that passed from V4L2. That is a silly situation
as the driver has to duplicate serialisation and other functions that
have already been implemented in V4L2/videobuf2.
As we had more V4L2 buffers than MMAL ones, the MMAL buffer headers
were returned to the VPU immediately on being filled, which is now
invalid.
Match the number of buffers notified in queue_setup with that used in
MMAL. Return buffers only when we get them from V4L2.
Signed-off-by: Dave Stevenson
Signed-off-by: Eric Anholt
---
.../bcm2835-camera/bcm2835-camera.c | 6 --
.../vc04_services/bcm2835-camera/mmal-vchiq.c | 21 +--
.../vc04_services/bcm2835-camera/mmal-vchiq.h | 4
3 files changed, 5 insertions(+), 26 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index dc1c2775bc0b..8553b677eb08 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -260,8 +260,10 @@ static int queue_setup(struct vb2_queue *vq,
return -EINVAL;
}
- if (*nbuffers < (dev->capture.port->current_buffer.num + 2))
- *nbuffers = (dev->capture.port->current_buffer.num + 2);
+ if (*nbuffers < dev->capture.port->minimum_buffer.num)
+ *nbuffers = dev->capture.port->minimum_buffer.num;
+
+ dev->capture.port->current_buffer.num = *nbuffers;
*nplanes = 1;
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index d6950226551f..0f1961aeb223 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -545,7 +545,6 @@ static int port_buffer_from_host(struct vchiq_mmal_instance
*instance,
/* peek buffer from queue */
spin_lock_irqsave(&port->slock, flags);
if (list_empty(&port->buffers)) {
- port->buffer_underflow++;
spin_unlock_irqrestore(&port->slock, flags);
return -ENOSPC;
}
@@ -636,9 +635,6 @@ static void buffer_to_host_cb(struct vchiq_mmal_instance
*instance,
msg->u.buffer_from_host.payload_in_message;
}
- /* replace the buffer header */
- port_buffer_from_host(instance, msg_context->u.bulk.port);
-
/* schedule the port callback */
schedule_work(&msg_context->u.bulk.work);
}
@@ -646,10 +642,6 @@ static void buffer_to_host_cb(struct vchiq_mmal_instance
*instance,
static void bulk_receive_cb(struct vchiq_mmal_instance *instance,
struct mmal_msg_context *msg_context)
{
- /* replace the buffer header */
- port_buffer_from_host(msg_context->u.bulk.instance,
- msg_context->u.bulk.port);
-
msg_context->u.bulk.status = 0;
/* schedule the port callback */
@@ -661,10 +653,6 @@ static void bulk_abort_cb(struct vchiq_mmal_instance
*instance,
{
pr_err("%s: bulk ABORTED msg_context:%p\n", __func__, msg_context);
- /* replace the buffer header */
- port_buffer_from_host(msg_context->u.bulk.instance,
- msg_context->u.bulk.port);
-
msg_context->u.bulk.status = -EINTR;
schedule_work(&msg_context->u.bulk.work);
@@ -1713,14 +1701,7 @@ int vchiq_mmal_submit_buffer(struct vchiq_mmal_instance
*instance,
list_add_tail(&buffer->list, &port->buffers);
spin_unlock_irqrestore(&port->slock, flags);
- /* the port previously underflowed because it was missing a
-* mmal_buffer which has just been added, submit that buffer
-* to the mmal service.
-*/
- if (port->buffer_underflow) {
- port_buffer_from_host(instance, port);
- port->buffer_underflow--;
- }
+ port_buffer_from_host(instance, port);
return 0;
}
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
index dadf47fe1bdc..0ab9f660b822 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
@@ -79,10 +79,6 @@ struct vchiq_mmal_port {
struct list_head buffers;
/* lock to serialise adding and removing buffers from list */
spinlock_t slock;
- /* count of how many buffer header refils have failed because
-* there was no buffer to satisfy them
-*/
- int buffer_underflow;
/* callback on buffer completion */
vchiq_mma
[PATCH 12/12] staging: bcm2835-camera: Fix identation of tables
From: Dave Stevenson
As requested by Mauro Carvalho Chehab in review.
Signed-off-by: Dave Stevenson
Signed-off-by: Eric Anholt
---
.../bcm2835-camera/bcm2835-camera.c | 289 +-
1 file changed, 139 insertions(+), 150 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index bd6bf3d991ef..ad53bce5c786 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -79,145 +79,132 @@ static const struct v4l2_fract
/* video formats */
static struct mmal_fmt formats[] = {
{
-.name = "4:2:0, planar, YUV",
-.fourcc = V4L2_PIX_FMT_YUV420,
-.flags = 0,
-.mmal = MMAL_ENCODING_I420,
-.depth = 12,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 1,
-.remove_padding = 1,
-},
- {
-.name = "4:2:2, packed, YUYV",
-.fourcc = V4L2_PIX_FMT_YUYV,
-.flags = 0,
-.mmal = MMAL_ENCODING_YUYV,
-.depth = 16,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 2,
-.remove_padding = 0,
-},
- {
-.name = "RGB24 (LE)",
-.fourcc = V4L2_PIX_FMT_RGB24,
-.flags = 0,
-.mmal = MMAL_ENCODING_RGB24,
-.depth = 24,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 3,
-.remove_padding = 0,
-},
- {
-.name = "JPEG",
-.fourcc = V4L2_PIX_FMT_JPEG,
-.flags = V4L2_FMT_FLAG_COMPRESSED,
-.mmal = MMAL_ENCODING_JPEG,
-.depth = 8,
-.mmal_component = MMAL_COMPONENT_IMAGE_ENCODE,
-.ybbp = 0,
-.remove_padding = 0,
-},
- {
-.name = "H264",
-.fourcc = V4L2_PIX_FMT_H264,
-.flags = V4L2_FMT_FLAG_COMPRESSED,
-.mmal = MMAL_ENCODING_H264,
-.depth = 8,
-.mmal_component = MMAL_COMPONENT_VIDEO_ENCODE,
-.ybbp = 0,
-.remove_padding = 0,
-},
- {
-.name = "MJPEG",
-.fourcc = V4L2_PIX_FMT_MJPEG,
-.flags = V4L2_FMT_FLAG_COMPRESSED,
-.mmal = MMAL_ENCODING_MJPEG,
-.depth = 8,
-.mmal_component = MMAL_COMPONENT_VIDEO_ENCODE,
-.ybbp = 0,
-.remove_padding = 0,
-},
- {
-.name = "4:2:2, packed, YVYU",
-.fourcc = V4L2_PIX_FMT_YVYU,
-.flags = 0,
-.mmal = MMAL_ENCODING_YVYU,
-.depth = 16,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 2,
-.remove_padding = 0,
-},
- {
-.name = "4:2:2, packed, VYUY",
-.fourcc = V4L2_PIX_FMT_VYUY,
-.flags = 0,
-.mmal = MMAL_ENCODING_VYUY,
-.depth = 16,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 2,
-.remove_padding = 0,
-},
- {
-.name = "4:2:2, packed, UYVY",
-.fourcc = V4L2_PIX_FMT_UYVY,
-.flags = 0,
-.mmal = MMAL_ENCODING_UYVY,
-.depth = 16,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 2,
-.remove_padding = 0,
-},
- {
-.name = "4:2:0, planar, NV12",
-.fourcc = V4L2_PIX_FMT_NV12,
-.flags = 0,
-.mmal = MMAL_ENCODING_NV12,
-.depth = 12,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 1,
-.remove_padding = 1,
-},
- {
-.name = "RGB24 (BE)",
-.fourcc = V4L2_PIX_FMT_BGR24,
-.flags = 0,
-.mmal = MMAL_ENCODING_BGR24,
-.depth = 24,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 3,
-.remove_padding = 0,
-},
- {
-.name = "4:2:0, planar, YVU",
-.fourcc = V4L2_PIX_FMT_YVU420,
-.flags = 0,
-.mmal = MMAL_ENCODING_YV12,
-.depth = 12,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 1,
-.remove_padding = 1,
-},
- {
-.name = "4:2:0, planar, NV21",
-.fourcc = V4L2_PIX_FMT_NV21,
-.flags = 0,
-.mmal = MMAL_ENCODING_NV21,
-.depth = 12,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 1,
-.remove_padding = 1,
-},
- {
-.name = "RGB32 (BE)",
-.fourcc = V4L2_PIX_FMT_BGR32,
-.flags = 0,
-.mmal = MMAL_ENCODING_BGRA,
-.depth = 32,
-.mmal_component = MMAL_COMPONENT_CAMERA,
-.ybbp = 4,
-.remove_padding = 0,
-},
+ .name = "4:2:0, planar, YUV",
+ .fourcc = V4L2_PIX_FMT_YUV420,
+ .flags = 0,
+ .mmal = MMAL_ENCODING_I420,
+ .depth = 12,
+ .mmal_
[PATCH 00/15] staging: bcm2835-camera probing and cleanup
I'm going to try to get Dave Stevenson's new zero-copy v4l2 M2M codec driver merged to staging (my primary motivation for getting vchi merged in the first place!), and this series makes the camera driver probe successfully (an important first step!) and brings in some of his cleanup changes. Also a couple of compiler warning fixes, for my own sanity. Apologies to those who saw the first two patches last night. My git-send-email-for-staging script was missing gregkh. Dave Stevenson (10): staging: bcm2835-camera: Skip ISP pass to eliminate padding. staging: bcm2835-camera: Allocate context once per buffer staging: bcm2835-camera: Remove bulk_mutex as it is not required staging: bcm2835-camera: Match MMAL buffer count to V4L2. staging: bcm2835-camera: Remove V4L2/MMAL buffer remapping staging: bcm2835-camera: Add multiple include protection staging: bcm2835-camera: Move struct vchiq_mmal_rect staging: bcm2835-camera: Replace BUG_ON with return error staging: bcm2835-camera: Fix comment typos. staging: bcm2835-camera: Fix indentation of tables Eric Anholt (5): staging/vc04_services: Register a platform device for the camera driver. staging/bcm2835-camera: Set ourselves up as a platform driver. staging: bcm2835-camera: Fix warnings about string ops on v4l2 uapi. staging: bcm2835: Remove dead code related to framerate. staging: bcm2835: Fix mmal_port_parameter_get() signed/unsigned warnings. .../staging/vc04_services/bcm2835-camera/TODO | 11 - .../bcm2835-camera/bcm2835-camera.c | 396 ++ .../bcm2835-camera/mmal-common.h | 7 + .../bcm2835-camera/mmal-msg-port.h| 2 +- .../bcm2835-camera/mmal-parameters.h | 13 + .../vc04_services/bcm2835-camera/mmal-vchiq.c | 210 +++--- .../vc04_services/bcm2835-camera/mmal-vchiq.h | 15 +- .../interface/vchiq_arm/vchiq_arm.c | 6 + 8 files changed, 301 insertions(+), 359 deletions(-) -- 2.17.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 15/15] staging: bcm2835: Fix mmal_port_parameter_get() signed/unsigned warnings.
The arg is a u32 *, so switch over to that in our declarations.
Signed-off-by: Eric Anholt
---
.../staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 879c0b0ed958..53f33fb3998b 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -492,7 +492,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
{
struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vq);
int ret;
- int parameter_size;
+ u32 parameter_size;
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, "%s: dev:%p\n",
__func__, dev);
@@ -1489,7 +1489,7 @@ static int get_num_cameras(struct vchiq_mmal_instance
*instance,
int ret;
struct vchiq_mmal_component *cam_info_component;
struct mmal_parameter_camera_info_t cam_info = {0};
- int param_size = sizeof(cam_info);
+ u32 param_size = sizeof(cam_info);
int i;
/* create a camera_info component */
@@ -1553,7 +1553,7 @@ static int __init mmal_init(struct bm2835_mmal_dev *dev)
int ret;
struct mmal_es_format_local *format;
u32 supported_encodings[MAX_SUPPORTED_ENCODINGS];
- int param_size;
+ u32 param_size;
struct vchiq_mmal_component *camera;
ret = vchiq_mmal_init(&dev->instance);
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/15] staging: bcm2835-camera: Skip ISP pass to eliminate padding.
From: Dave Stevenson <6...@users.noreply.github.com>
Interleaved RGB and single plane YUV formats can be delivered by the
GPU without the secondary step of removing padding, as the
bytesperline field can be set appropriately.
Planar YUV needs the GPU to still remove padding, as there is no way
to report that there is padding between the planes (ie on the height).
The multi-planar formats are NOT applicable, as there is no easy way
to make them contiguous in memory (ie one large allocation that gets
broken up). The whole task is passed across to videobuf2 which has no
notion of that requirement.
v2: Changes by anholt from the downstream driver: Flag two more planar
formats as needing padding removal, and remove broken userspace
workaround.
Signed-off-by: Dave Stevenson
Signed-off-by: Eric Anholt
---
.../bcm2835-camera/bcm2835-camera.c | 44 ++-
.../bcm2835-camera/mmal-common.h | 3 ++
2 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index aac876c35dea..7b32c3a93873 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -86,6 +86,7 @@ static struct mmal_fmt formats[] = {
.depth = 12,
.mmal_component = MMAL_COMPONENT_CAMERA,
.ybbp = 1,
+.remove_padding = 1,
},
{
.name = "4:2:2, packed, YUYV",
@@ -95,6 +96,7 @@ static struct mmal_fmt formats[] = {
.depth = 16,
.mmal_component = MMAL_COMPONENT_CAMERA,
.ybbp = 2,
+.remove_padding = 0,
},
{
.name = "RGB24 (LE)",
@@ -104,6 +106,7 @@ static struct mmal_fmt formats[] = {
.depth = 24,
.mmal_component = MMAL_COMPONENT_CAMERA,
.ybbp = 3,
+.remove_padding = 0,
},
{
.name = "JPEG",
@@ -113,6 +116,7 @@ static struct mmal_fmt formats[] = {
.depth = 8,
.mmal_component = MMAL_COMPONENT_IMAGE_ENCODE,
.ybbp = 0,
+.remove_padding = 0,
},
{
.name = "H264",
@@ -122,6 +126,7 @@ static struct mmal_fmt formats[] = {
.depth = 8,
.mmal_component = MMAL_COMPONENT_VIDEO_ENCODE,
.ybbp = 0,
+.remove_padding = 0,
},
{
.name = "MJPEG",
@@ -131,6 +136,7 @@ static struct mmal_fmt formats[] = {
.depth = 8,
.mmal_component = MMAL_COMPONENT_VIDEO_ENCODE,
.ybbp = 0,
+.remove_padding = 0,
},
{
.name = "4:2:2, packed, YVYU",
@@ -140,6 +146,7 @@ static struct mmal_fmt formats[] = {
.depth = 16,
.mmal_component = MMAL_COMPONENT_CAMERA,
.ybbp = 2,
+.remove_padding = 0,
},
{
.name = "4:2:2, packed, VYUY",
@@ -149,6 +156,7 @@ static struct mmal_fmt formats[] = {
.depth = 16,
.mmal_component = MMAL_COMPONENT_CAMERA,
.ybbp = 2,
+.remove_padding = 0,
},
{
.name = "4:2:2, packed, UYVY",
@@ -158,6 +166,7 @@ static struct mmal_fmt formats[] = {
.depth = 16,
.mmal_component = MMAL_COMPONENT_CAMERA,
.ybbp = 2,
+.remove_padding = 0,
},
{
.name = "4:2:0, planar, NV12",
@@ -167,6 +176,7 @@ static struct mmal_fmt formats[] = {
.depth = 12,
.mmal_component = MMAL_COMPONENT_CAMERA,
.ybbp = 1,
+.remove_padding = 1,
},
{
.name = "RGB24 (BE)",
@@ -176,6 +186,7 @@ static struct mmal_fmt formats[] = {
.depth = 24,
.mmal_component = MMAL_COMPONENT_CAMERA,
.ybbp = 3,
+.remove_padding = 0,
},
{
.name = "4:2:0, planar, YVU",
@@ -185,6 +196,7 @@ static struct mmal_fmt formats[] = {
.depth = 12,
.mmal_component = MMAL_COMPONENT_CAMERA,
.ybbp = 1,
+.remove_padding = 1,
},
{
.name = "4:2:0, planar, NV21",
@@ -194,6 +206,7 @@ static struct mmal_fmt formats[] = {
.depth = 12,
.mmal_component = MMAL_COMPONENT_CAMERA,
.ybbp = 1,
+.remove_padding = 1,
},
{
.name = "RGB32 (BE)",
@@ -203,6 +216,7 @@ static struct mmal_fmt formats[] = {
.depth = 32,
.mmal_component = MMAL_COMPONENT_CAMERA,
.ybbp = 4,
+.remove_padding = 0,
},
};
@@ -929,9 +943,19 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void
*priv,
&f->fmt.pix.height, MIN_HEIGHT, dev->max_height,
1, 0);
f->fmt.pix.bytesperline = f->fmt.pix.width
[PATCH 09/15] staging: bcm2835-camera: Move struct vchiq_mmal_rect
From: Dave Stevenson
struct vchiq_mmal_rect is only referenced from mmal-parameters.h, yet
was defined in mmal-vchiq.h.
Move it to avoid having to include multiple headers for no reason.
Signed-off-by: Dave Stevenson
Signed-off-by: Eric Anholt
---
.../vc04_services/bcm2835-camera/mmal-parameters.h| 8
drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h | 8
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
b/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
index 3dc50593a665..184024dfb8b7 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
@@ -564,6 +564,14 @@ enum mmal_parameter_displayset {
MMAL_DISPLAY_SET_ALPHA = 0x400,
};
+/* rectangle, used lots so it gets its own struct */
+struct vchiq_mmal_rect {
+ s32 x;
+ s32 y;
+ s32 width;
+ s32 height;
+};
+
struct mmal_parameter_displayregion {
/** Bitfield that indicates which fields are set and should be
* used. All other fields will maintain their current value.
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
index 0ab9f660b822..22b839ecd5f0 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
@@ -32,14 +32,6 @@ enum vchiq_mmal_es_type {
MMAL_ES_TYPE_SUBPICTURE /**< Sub-picture elementary stream */
};
-/* rectangle, used lots so it gets its own struct */
-struct vchiq_mmal_rect {
- s32 x;
- s32 y;
- s32 width;
- s32 height;
-};
-
struct vchiq_mmal_port_buffer {
unsigned int num; /* number of buffers */
u32 size; /* size of buffers */
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/15] staging/vc04_services: Register a platform device for the camera driver.
We had the camera driver set up in a module_init function, but that
meant that the camera driver would fail to load if it was initialized
before VCHI. By attaching to this platform_device, it can get a
defined load order.
Signed-off-by: Eric Anholt
---
.../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 3cd6177a7373..aaa264f3b598 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -168,6 +168,7 @@ static VCHIQ_STATE_T g_state;
static struct class *vchiq_class;
static struct device *vchiq_dev;
static DEFINE_SPINLOCK(msg_queue_spinlock);
+static struct platform_device *bcm2835_camera;
static const char *const ioctl_names[] = {
"CONNECT",
@@ -3638,6 +3639,10 @@ static int vchiq_probe(struct platform_device *pdev)
VCHIQ_VERSION, VCHIQ_VERSION_MIN,
MAJOR(vchiq_devid), MINOR(vchiq_devid));
+ bcm2835_camera = platform_device_register_data(&pdev->dev,
+ "bcm2835-camera", -1,
+ NULL, 0);
+
return 0;
failed_debugfs_init:
@@ -3655,6 +3660,7 @@ static int vchiq_probe(struct platform_device *pdev)
static int vchiq_remove(struct platform_device *pdev)
{
+ platform_device_unregister(bcm2835_camera);
vchiq_debugfs_deinit();
device_destroy(vchiq_class, vchiq_devid);
class_destroy(vchiq_class);
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 12/12] staging: bcm2835-camera: Fix identation of tables
Eric Anholt writes: > From: Dave Stevenson > > As requested by Mauro Carvalho Chehab in review. > > Signed-off-by: Dave Stevenson > Signed-off-by: Eric Anholt Ignore this one, the other 12/12 is just this with "identation" spelled right. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: bcm2835-camera: Replace open-coded idr with a struct idr.
We just need some integer handles that can map back to our message
struct when we're handling a reply, which struct idr is perfect for.
Signed-off-by: Eric Anholt
---
.../vc04_services/bcm2835-camera/mmal-vchiq.c | 135 --
1 file changed, 31 insertions(+), 104 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 3a3b843fc122..f0dac6440cfb 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -21,7 +21,6 @@
#include
#include
#include
-#include
#include
#include
@@ -111,7 +110,11 @@ struct vchiq_mmal_instance;
/* normal message context */
struct mmal_msg_context {
struct vchiq_mmal_instance *instance;
- u32 handle;
+
+ /* Index in the context_map idr so that we can find the
+* mmal_msg_context again when servicing the VCHI reply.
+*/
+ int handle;
union {
struct {
@@ -149,13 +152,6 @@ struct mmal_msg_context {
};
-struct vchiq_mmal_context_map {
- /* ensure serialized access to the btree(contention should be low) */
- struct mutex lock;
- struct btree_head32 btree_head;
- u32 last_handle;
-};
-
struct vchiq_mmal_instance {
VCHI_SERVICE_HANDLE_T handle;
@@ -165,92 +161,19 @@ struct vchiq_mmal_instance {
/* vmalloc page to receive scratch bulk xfers into */
void *bulk_scratch;
- /* mapping table between context handles and mmal_msg_contexts */
- struct vchiq_mmal_context_map context_map;
+ struct idr context_map;
+ spinlock_t context_map_lock;
/* component to use next */
int component_idx;
struct vchiq_mmal_component component[VCHIQ_MMAL_MAX_COMPONENTS];
};
-static int __must_check
-mmal_context_map_init(struct vchiq_mmal_context_map *context_map)
-{
- mutex_init(&context_map->lock);
- context_map->last_handle = 0;
- return btree_init32(&context_map->btree_head);
-}
-
-static void mmal_context_map_destroy(struct vchiq_mmal_context_map
*context_map)
-{
- mutex_lock(&context_map->lock);
- btree_destroy32(&context_map->btree_head);
- mutex_unlock(&context_map->lock);
-}
-
-static u32
-mmal_context_map_create_handle(struct vchiq_mmal_context_map *context_map,
- struct mmal_msg_context *msg_context,
- gfp_t gfp)
-{
- u32 handle;
-
- mutex_lock(&context_map->lock);
-
- while (1) {
- /* just use a simple count for handles, but do not use 0 */
- context_map->last_handle++;
- if (!context_map->last_handle)
- context_map->last_handle++;
-
- handle = context_map->last_handle;
-
- /* check if the handle is already in use */
- if (!btree_lookup32(&context_map->btree_head, handle))
- break;
- }
-
- if (btree_insert32(&context_map->btree_head, handle,
- msg_context, gfp)) {
- /* probably out of memory */
- mutex_unlock(&context_map->lock);
- return 0;
- }
-
- mutex_unlock(&context_map->lock);
- return handle;
-}
-
-static struct mmal_msg_context *
-mmal_context_map_lookup_handle(struct vchiq_mmal_context_map *context_map,
- u32 handle)
-{
- struct mmal_msg_context *msg_context;
-
- if (!handle)
- return NULL;
-
- mutex_lock(&context_map->lock);
-
- msg_context = btree_lookup32(&context_map->btree_head, handle);
-
- mutex_unlock(&context_map->lock);
- return msg_context;
-}
-
-static void
-mmal_context_map_destroy_handle(struct vchiq_mmal_context_map *context_map,
- u32 handle)
-{
- mutex_lock(&context_map->lock);
- btree_remove32(&context_map->btree_head, handle);
- mutex_unlock(&context_map->lock);
-}
-
static struct mmal_msg_context *
get_msg_context(struct vchiq_mmal_instance *instance)
{
struct mmal_msg_context *msg_context;
+ int handle;
/* todo: should this be allocated from a pool to avoid kzalloc */
msg_context = kzalloc(sizeof(*msg_context), GFP_KERNEL);
@@ -258,32 +181,40 @@ get_msg_context(struct vchiq_mmal_instance *instance)
if (!msg_context)
return ERR_PTR(-ENOMEM);
- msg_context->instance = instance;
- msg_context->handle =
- mmal_context_map_create_handle(&instance->context_map,
- msg_context,
- GFP_KERNEL);
+ /* Create an ID that wi
Re: [PATCH] staging: bcm2835-camera: Replace open-coded idr with a struct idr.
Stefan Wahren writes:
> Hi Eric,
>
>> Eric Anholt hat am 11. Mai 2018 um 01:31 geschrieben:
>>
>>
>> We just need some integer handles that can map back to our message
>> struct when we're handling a reply, which struct idr is perfect for.
>>
>> Signed-off-by: Eric Anholt
>> ---
>> .../vc04_services/bcm2835-camera/mmal-vchiq.c | 135 --
>> 1 file changed, 31 insertions(+), 104 deletions(-)
>>
>> diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
>> b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
>> index 3a3b843fc122..f0dac6440cfb 100644
>> --- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
>> +++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
>> @@ -21,7 +21,6 @@
>> #include
>> #include
>> #include
>> -#include
>> #include
>> #include
>>
>> @@ -111,7 +110,11 @@ struct vchiq_mmal_instance;
>> /* normal message context */
>> struct mmal_msg_context {
>> struct vchiq_mmal_instance *instance;
>> -u32 handle;
>> +
>> +/* Index in the context_map idr so that we can find the
>> + * mmal_msg_context again when servicing the VCHI reply.
>> + */
>> +int handle;
>>
>> union {
>> struct {
>> @@ -149,13 +152,6 @@ struct mmal_msg_context {
>>
>> };
>>
>> -struct vchiq_mmal_context_map {
>> -/* ensure serialized access to the btree(contention should be low) */
>> -struct mutex lock;
>> -struct btree_head32 btree_head;
>> -u32 last_handle;
>> -};
>> -
>> struct vchiq_mmal_instance {
>> VCHI_SERVICE_HANDLE_T handle;
>>
>> @@ -165,92 +161,19 @@ struct vchiq_mmal_instance {
>> /* vmalloc page to receive scratch bulk xfers into */
>> void *bulk_scratch;
>>
>> -/* mapping table between context handles and mmal_msg_contexts */
>> -struct vchiq_mmal_context_map context_map;
>> +struct idr context_map;
>> +spinlock_t context_map_lock;
>>
>> /* component to use next */
>> int component_idx;
>> struct vchiq_mmal_component component[VCHIQ_MMAL_MAX_COMPONENTS];
>> };
>>
>> -static int __must_check
>> -mmal_context_map_init(struct vchiq_mmal_context_map *context_map)
>> -{
>> -mutex_init(&context_map->lock);
>> -context_map->last_handle = 0;
>> -return btree_init32(&context_map->btree_head);
>> -}
>> -
>> -static void mmal_context_map_destroy(struct vchiq_mmal_context_map
>> *context_map)
>> -{
>> -mutex_lock(&context_map->lock);
>> -btree_destroy32(&context_map->btree_head);
>> -mutex_unlock(&context_map->lock);
>> -}
>> -
>> -static u32
>> -mmal_context_map_create_handle(struct vchiq_mmal_context_map *context_map,
>> - struct mmal_msg_context *msg_context,
>> - gfp_t gfp)
>> -{
>> -u32 handle;
>> -
>> -mutex_lock(&context_map->lock);
>> -
>> -while (1) {
>> -/* just use a simple count for handles, but do not use 0 */
>> -context_map->last_handle++;
>> -if (!context_map->last_handle)
>> -context_map->last_handle++;
>> -
>> -handle = context_map->last_handle;
>> -
>> -/* check if the handle is already in use */
>> -if (!btree_lookup32(&context_map->btree_head, handle))
>> -break;
>> -}
>> -
>> -if (btree_insert32(&context_map->btree_head, handle,
>> - msg_context, gfp)) {
>> -/* probably out of memory */
>> -mutex_unlock(&context_map->lock);
>> -return 0;
>> -}
>> -
>> -mutex_unlock(&context_map->lock);
>> -return handle;
>> -}
>> -
>> -static struct mmal_msg_context *
>> -mmal_context_map_lookup_handle(struct vchiq_mmal_context_map *context_map,
>> - u32 handle)
>> -{
>> -struct mmal_msg_context *msg_context;
>> -
>> -if (!handle)
>> -return NULL;
>> -
>> -mutex_lock(&context_map->lock);
>> -
>> -msg_context = btree_lookup32(&context_map->btree_head, handle);
>> -
>> -mutex_unlock(&conte
[PATCH v2] staging: bcm2835-camera: Replace open-coded idr with a struct idr.
We just need some integer handles that can map back to our message
struct when we're handling a reply, which struct idr is perfect for.
v2: Fix error check to look at the right variable.
Signed-off-by: Eric Anholt
---
.../vc04_services/bcm2835-camera/mmal-vchiq.c | 135 --
1 file changed, 31 insertions(+), 104 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 3a3b843fc122..f5b5ead6347c 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -21,7 +21,6 @@
#include
#include
#include
-#include
#include
#include
@@ -111,7 +110,11 @@ struct vchiq_mmal_instance;
/* normal message context */
struct mmal_msg_context {
struct vchiq_mmal_instance *instance;
- u32 handle;
+
+ /* Index in the context_map idr so that we can find the
+* mmal_msg_context again when servicing the VCHI reply.
+*/
+ int handle;
union {
struct {
@@ -149,13 +152,6 @@ struct mmal_msg_context {
};
-struct vchiq_mmal_context_map {
- /* ensure serialized access to the btree(contention should be low) */
- struct mutex lock;
- struct btree_head32 btree_head;
- u32 last_handle;
-};
-
struct vchiq_mmal_instance {
VCHI_SERVICE_HANDLE_T handle;
@@ -165,92 +161,19 @@ struct vchiq_mmal_instance {
/* vmalloc page to receive scratch bulk xfers into */
void *bulk_scratch;
- /* mapping table between context handles and mmal_msg_contexts */
- struct vchiq_mmal_context_map context_map;
+ struct idr context_map;
+ spinlock_t context_map_lock;
/* component to use next */
int component_idx;
struct vchiq_mmal_component component[VCHIQ_MMAL_MAX_COMPONENTS];
};
-static int __must_check
-mmal_context_map_init(struct vchiq_mmal_context_map *context_map)
-{
- mutex_init(&context_map->lock);
- context_map->last_handle = 0;
- return btree_init32(&context_map->btree_head);
-}
-
-static void mmal_context_map_destroy(struct vchiq_mmal_context_map
*context_map)
-{
- mutex_lock(&context_map->lock);
- btree_destroy32(&context_map->btree_head);
- mutex_unlock(&context_map->lock);
-}
-
-static u32
-mmal_context_map_create_handle(struct vchiq_mmal_context_map *context_map,
- struct mmal_msg_context *msg_context,
- gfp_t gfp)
-{
- u32 handle;
-
- mutex_lock(&context_map->lock);
-
- while (1) {
- /* just use a simple count for handles, but do not use 0 */
- context_map->last_handle++;
- if (!context_map->last_handle)
- context_map->last_handle++;
-
- handle = context_map->last_handle;
-
- /* check if the handle is already in use */
- if (!btree_lookup32(&context_map->btree_head, handle))
- break;
- }
-
- if (btree_insert32(&context_map->btree_head, handle,
- msg_context, gfp)) {
- /* probably out of memory */
- mutex_unlock(&context_map->lock);
- return 0;
- }
-
- mutex_unlock(&context_map->lock);
- return handle;
-}
-
-static struct mmal_msg_context *
-mmal_context_map_lookup_handle(struct vchiq_mmal_context_map *context_map,
- u32 handle)
-{
- struct mmal_msg_context *msg_context;
-
- if (!handle)
- return NULL;
-
- mutex_lock(&context_map->lock);
-
- msg_context = btree_lookup32(&context_map->btree_head, handle);
-
- mutex_unlock(&context_map->lock);
- return msg_context;
-}
-
-static void
-mmal_context_map_destroy_handle(struct vchiq_mmal_context_map *context_map,
- u32 handle)
-{
- mutex_lock(&context_map->lock);
- btree_remove32(&context_map->btree_head, handle);
- mutex_unlock(&context_map->lock);
-}
-
static struct mmal_msg_context *
get_msg_context(struct vchiq_mmal_instance *instance)
{
struct mmal_msg_context *msg_context;
+ int handle;
/* todo: should this be allocated from a pool to avoid kzalloc */
msg_context = kzalloc(sizeof(*msg_context), GFP_KERNEL);
@@ -258,32 +181,40 @@ get_msg_context(struct vchiq_mmal_instance *instance)
if (!msg_context)
return ERR_PTR(-ENOMEM);
- msg_context->instance = instance;
- msg_context->handle =
- mmal_context_map_create_handle(&instance->context_map,
- msg_context,
-
[PATCH] staging: bcm2835-camera: Fix module section mismatch warnings.
Noticed by Stephen Rothwell in -next.
Signed-off-by: Eric Anholt
Fixes: 4bebb0312ea9 ("staging/bcm2835-camera: Set ourselves up as a platform
driver.")
Cc: Stephen Rothwell
---
Note: only compile tested here.
.../staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 53f33fb3998b..ce26741ae9d9 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -1548,7 +1548,7 @@ static int set_camera_parameters(struct
vchiq_mmal_instance *instance,
#define MAX_SUPPORTED_ENCODINGS 20
/* MMAL instance and component init */
-static int __init mmal_init(struct bm2835_mmal_dev *dev)
+static int mmal_init(struct bm2835_mmal_dev *dev)
{
int ret;
struct mmal_es_format_local *format;
@@ -1756,8 +1756,8 @@ static int __init mmal_init(struct bm2835_mmal_dev *dev)
return ret;
}
-static int __init bm2835_mmal_init_device(struct bm2835_mmal_dev *dev,
- struct video_device *vfd)
+static int bm2835_mmal_init_device(struct bm2835_mmal_dev *dev,
+ struct video_device *vfd)
{
int ret;
@@ -1836,7 +1836,7 @@ static struct v4l2_format default_v4l2_format = {
.fmt.pix.sizeimage = 1024 * 768,
};
-static int __init bcm2835_mmal_probe(struct platform_device *pdev)
+static int bcm2835_mmal_probe(struct platform_device *pdev)
{
int ret;
struct bm2835_mmal_dev *dev;
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: bcm2835-camera: Replace open-coded idr with a struct idr.
Dan Carpenter writes:
> On Thu, May 10, 2018 at 04:31:07PM -0700, Eric Anholt wrote:
>> @@ -258,32 +181,40 @@ get_msg_context(struct vchiq_mmal_instance *instance)
>> if (!msg_context)
>> return ERR_PTR(-ENOMEM);
>>
>> -msg_context->instance = instance;
>> -msg_context->handle =
>> -mmal_context_map_create_handle(&instance->context_map,
>> - msg_context,
>> - GFP_KERNEL);
>> +/* Create an ID that will be passed along with our message so
>> + * that when we service the VCHI reply, we can look up what
>> + * message is being replied to.
>> + */
>> +spin_lock(&instance->context_map_lock);
>> +handle = idr_alloc(&instance->context_map, msg_context,
>> + 0, 0, GFP_KERNEL);
>> +spin_unlock(&instance->context_map_lock);
>>
>> -if (!msg_context->handle) {
>> +if (msg_context->handle < 0) {
>
> This should probably be testing:
>
> if (handle < 0) {
That's what Stefan said and was fixed in v2 already.
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/13] staging: vc04_services: no need to check debugfs return values
Greg Kroah-Hartman writes:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> Clean up the vchiq_arm code by not caring about the value of debugfs
> calls. This ends up removing a number of lines of code that are not
> needed.
>
> Cc: Eric Anholt
> Cc: Stefan Wahren
> Cc: Kees Cook
> Cc: Dan Carpenter
> Cc: Arnd Bergmann
> Cc: Keerthi Reddy
> Cc: linux-rpi-ker...@lists.infradead.org
> Cc: linux-arm-ker...@lists.infradead.org
> Signed-off-by: Greg Kroah-Hartman
> ---
> .../interface/vchiq_arm/vchiq_arm.c | 13 +---
> .../interface/vchiq_arm/vchiq_debugfs.c | 72 +++
> .../interface/vchiq_arm/vchiq_debugfs.h | 4 +-
> 3 files changed, 15 insertions(+), 74 deletions(-)
>
> diff --git
> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
> index 766b4fe5f32c..103fec955e2c 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
> @@ -314,31 +284,13 @@ void vchiq_debugfs_remove_instance(VCHIQ_INSTANCE_T
> instance)
> debugfs_remove_recursive(node->dentry);
> }
>
> -int vchiq_debugfs_init(void)
> +void vchiq_debugfs_init(void)
> {
> - BUG_ON(debugfs_info.vchiq_cfg_dir != NULL);
> -
> debugfs_info.vchiq_cfg_dir = debugfs_create_dir("vchiq", NULL);
> - if (debugfs_info.vchiq_cfg_dir == NULL)
> - goto fail;
> -
I think now that we allow successful probe with this value NULL, module
remove could vchiq_debugfs_deinit() -> vchiq_debugfs_top() -> BUG_ON().
I think the right solution would be to just drop that BUG_ON(). With
that change (and probably just garbage-collecting that function), this
will be enthusiastically:
Reviewed-by: Eric Anholt
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: vc04_services: no need to check debugfs return values
Greg Kroah-Hartman writes: > When calling debugfs functions, there is no need to ever check the > return value. The function can work or not, but the code logic should > never do something different based on this. > > Clean up the vchiq_arm code by not caring about the value of debugfs > calls. This ends up removing a number of lines of code that are not > needed. This series is: Reviewed-by: Eric Anholt Thanks for the cleanups! signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Bug] VCHIQ functional test broken
Rabin Vincent writes:
> On Thu, Apr 13, 2017 at 11:29:15PM +0100, Russell King - ARM Linux wrote:
>> > 00a19f3e25c0c40e0ec77f52d4841d23ad269169 is the first bad commit
>> > commit 00a19f3e25c0c40e0ec77f52d4841d23ad269169
>> > Author: Rabin Vincent
>> > Date: Tue Nov 8 09:21:19 2016 +0100
>> >
>> > ARM: 8627/1: avoid cache flushing in flush_dcache_page()
>> >
>> > When the data cache is PIPT or VIPT non-aliasing, and cache operations
>> > are broadcast by the hardware, we can always postpone the flush in
>> > flush_dcache_page(). A similar change was done for ARM64 in commit
>> > b5b6c9e9149d ("arm64: Avoid cache flushing in flush_dcache_page()").
>> >
>> > Reviewed-by: Catalin Marinas
>> > Signed-off-by: Rabin Vincent
>> > Signed-off-by: Russell King
>> >
>> > It seems that staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm
>> > relies on the behavior of flush_dcache_page before this patch get
>> > applied. Any advices to fix this issues are appreciated.
>>
>> Any ideas why this causes a problem for this driver? From what I can see,
>> it doesn't make use of flush_dcache_page().
>
> The driver's create_pagelist() uses get_free_pages(), and
> get_free_pages() calls flush_dcache_page().
>
> The problem is that the driver fails to flush the pages which it
> acquires via get_free_pages(). It's clear that the driver needs to do
> it, since there is a flush in the is_vmalloc_addr() path in the same
> function. The driver probably worked earlier because of the unecessary
> flush in flush_dcache_page() which existed before this patch, but the
> purpose of that flush was not DMA coherency and it was never guaranteed
> that it would flush all the way to the point that devices could see the
> data.
>
> See radeon_ttm_tt_pin_userptr() in drivers/gpu/drm/radeon/radeon_ttm.c
> for an example of how a driver can ensure cache coherency using the DMA
> mapping API if it intends to DMA from/to pages acquired by
> get_free_pages().
>
> The rest of the driver should also be converted to the DMA mapping API
> instead of abusing the API's private functions (dmac_map_area etc.)
I'm confused by what you're saying here. The driver has already been
converted to not use dmac_map_area (commit
cf9caf1929882b66922aee698e99e6c8f357bee5), and uses dma_map_sg instead,
matching the radeon driver you give as a model as far as I can see.
That commit is in v4.11-rc6 from Stefan's regression report.
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Bug] VCHIQ functional test broken
Rabin Vincent writes: > On Thu, Apr 20, 2017 at 11:27:38AM -0700, Eric Anholt wrote: >> I'm confused by what you're saying here. The driver has already been >> converted to not use dmac_map_area (commit >> cf9caf1929882b66922aee698e99e6c8f357bee5), and uses dma_map_sg instead, >> matching the radeon driver you give as a model as far as I can see. >> That commit is in v4.11-rc6 from Stefan's regression report. > > Right. Sorry. I must have had an old tag checked out when I looked at > the driver earlier. The DMA API usage in the driver in v4.11-rc6 and > current master looks fine, except for one thing: > > The flush in flush_dcache_page() (from get_user_pages()) was done with a > v6_flush_kern_dcache_page() which always did a clean+invalidate while > the DMA API only does what is required by the direction, which is only a > invalidate for DMA_FROM_DEVICE. Since the driver calls dma_from_sg() on > the entire page, even if userspace sent in an offset into the page, > unrelated data in userspace may be thrown away. > > Does changing the dma API calls to always use DMA_BIDIRECTIONAL make the > test pass? Oh, that's a neat explanation for what might be wrong, and there seem to be tests trying to poke at that within the functional test code. Hopefully Stefan can try that out. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: Fix bracing on single statement blocks
Aaron Moore writes: > Fix coding style issue caught by checkpatch.pl relating to braces on > single statement blocks. This issue was corrected in 3 locations. > > Signed-off-by: Aaron Moore Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations
SF Markus Elfring writes: > From: Markus Elfring > Date: Sat, 31 Dec 2016 22:42:34 +0100 > > Some update suggestions were taken into account > from static source code analysis. This series is: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/9] staging: vc04_services: Clean up vchiq driver
Greg Kroah-Hartman writes: > On Sun, Jan 08, 2017 at 06:15:09PM +, Stefan Wahren wrote: >> This is a loose bunch of clean up patches for the bcm2835 vchiq driver. > > All now applied, thanks. Thanks! I've been off on vacation and wasn't reviewing anything. In general, do you want to see R-Bs from me before applying? I was assuming you did, but I can prioritize reviewing other code if you don't need my reviews for vchi. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 0/3] Add vchi_queue_kernel_message and vchi_queue_user_message
Greg KH writes: > On Wed, Jan 25, 2017 at 05:27:14AM -0800, Michael Zoran wrote: >> On Wed, 2017-01-25 at 16:14 +0300, Dan Carpenter wrote: >> > On Wed, Jan 25, 2017 at 04:33:46AM -0800, Michael Zoran wrote: >> > > Just for clarity, when you mean staging code? Are you talking about >> > > the >> > > entire vc04_services driver or are we talking about the >> > > vchi_msg_queue >> > > function? >> > > >> > > At this point, vc04_services isn't used by anything in the tree at >> > > all. >> > > So if you want all the unused code deleted, well that sounds to me >> > > like you are talking about the whole driver. >> > >> > Yep. Let's either merge the rest of the code or delete the driver. >> > What we have now doesn't work. >> > >> >> That's Greg and Eric's decision. >> >> The vc04_services was already checked into staging when I was asked to >> submit some ARM64 patches to remove some compiler warnings. My goal has >> always been simply to get ARM64 to work, and I can probably accomplish >> that by submitting changes to the RPI specific tree. >> >> I'm just saying that IF the decision was made to continue upstreaming >> these drivers, then I'm willing to continue helping by submitting >> improvements. > > Either they should be merged, or these apis and structures will be > deleted from the in-kernel code, it can't stay as-is. FWIW, I've just I've started on the camera driver. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] Add vchi_queue_kernel_message and vchi_queue_user_message
Michael Zoran writes: > The vchi_msg_queue function which is used by other drivers > to queue a message is difficult to understand and overly > generic. Remove it and replace it with two more specific functions. > > int > vchi_queue_kernel_message(VCHI_SERVICE_HANDLE_T handle, > void *data, > unsigned int size) > > int > vchi_queue_user_message(VCHI_SERVICE_HANDLE_T handle, > void __user *data, > unsigned int size) This is much nicer than the old interface, thanks. Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/6] staging: bcm2835-v4l2: Add a TODO file for improvements we need.
Signed-off-by: Eric Anholt --- drivers/staging/media/platform/bcm2835/TODO | 39 + 1 file changed, 39 insertions(+) create mode 100644 drivers/staging/media/platform/bcm2835/TODO diff --git a/drivers/staging/media/platform/bcm2835/TODO b/drivers/staging/media/platform/bcm2835/TODO new file mode 100644 index ..61a509992b9a --- /dev/null +++ b/drivers/staging/media/platform/bcm2835/TODO @@ -0,0 +1,39 @@ +1) Support dma-buf memory management. + +In order to zero-copy import camera images into the 3D or display +pipelines, we need to export our buffers through dma-buf so that the +vc4 driver can import them. This may involve bringing in the VCSM +driver (which allows long-term management of regions of memory in the +space that the VPU reserved and Linux otherwise doesn't have access +to), or building some new protocol that allows VCSM-style management +of Linux's CMA memory. + +2) Avoid extra copies for padding of images. + +We expose V4L2_PIX_FMT_* formats that have a specified stride/height +padding in the V4L2 spec, but that padding doesn't match what the +hardware can do. If we exposed the native padding requirements +through the V4L2 "multiplanar" formats, the firmware would have one +less copy it needed to do. + +3) Port to ARM64 + +The bulk_receive() does some manual cache flushing that are 32-bit ARM +only, which we should convert to proper cross-platform APIs. + +4) Convert to be a platform driver. + +Right now when the module probes, it tries to initialize VCHI and +errors out if it wasn't ready yet. If bcm2835-v4l2 was built in, then +VCHI generally isn't ready because it depends on both the firmware and +mailbox drivers having already loaded. + +We should have VCHI create a platform device once it's initialized, +and have this driver bind to it, so that we automatically load the +v4l2 module after VCHI loads. + +5) Drop the gstreamer workaround. + +This was a temporary workaround for a bug that was fixed mid-2014, and +we should remove it before stabilizing the driver. + -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
Here's my first pass at importing the camera driver. There's a bunch of TODO left to it, most of which is documented, and the rest being standard checkpatch fare. Unfortunately, when I try modprobing it on my pi3, the USB network device dies, consistently. I'm not sure what's going on here yet, but I'm going to keep working on some debug of it. I've unfortunately changed a lot of variables (pi3 vs pi2, upstream vs downstream, vchi's updates while in staging, 4.9 vs 4.4), so I probably won't figure it out today. Note that the "Update the driver to the current VCHI API" patch will conflict with the outstanding "Add vchi_queue_kernel_message and vchi_queue_user_message" series, but the fix should be pretty obvious when that lands. I built this against 4.10-rc1, but a merge with staging-next was clean and still built fine. Eric Anholt (6): staging: Import the BCM2835 MMAL-based V4L2 camera driver. staging: bcm2835-v4l2: Update the driver to the current VCHI API. staging: bcm2835-v4l2: Add a build system for the module. staging: bcm2835-v4l2: Add a TODO file for improvements we need. staging: bcm2835-v4l2: Apply many whitespace fixes from checkpatch. staging: bcm2835-v4l2: Apply spelling fixes from checkpatch. drivers/staging/media/Kconfig |2 + drivers/staging/media/Makefile |1 + drivers/staging/media/platform/bcm2835/Kconfig | 10 + drivers/staging/media/platform/bcm2835/Makefile| 11 + drivers/staging/media/platform/bcm2835/TODO| 39 + .../media/platform/bcm2835/bcm2835-camera.c| 2024 .../media/platform/bcm2835/bcm2835-camera.h| 145 ++ drivers/staging/media/platform/bcm2835/controls.c | 1335 + .../staging/media/platform/bcm2835/mmal-common.h | 53 + .../media/platform/bcm2835/mmal-encodings.h| 127 ++ .../media/platform/bcm2835/mmal-msg-common.h | 50 + .../media/platform/bcm2835/mmal-msg-format.h | 81 + .../staging/media/platform/bcm2835/mmal-msg-port.h | 107 ++ drivers/staging/media/platform/bcm2835/mmal-msg.h | 404 .../media/platform/bcm2835/mmal-parameters.h | 689 +++ .../staging/media/platform/bcm2835/mmal-vchiq.c| 1920 +++ .../staging/media/platform/bcm2835/mmal-vchiq.h| 178 ++ 17 files changed, 7176 insertions(+) create mode 100644 drivers/staging/media/platform/bcm2835/Kconfig create mode 100644 drivers/staging/media/platform/bcm2835/Makefile create mode 100644 drivers/staging/media/platform/bcm2835/TODO create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.c create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.h create mode 100644 drivers/staging/media/platform/bcm2835/controls.c create mode 100644 drivers/staging/media/platform/bcm2835/mmal-common.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-encodings.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-common.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-format.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-port.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-parameters.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.c create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.h -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.
Generated with checkpatch.pl --fix-inplace and git add -p out of the
results.
Signed-off-by: Eric Anholt
---
drivers/staging/media/platform/bcm2835/bcm2835-camera.c | 6 +++---
drivers/staging/media/platform/bcm2835/mmal-vchiq.c | 12 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
index 4541a363905c..105d88102cd9 100644
--- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
+++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
@@ -1027,7 +1027,7 @@ static int mmal_setup_components(struct bm2835_mmal_dev
*dev,
dev->capture.encode_component = NULL;
}
- /* format dependant port setup */
+ /* format dependent port setup */
switch (mfmt->mmal_component) {
case MMAL_COMPONENT_CAMERA:
/* Make a further decision on port based on resolution */
@@ -1336,7 +1336,7 @@ int vidioc_enum_framesizes(struct file *file, void *fh,
return 0;
}
-/* timeperframe is arbitrary and continous */
+/* timeperframe is arbitrary and continuous */
static int vidioc_enum_frameintervals(struct file *file, void *priv,
struct v4l2_frmivalenum *fival)
{
@@ -1359,7 +1359,7 @@ static int vidioc_enum_frameintervals(struct file *file,
void *priv,
fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
- /* fill in stepwise (step=1.0 is requred by V4L2 spec) */
+ /* fill in stepwise (step=1.0 is required by V4L2 spec) */
fival->stepwise.min = tpf_min;
fival->stepwise.max = tpf_max;
fival->stepwise.step = (struct v4l2_fract) {1, 1};
diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
index cc968442adc4..f71dc3e9c3ae 100644
--- a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
+++ b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
@@ -239,7 +239,7 @@ static int bulk_receive(struct vchiq_mmal_instance
*instance,
pr_err("buffer list empty trying to submit bulk receive\n");
/* todo: this is a serious error, we should never have
-* commited a buffer_to_host operation to the mmal
+* committed a buffer_to_host operation to the mmal
* port without the buffer to back it up (underflow
* handling) and there is no obvious way to deal with
* this - how is the mmal servie going to react when
@@ -352,7 +352,7 @@ static int inline_receive(struct vchiq_mmal_instance
*instance,
pr_err("buffer list empty trying to receive inline\n");
/* todo: this is a serious error, we should never have
-* commited a buffer_to_host operation to the mmal
+* committed a buffer_to_host operation to the mmal
* port without the buffer to back it up (with
* underflow handling) and there is no obvious way to
* deal with this. Less bad than the bulk case as we
@@ -653,7 +653,7 @@ static void service_callback(void *param,
break;
default:
- /* messages dependant on header context to complete */
+ /* messages dependent on header context to complete */
/* todo: the msg.context really ought to be sanity
* checked before we just use it, afaict it comes back
@@ -780,7 +780,7 @@ static void dump_port_info(struct vchiq_mmal_port *port)
port->current_buffer.num,
port->current_buffer.size, port->current_buffer.alignment);
- pr_debug("elementry stream: type:%d encoding:0x%x varient:0x%x\n",
+ pr_debug("elementry stream: type:%d encoding:0x%x variant:0x%x\n",
port->format.type,
port->format.encoding, port->format.encoding_variant);
@@ -883,7 +883,7 @@ static int port_info_set(struct vchiq_mmal_instance
*instance,
return ret;
}
-/* use port info get message to retrive port information */
+/* use port info get message to retrieve port information */
static int port_info_get(struct vchiq_mmal_instance *instance,
struct vchiq_mmal_port *port)
{
@@ -923,7 +923,7 @@ static int port_info_get(struct vchiq_mmal_instance
*instance,
/* copy the values out of the message */
port->handle = rmsg->u.port_info_get_reply.port_handle;
- /* port type and index cached to use on port info set becuase
+ /* port type and index cached to use on port info set because
* it does not use a port handle
*/
port->type = rmsg->u.port_info_get_reply.port_type;
--
2.11.0
[PATCH 2/6] staging: bcm2835-v4l2: Update the driver to the current VCHI API.
49bec49fd7f2 ("staging: vc04_services: remove vchiq_copy_from_user")
removed the flags/msg_handle arguments, which were unused, and pushed
the implementation of copying using memcpy vs copy_from_user to the
caller.
Signed-off-by: Eric Anholt
---
drivers/staging/media/platform/bcm2835/mmal-vchiq.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
index 781322542d5a..24bd2948136c 100644
--- a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
+++ b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
@@ -378,6 +378,14 @@ static int inline_receive(struct vchiq_mmal_instance
*instance,
return 0;
}
+static ssize_t mmal_memcpy_wrapper(void *src, void *dst,
+ size_t offset, size_t size)
+{
+ memcpy(dst + offset, src + offset, size);
+
+ return size;
+}
+
/* queue the buffer availability with MMAL_MSG_TYPE_BUFFER_FROM_HOST */
static int
buffer_from_host(struct vchiq_mmal_instance *instance,
@@ -442,10 +450,9 @@ buffer_from_host(struct vchiq_mmal_instance *instance,
vchi_service_use(instance->handle);
- ret = vchi_msg_queue(instance->handle, &m,
+ ret = vchi_msg_queue(instance->handle, mmal_memcpy_wrapper, &m,
sizeof(struct mmal_msg_header) +
-sizeof(m.u.buffer_from_host),
-VCHI_FLAGS_BLOCK_UNTIL_QUEUED, NULL);
+sizeof(m.u.buffer_from_host));
if (ret != 0) {
release_msg_context(msg_context);
@@ -731,9 +738,9 @@ static int send_synchronous_mmal_msg(struct
vchiq_mmal_instance *instance,
vchi_service_use(instance->handle);
ret = vchi_msg_queue(instance->handle,
+mmal_memcpy_wrapper,
msg,
-sizeof(struct mmal_msg_header) + payload_len,
-VCHI_FLAGS_BLOCK_UNTIL_QUEUED, NULL);
+sizeof(struct mmal_msg_header) + payload_len);
vchi_service_release(instance->handle);
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] staging: bcm2835-v4l2: Apply many whitespace fixes from checkpatch.
Generated with checkpatch.pl --fix-inplace, some manual fixes for
cases where checkpatch fixed one out of multiple lines of mis-indented
function parameters, and then git add -p out of the results.
I skipped some fixes that should probably instead be replaced with the
BIT() macro.
Signed-off-by: Eric Anholt
---
.../media/platform/bcm2835/bcm2835-camera.c| 90
drivers/staging/media/platform/bcm2835/controls.c | 236 ++---
.../staging/media/platform/bcm2835/mmal-vchiq.c| 13 +-
3 files changed, 167 insertions(+), 172 deletions(-)
diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
index 4f03949aecf3..4541a363905c 100644
--- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
+++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
@@ -38,7 +38,7 @@
#define BM2835_MMAL_MODULE_NAME "bcm2835-v4l2"
#define MIN_WIDTH 32
#define MIN_HEIGHT 32
-#define MIN_BUFFER_SIZE (80*1024)
+#define MIN_BUFFER_SIZE (80 * 1024)
#define MAX_VIDEO_MODE_WIDTH 1280
#define MAX_VIDEO_MODE_HEIGHT 720
@@ -420,6 +420,7 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
static int enable_camera(struct bm2835_mmal_dev *dev)
{
int ret;
+
if (!dev->camera_use_count) {
ret = vchiq_mmal_port_parameter_set(
dev->instance,
@@ -451,6 +452,7 @@ static int enable_camera(struct bm2835_mmal_dev *dev)
static int disable_camera(struct bm2835_mmal_dev *dev)
{
int ret;
+
if (!dev->camera_use_count) {
v4l2_err(&dev->v4l2_dev,
"Disabled the camera when already disabled\n");
@@ -459,6 +461,7 @@ static int disable_camera(struct bm2835_mmal_dev *dev)
dev->camera_use_count--;
if (!dev->camera_use_count) {
unsigned int i = 0x;
+
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
"Disabling camera\n");
ret =
@@ -643,12 +646,14 @@ static void stop_streaming(struct vb2_queue *vq)
static void bm2835_mmal_lock(struct vb2_queue *vq)
{
struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vq);
+
mutex_lock(&dev->mutex);
}
static void bm2835_mmal_unlock(struct vb2_queue *vq)
{
struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vq);
+
mutex_unlock(&dev->mutex);
}
@@ -737,7 +742,7 @@ static int vidioc_try_fmt_vid_overlay(struct file *file,
void *priv,
1, 0);
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
- "Overlay: Now w/h %dx%d l/t %dx%d\n",
+"Overlay: Now w/h %dx%d l/t %dx%d\n",
f->fmt.win.w.width, f->fmt.win.w.height,
f->fmt.win.w.left, f->fmt.win.w.top);
@@ -759,7 +764,7 @@ static int vidioc_s_fmt_vid_overlay(struct file *file, void
*priv,
dev->overlay = f->fmt.win;
if (dev->component[MMAL_COMPONENT_PREVIEW]->enabled) {
set_overlay_params(dev,
- &dev->component[MMAL_COMPONENT_PREVIEW]->input[0]);
+
&dev->component[MMAL_COMPONENT_PREVIEW]->input[0]);
}
return 0;
@@ -771,6 +776,7 @@ static int vidioc_overlay(struct file *file, void *f,
unsigned int on)
struct bm2835_mmal_dev *dev = video_drvdata(file);
struct vchiq_mmal_port *src;
struct vchiq_mmal_port *dst;
+
if ((on && dev->component[MMAL_COMPONENT_PREVIEW]->enabled) ||
(!on && !dev->component[MMAL_COMPONENT_PREVIEW]->enabled))
return 0; /* already in requested state */
@@ -842,7 +848,7 @@ static int vidioc_g_fbuf(struct file *file, void *fh,
a->fmt.pixelformat = V4L2_PIX_FMT_YUV420;
a->fmt.bytesperline = preview_port->es.video.width;
a->fmt.sizeimage = (preview_port->es.video.width *
- preview_port->es.video.height * 3)>>1;
+ preview_port->es.video.height * 3) >> 1;
a->fmt.colorspace = V4L2_COLORSPACE_SMPTE170M;
return 0;
@@ -958,8 +964,8 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void
*priv,
f->fmt.pix.field = V4L2_FIELD_NONE;
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
- "Clipping/aligning %dx%d format %08X\n",
- f->fmt.pix.width, f->fmt.pix.height, f->fmt.pix.pixelformat);
+"Clipping/aligning %dx%d format %08X\n",
+f->fmt.pix.width, f->fmt.pix.height, f->fmt.pix.pixelformat);
v4l_bound_align_image(&f->fmt.pix.width, MIN_WIDTH, dev->max_width, 1,
&f
[PATCH 3/6] staging: bcm2835-v4l2: Add a build system for the module.
This is derived from the downstream tree's build system, but with just a single Kconfig option. For now the driver only builds on 32-bit arm -- the aarch64 build breaks due to the driver using arm-specific cache flushing functions. Signed-off-by: Eric Anholt --- drivers/staging/media/Kconfig | 2 ++ drivers/staging/media/Makefile | 1 + drivers/staging/media/platform/bcm2835/Kconfig | 10 ++ drivers/staging/media/platform/bcm2835/Makefile | 11 +++ 4 files changed, 24 insertions(+) create mode 100644 drivers/staging/media/platform/bcm2835/Kconfig create mode 100644 drivers/staging/media/platform/bcm2835/Makefile diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig index ffb8fa72c3da..abd0e2d57c20 100644 --- a/drivers/staging/media/Kconfig +++ b/drivers/staging/media/Kconfig @@ -27,6 +27,8 @@ source "drivers/staging/media/davinci_vpfe/Kconfig" source "drivers/staging/media/omap4iss/Kconfig" +source "drivers/staging/media/platform/bcm2835/Kconfig" + source "drivers/staging/media/s5p-cec/Kconfig" # Keep LIRC at the end, as it has sub-menus diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile index a28e82cf6447..dc89325c463d 100644 --- a/drivers/staging/media/Makefile +++ b/drivers/staging/media/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_I2C_BCM2048) += bcm2048/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/ obj-$(CONFIG_DVB_CXD2099) += cxd2099/ obj-$(CONFIG_LIRC_STAGING) += lirc/ +obj-$(CONFIG_VIDEO_BCM2835)+= platform/bcm2835/ obj-$(CONFIG_VIDEO_DM365_VPFE) += davinci_vpfe/ obj-$(CONFIG_VIDEO_OMAP4) += omap4iss/ obj-$(CONFIG_VIDEO_STI_HDMI_CEC) += st-cec/ diff --git a/drivers/staging/media/platform/bcm2835/Kconfig b/drivers/staging/media/platform/bcm2835/Kconfig new file mode 100644 index ..7c5245dc3225 --- /dev/null +++ b/drivers/staging/media/platform/bcm2835/Kconfig @@ -0,0 +1,10 @@ +config VIDEO_BCM2835 + tristate "Broadcom BCM2835 camera driver" + depends on VIDEO_V4L2 && (ARCH_BCM2835 || COMPILE_TEST) + depends on BCM2835_VCHIQ + depends on ARM + select VIDEOBUF2_VMALLOC + help + Say Y here to enable camera host interface devices for + Broadcom BCM2835 SoC. This operates over the VCHIQ interface + to a service running on VideoCore. diff --git a/drivers/staging/media/platform/bcm2835/Makefile b/drivers/staging/media/platform/bcm2835/Makefile new file mode 100644 index ..d7900a5951a8 --- /dev/null +++ b/drivers/staging/media/platform/bcm2835/Makefile @@ -0,0 +1,11 @@ +bcm2835-v4l2-$(CONFIG_VIDEO_BCM2835) := \ + bcm2835-camera.o \ + controls.o \ + mmal-vchiq.o + +obj-$(CONFIG_VIDEO_BCM2835) += bcm2835-v4l2.o + +ccflags-y += \ + -Idrivers/staging/vc04_services \ + -Idrivers/staging/vc04_services/interface/vcos/linuxkernel \ + -D__VCCOREVER__=0x0400 -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: bcm2835-audio: Strengthen build dependencies
Michael Zoran writes: > This driver makes no sense outside of ARM or ARM64. > Add an explicit build dependency on: > (ARM || ARM64 || COMPILE_TEST) > > Also set the default build to n > > Signed-off-by: Michael Zoran > --- > drivers/staging/bcm2835-audio/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/staging/bcm2835-audio/Kconfig > b/drivers/staging/bcm2835-audio/Kconfig > index 32a2ff9ef9b2..840faa21f665 100644 > --- a/drivers/staging/bcm2835-audio/Kconfig > +++ b/drivers/staging/bcm2835-audio/Kconfig > @@ -1,6 +1,8 @@ > config SND_BCM2835 > tristate "BCM2835 ALSA driver" > depends on ARCH_BCM2835 && BCM2835_VCHIQ && SND > + depends on (ARM || ARM64 || COMPILE_TEST) > + default n > select SND_PCM > help >Say Y or M if you want to support BCM2835 Alsa pcm card driver We've already got a DEPENDS on ARCH_BCM2835, how would that be present without one of those three being set? signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.
Joe Perches writes:
> On Fri, 2017-01-27 at 13:55 -0800, Eric Anholt wrote:
>> Generated with checkpatch.pl --fix-inplace and git add -p out of the
>> results.
>
> Maybe another.
>
>> diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
>> b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
> []
>> @@ -239,7 +239,7 @@ static int bulk_receive(struct vchiq_mmal_instance
>> *instance,
>> pr_err("buffer list empty trying to submit bulk receive\n");
>>
>> /* todo: this is a serious error, we should never have
>> - * commited a buffer_to_host operation to the mmal
>> + * committed a buffer_to_host operation to the mmal
>> * port without the buffer to back it up (underflow
>> * handling) and there is no obvious way to deal with
>> * this - how is the mmal servie going to react when
>
> Perhaps s/servie/service/ ?
I was trying to restrict this patch to just the fixes from checkpatch.
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.
Joe Perches writes:
> On Mon, 2017-01-30 at 12:05 -0800, Eric Anholt wrote:
>> Joe Perches writes:
>>
>> > On Fri, 2017-01-27 at 13:55 -0800, Eric Anholt wrote:
>> > > Generated with checkpatch.pl --fix-inplace and git add -p out of the
>> > > results.
>> >
>> > Maybe another.
>> >
>> > > diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
>> > > b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
>> >
>> > []
>> > > @@ -239,7 +239,7 @@ static int bulk_receive(struct vchiq_mmal_instance
>> > > *instance,
>> > > pr_err("buffer list empty trying to submit bulk
>> > > receive\n");
>> > >
>> > > /* todo: this is a serious error, we should never have
>> > > - * commited a buffer_to_host operation to the mmal
>> > > + * committed a buffer_to_host operation to the mmal
>> > > * port without the buffer to back it up (underflow
>> > > * handling) and there is no obvious way to deal with
>> > > * this - how is the mmal servie going to react when
>> >
>> > Perhaps s/servie/service/ ?
>>
>> I was trying to restrict this patch to just the fixes from checkpatch.
>
> That's the wrong thing to do if you're fixing
> spelling defects. checkpatch is just one mechanism
> to identify some, and definitely not all, typos and
> spelling defects.
>
> If you fixing, fix. Don't just rely on the brainless
> tools, use your decidedly non-mechanical brain.
"if you touch anything, you must fix everything." If that's how things
work, I would just retract the patch.
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: remove unused functions
Dan Carpenter writes: > There is a bunch of vc04_services that we're still looking to merge in > the near future. Please hold off deleting these until we are further > along on that. Checking the downstream tree, these are actually dead. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: vc04_services: Replace dmac_map_area with dmac_map_sg
Greg KH writes: > On Sun, Oct 23, 2016 at 10:29:32PM -0700, mzo...@crowfest.net wrote: >> From: Michael Zoran >> >> The original arm implementation uses dmac_map_area which is not >> portable. Replace it with an architecture neutral version >> which uses dma_map_sg. >> >> As you can see that for larger page sizes, the dma_map_sg >> implementation is faster then the original unportable dma_map_area >> implementation. >> >> Test dmac_map_area dma_map_page dma_map_sg >> vchiq_test -b 4 1 51us/iter 76us/iter76us >> vchiq_test -b 8 1 70us/iter 82us/iter91us >> vchiq_test -b 16 1 94us/iter 118us/iter 121us >> vchiq_test -b 32 1 146us/iter 173us/iter 187us >> vchiq_test -b 64 1 263us/iter 328us/iter 299us >> vchiq_test -b 128 1529us/iter 631us/iter 595us >> vchiq_test -b 256 12285us/iter 2275us/iter 2001us >> vchiq_test -b 512 14372us/iter 4616us/iter 4123us >> >> For message sizes >= 64KB, dma_map_sg is faster then dma_map_page. >> >> For message size >= 256KB, the dma_map_sg is the fastest >> implementation. > > What is the "normal" message size value when using this driver? For the contents of a bulk transfer, the performance case you care about is moving an image (video decode, camera, etc.), so on the order of 1MB. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: vc04_services: Replace dmac_map_area with dmac_map_sg
mzo...@crowfest.net writes:
> From: Michael Zoran
>
> The original arm implementation uses dmac_map_area which is not
> portable. Replace it with an architecture neutral version
> which uses dma_map_sg.
>
> As you can see that for larger page sizes, the dma_map_sg
> implementation is faster then the original unportable dma_map_area
> implementation.
>
> Test dmac_map_area dma_map_page dma_map_sg
> vchiq_test -b 4 1 51us/iter 76us/iter76us
> vchiq_test -b 8 1 70us/iter 82us/iter91us
> vchiq_test -b 16 1 94us/iter 118us/iter 121us
> vchiq_test -b 32 1 146us/iter 173us/iter 187us
> vchiq_test -b 64 1 263us/iter 328us/iter 299us
> vchiq_test -b 128 1529us/iter 631us/iter 595us
> vchiq_test -b 256 12285us/iter 2275us/iter 2001us
> vchiq_test -b 512 14372us/iter 4616us/iter 4123us
>
> For message sizes >= 64KB, dma_map_sg is faster then dma_map_page.
>
> For message size >= 256KB, the dma_map_sg is the fastest
> implementation.
>
> Signed-off-by: Michael Zoran
> ---
> .../interface/vchiq_arm/vchiq_2835_arm.c | 153
> -
> 1 file changed, 91 insertions(+), 62 deletions(-)
>
> diff --git
> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index 98c6819..5ca66ee 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -45,13 +45,8 @@
> #include
> #include
>
> -#define dmac_map_area__glue(_CACHE,_dma_map_area)
> -#define dmac_unmap_area __glue(_CACHE,_dma_unmap_area)
> -
> #define TOTAL_SLOTS (VCHIQ_SLOT_ZERO_SLOTS + 2 * 32)
>
> -#define VCHIQ_ARM_ADDRESS(x) ((void *)((char *)x + g_virt_to_bus_offset))
> -
> #include "vchiq_arm.h"
> #include "vchiq_2835.h"
> #include "vchiq_connected.h"
> @@ -73,7 +68,7 @@ static unsigned int g_fragments_size;
> static char *g_fragments_base;
> static char *g_free_fragments;
> static struct semaphore g_free_fragments_sema;
> -static unsigned long g_virt_to_bus_offset;
> +static struct device *g_dev;
>
> extern int vchiq_arm_log_level;
>
> @@ -84,10 +79,11 @@ vchiq_doorbell_irq(int irq, void *dev_id);
>
> static int
> create_pagelist(char __user *buf, size_t count, unsigned short type,
> -struct task_struct *task, PAGELIST_T ** ppagelist);
> + struct task_struct *task, PAGELIST_T **ppagelist,
> + dma_addr_t *dma_addr);
>
> static void
> -free_pagelist(PAGELIST_T *pagelist, int actual);
> +free_pagelist(dma_addr_t dma_addr, PAGELIST_T *pagelist, int actual);
>
> int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
> {
> @@ -101,8 +97,6 @@ int vchiq_platform_init(struct platform_device *pdev,
> VCHIQ_STATE_T *state)
> int slot_mem_size, frag_mem_size;
> int err, irq, i;
>
> - g_virt_to_bus_offset = virt_to_dma(dev, (void *)0);
> -
> (void)of_property_read_u32(dev->of_node, "cache-line-size",
> &g_cache_line_size);
> g_fragments_size = 2 * g_cache_line_size;
> @@ -118,7 +112,7 @@ int vchiq_platform_init(struct platform_device *pdev,
> VCHIQ_STATE_T *state)
> return -ENOMEM;
> }
>
> - WARN_ON(((int)slot_mem & (PAGE_SIZE - 1)) != 0);
> + WARN_ON(((unsigned long)slot_mem & (PAGE_SIZE - 1)) != 0);
>
> vchiq_slot_zero = vchiq_init_slots(slot_mem, slot_mem_size);
> if (!vchiq_slot_zero)
> @@ -170,6 +164,7 @@ int vchiq_platform_init(struct platform_device *pdev,
> VCHIQ_STATE_T *state)
> return err ? : -ENXIO;
> }
>
> + g_dev = dev;
> vchiq_log_info(vchiq_arm_log_level,
> "vchiq_init - done (slots %pK, phys %pad)",
> vchiq_slot_zero, &slot_phys);
> @@ -233,6 +228,7 @@ vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk,
> VCHI_MEM_HANDLE_T memhandle,
> {
> PAGELIST_T *pagelist;
> int ret;
> + dma_addr_t dma_addr;
>
> WARN_ON(memhandle != VCHI_MEM_HANDLE_INVALID);
>
> @@ -241,12 +237,14 @@ vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk,
> VCHI_MEM_HANDLE_T memhandle,
> ? PAGELIST_READ
> : PAGELIST_WRITE,
> current,
> - &pagelist);
> + &pagelist,
> + &dma_addr);
> +
> if (ret != 0)
> return VCHIQ_ERROR;
>
> bulk->handle = memhandle;
> - bulk->data = VCHIQ_ARM_ADDRESS(pagelist);
> + bulk->data = (void *)(unsigned long)dma_addr;
>
> /* Store the pagelist address in remote_data, which isn't used by the
> slave. */
> @@ -259,7 +257,8 @@ void
> vchiq_complete_bulk(VCHIQ_BULK_T *bulk)
> {
> if (bulk && bulk->remote_data && bulk->
Re: [PATCH 2/2] staging: vc04_services: Replace dmac_map_area with dmac_map_sg
Michael Zoran writes:
> On Tue, 2016-10-25 at 08:00 -0700, Michael Zoran wrote:
>> On Mon, 2016-10-24 at 10:31 -0700, Eric Anholt wrote:
>> > mzo...@crowfest.net writes:
>> >
>> > > */
>> > >
>> > > static int
>> > > create_pagelist(char __user *buf, size_t count, unsigned short
>> > > type,
>> > > -struct task_struct *task, PAGELIST_T ** ppagelist)
>> > > +struct task_struct *task, PAGELIST_T
>> > > **ppagelist,
>> > > +dma_addr_t *dma_addr)
>> > > {
>> > > PAGELIST_T *pagelist;
>> > > struct page **pages;
>> > > -unsigned long *addrs;
>> > > -unsigned int num_pages, offset, i;
>> > > -char *addr, *base_addr, *next_addr;
>> > > -int run, addridx, actual_pages;
>> > > +u32 *addrs;
>> > > +unsigned int num_pages, offset, i, j, k;
>> > > +int actual_pages;
>> > > unsigned long *need_release;
>> > > +size_t pagelist_size;
>> > > +struct scatterlist *scatterlist;
>> > > +int dma_buffers;
>> > > +int dir;
>> > >
>> > > -offset = (unsigned int)buf & (PAGE_SIZE - 1);
>> > > +offset = ((unsigned int)(unsigned long)buf & (PAGE_SIZE
>> > > -
>> > > 1));
>> > > num_pages = (count + offset + PAGE_SIZE - 1) /
>> > > PAGE_SIZE;
>> > >
>> > > +pagelist_size = sizeof(PAGELIST_T) +
>> > > +(num_pages * sizeof(unsigned long)) +
>> > > +sizeof(unsigned long) +
>> > > +(num_pages * sizeof(pages[0]) +
>> > > +(num_pages * sizeof(struct
>> > > scatterlist)));
>> > > +
>> > > *ppagelist = NULL;
>> > >
>> > > +dir = (type == PAGELIST_WRITE) ? DMA_TO_DEVICE :
>> > > DMA_FROM_DEVICE;
>> > > +
>> > > /* Allocate enough storage to hold the page pointers and
>> > > the page
>> > > ** list
>> > > */
>> > > -pagelist = kmalloc(sizeof(PAGELIST_T) +
>> > > - (num_pages * sizeof(unsigned long)) +
>> > > - sizeof(unsigned long) +
>> > > - (num_pages * sizeof(pages[0])),
>> > > - GFP_KERNEL);
>> > > +pagelist = dma_zalloc_coherent(g_dev,
>> > > + pagelist_size,
>> > > + dma_addr,
>> > > + GFP_KERNEL);
>> > >
>> > > vchiq_log_trace(vchiq_arm_log_level, "create_pagelist -
>> > > %pK",
>> > > pagelist);
>> > > @@ -394,10 +399,9 @@ create_pagelist(char __user *buf, size_t
>> > > count, unsigned short type,
>> > > addrs = pagelist->addrs;
>> > > need_release = (unsigned long *)(addrs + num_pages);
>> > > pages = (struct page **)(addrs + num_pages + 1);
>> > > +scatterlist = (struct scatterlist *)(pages + num_pages);
>> > >
>> > > if (is_vmalloc_addr(buf)) {
>> > > -int dir = (type == PAGELIST_WRITE) ?
>> > > -DMA_TO_DEVICE : DMA_FROM_DEVICE;
>> > > unsigned long length = count;
>> > > unsigned int off = offset;
>> > >
>> > > @@ -410,7 +414,6 @@ create_pagelist(char __user *buf, size_t
>> > > count,
>> > > unsigned short type,
>> > > if (bytes > length)
>> > > bytes = length;
>> > > pages[actual_pages] = pg;
>> > > -dmac_map_area(page_address(pg) + off,
>> > > bytes, dir);
>> > > length -= bytes;
>> > > off = 0;
>> > > }
>> > > @@ -438,7 +441,8 @@ create_pagelist(char __user *buf, size_t
>> > > count,
>> > > unsigned short type,
>> > >
Re: [PATCH v2] staging: vc04_services: Replace dmac_map_area with dmac_map_sg
Michael Zoran writes: > The original arm implementation uses dmac_map_area which is not > portable. Replace it with an architecture neutral version > which uses dma_map_sg. > > As you can see that for larger page sizes, the dma_map_sg > implementation is faster then the original unportable dma_map_area > implementation. > > Test dmac_map_area dma_map_page dma_map_sg > vchiq_test -b 4 1 51us/iter 76us/iter76us > vchiq_test -b 8 1 70us/iter 82us/iter91us > vchiq_test -b 16 1 94us/iter 118us/iter 121us > vchiq_test -b 32 1 146us/iter 173us/iter 187us > vchiq_test -b 64 1 263us/iter 328us/iter 299us > vchiq_test -b 128 1529us/iter 631us/iter 595us > vchiq_test -b 256 12285us/iter 2275us/iter 2001us > vchiq_test -b 512 1 4372us/iter 4616us/iter 4123us Reviewed-by: Eric Anholt Nice work! More portability and better performance at the same time. A possible future improvement would be to track the pagelist, num_pages, and pagelist_size in a struct in the bulk->remote_data so we didn't need to recalculate them at free time. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/5] staging: vc04_services: fix CamelCase
Stefan Wahren writes: > This fixes the CamelCase of some variables. > > Signed-off-by: Stefan Wahren Greg, are these kinds of reformats something that you're looking for in the staging tree? I'm fine with the patch but I didn't see camel case explicitly called out in CodingStyle after a brief search. Other than this concern, all 5 patches are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: call sg_init_table to init scatterlist
Michael Zoran writes: > Call the sg_init_table function to correctly initialze > the DMA scatterlist. This function is required to completely > initialize the list and is mandatory if DMA debugging is > enabled in the build configuration. > > One of the purposes of sg_init_table is to set > the magic "cookie" on each list element and ensure > the chain end is marked. > > Signed-off-by: Michael Zoran > --- > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c > index 6fa2b5a..21b26e5 100644 > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c > @@ -464,6 +464,12 @@ create_pagelist(char __user *buf, size_t count, unsigned > short type, > pagelist->type = type; > pagelist->offset = offset; > > + /* > + * Initialize the scatterlist so that the magic cookie > + * is filled if debugging is enabled > + */ > + sg_init_table(scatterlist, num_pages); > + /* Now set the pages for each scatterlist */ I feel like the comments don't add much, but either way: Acked-by: Eric Anholt > for (i = 0; i < num_pages; i++) > sg_set_page(scatterlist + i, pages[i], PAGE_SIZE, 0); > > -- > 2.10.1 signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: setup DMA and coherent mask
Michael Zoran writes: > Setting the DMA mask is optional on 32 bit but > is mandatory on 64 bit. Set the DMA mask and coherent > to force all DMA to be in the 32 bit address space. > > This is considered a "good practice" and most drivers > already do this. > > Signed-off-by: Michael Zoran > --- > .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 10 > ++ > 1 file changed, 10 insertions(+) > > diff --git > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c > index a5afcc5..6fa2b5a 100644 > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c > @@ -97,6 +97,16 @@ int vchiq_platform_init(struct platform_device *pdev, > VCHIQ_STATE_T *state) > int slot_mem_size, frag_mem_size; > int err, irq, i; > > + /* > + * Setting the DMA mask is necessary in the 64 bit environment. > + * It isn't necessary in a 32 bit environment but is considered > + * a good practice. > + */ > + err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); I think a better comment here would be simply: /* VCHI messages between the CPU and firmware use 32-bit bus addresses. */ explaining why the value is chosen (once you know that the 32 bit restriction exists, reporting it is obviously needed). I'm curious, though: what failed when you didn't set it? > + > + if (err < 0) > + return err; > + > (void)of_property_read_u32(dev->of_node, "cache-line-size", > &g_cache_line_size); > g_fragments_size = 2 * g_cache_line_size; > -- > 2.10.1 signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: parse_rx_slots() - Fix compiler warning
Michael Zoran writes: > vc04_services contains a debug logging mechanism. The log is > maintained in a shared memory area between the kernel and the > firmware. Changing the sizes of the data in this area would > require a firmware change which is distributed independently > from the kernel binary. > > One of the items logged is the address of received messages. > This address is a pointer, but the debugging slot used to store > the information is a 32 bit integer. > > Luckily, this value is never interpreted by anything other > then debug tools and it is expected that a human debugging > the kernel interpret it. > > This change adds a cast to long before the original cast > to int to silence the warning. > > Signed-off-by: Michael Zoran Thanks for sorting this out. Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: vc04_services: add vchiq_pagelist_info structure
Michael Zoran writes: > The current dma_map_sg based implementation for bulk messages > computes many offsets into a single allocation multiple times in > both the create and free code paths. This is inefficient, > error prone and in fact still has a few lingering issues > with arm64. > > This change replaces a small portion of that inplementation with > new code that uses a new struct vchiq_pagelist_info to store the > needed information rather then complex offset calculations. > > This improved implementation should be more efficient and easier > to understand and maintain. > > Tests Run(Both Pass): > vchiq_test -p 1 > vchiq_test -f 10 Looks good, and it's a nice cleanup. Thanks! Reviewed-by: Eric Anholt I had one style note, which was that you're using an int and 0/1 for a boolean value, but we like to use proper bools and true/false instead. However, you're modifying code that was already using an int for related booleans, so that can be a separate cleanup. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: vc04_services: setup DMA and coherent mask
Michael Zoran writes: > VCHI messages between the CPU and firmware use 32-bit > bus addresses. Explicitly set the DMA mask and coherent > on all platforms. Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: fix array_size.cocci warnings
kbuild test robot writes: > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:193:39-40: > WARNING: Use ARRAY_SIZE > > Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element > > Semantic patch information: > This makes an effort to find cases where ARRAY_SIZE can be used such as > where there is a division of sizeof the array by the sizeof its first > element or by any indexed element or the element type. It replaces the > division of the two sizeofs by ARRAY_SIZE. > > Generated by: scripts/coccinelle/misc/array_size.cocci > > Signed-off-by: Fengguang Wu These 3 are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: fix ifnullfree.cocci warnings
kbuild test robot writes: > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_util.c:65:2-7: > WARNING: NULL check before freeing functions like kfree, debugfs_remove, > debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider > reorganizing relevant code to avoid passing NULL values. > > NULL check before some freeing functions is not needed. > > Based on checkpatch warning > "kfree(NULL) is safe this check is probably not required" > and kfreeaddr.cocci by Julia Lawall. > > Generated by: scripts/coccinelle/free/ifnullfree.cocci > > Signed-off-by: Fengguang Wu Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [bug report] staging: add bcm2708 vchiq driver
Michael Zoran writes: > On Tue, 2016-11-15 at 22:04 -0500, Vince Weaver wrote: >> On Tue, 15 Nov 2016, Michael Zoran wrote: >> >> > I'm still interested to know more about the MMU statement. I would >> > think at least some of the RPI models have one because swapping >> > appears >> > to be at least somewhat functional on the later models. >> >> All Raspberry Pi models have an MMU. I'm not sure why anyone thinks >> otherwise. >> >> Though just to be pedantic, you don't need an MMU to "swap". In >> general >> you do to "page". >> >> Vince > > From the BCM2835 doc floating around the web the RPI should have 2 > MMUs. One on the VC side and one on the ARM side. Since we are being > pedantic here, I'll ask the question this way. > > Do all RPIs has an enabled MMU on the ARM side and do all RPIs support > paging? Yes. The MMU between ARM's virtual and ARM's physical memory addresses is always present and is totally normal. The second MMU maps between ARM's physical and bus, and is too coarse to be of much use so it's almost a 1:1 mapping. When people talk about RPi lacking an MMU, they're talking about between devices (such as the 3D engine, video decode/encode, VPU, etc.) and memory. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: vc04_services: remove duplicate mutex_lock_interruptible
Arnd Bergmann writes: > The driver tries to redefine mutex_lock_interruptible as an open-coded > mutex_lock_killable, but that definition clashes with the normal > mutex_lock_interruptible definition when CONFIG_DEBUG_LOCK_ALLOC > is set: These two are: Acked-by: Eric Anholt signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 06/24] staging: vc04_services: bcm2835-audio: remove redundant license text
Greg Kroah-Hartman writes: > Now that the SPDX tag is in all > drivers/staging/vc04_services/bcm2835-audio/ files, that identifies the > license in a specific and legally-defined manner. So the extra GPL text > wording can be removed as it is no longer needed at all. > > This is done on a quest to remove the 700+ different ways that files in > the kernel describe the GPL license text. And there's unneeded stuff > like the address (sometimes incorrect) for the FSF which is never > needed. > > No copyright headers or other non-license-description text was removed. > diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c > b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c > index 4ad67e64e6b9..ec468d5719b1 100644 > --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c > +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c > @@ -1,17 +1,5 @@ > // SPDX-License-Identifier: GPL-2.0 > -/* > - * Copyright 2011 Broadcom Corporation. All rights reserved. > - * > - * Unless you and Broadcom execute a separate written software license > - * agreement governing use of this software, this software is licensed to you > - * under the terms of the GNU General Public License version 2, available at > - * http://www.broadcom.com/licenses/GPLv2.php (the "GPL"). > - * > - * Notwithstanding the above, under no circumstances may you combine this > - * software in any way with any other Broadcom software provided under a > - * license other than the GPL, without Broadcom's express prior written > - * consent. > - > */ > +/* Copyright 2011 Broadcom Corporation. All rights reserved. */ The "Notwithstanding..." section feels like something unusual and not represented by the SPDX identifier. Not sure what to do about that. The 2 camera patches are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vchiq_arm: Clear VLA warning
"Tobin C. Harding" writes: > The kernel would like to have all stack VLA usage removed[1]. The > array here is fixed (declared with a const variable) but it appears > like VLA to the compiler. We can use a pre-processor define to quiet > the compiler. > > [1]: https://lkml.org/lkml/2018/3/7/621 > > Signed-off-by: Tobin C. Harding > --- > > The name of this constant may need changing, there is already a > pre-processor constant VCHIQ_MAX_SERVICES Maybe just use ARRAY_SIZE(local_max_services) and not have the #define? signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: vc04_services: Add outstanding VCHI TODOs
Stefan Wahren writes: > The TODO list missed some issues before we can move the driver out of staging. > > Signed-off-by: Stefan Wahren > --- > drivers/staging/vc04_services/interface/vchi/TODO | 27 > +++ > 1 file changed, 27 insertions(+) > > diff --git a/drivers/staging/vc04_services/interface/vchi/TODO > b/drivers/staging/vc04_services/interface/vchi/TODO > index 84e6733..7144de2 100644 > --- a/drivers/staging/vc04_services/interface/vchi/TODO > +++ b/drivers/staging/vc04_services/interface/vchi/TODO > @@ -23,3 +23,30 @@ there's a lot code that got built that's probably > unnecessary these > days. Once we have the set of VCHI-using drivers we want in tree, we > should be able to do a sweep of the code to see what's left that's > unused. > + > +3) Make driver more portable > + > +Building this driver with arm/multi_v7_defconfig or arm64/defconfig > +leads to data corruption during vchiq_test. This should be fixed. Sounds good, but could you document that it's "vchiq_test -f" for testing this? With that, these two get my: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
