Re: [PATCH 1/2] [media] v4l2-mem2mem: set the queue owner field just as vb2_ioctl_reqbufs does
Hi Philipp, On 25/06/15 17:12, Philipp Zabel wrote: Am Donnerstag, den 25.06.2015, 15:11 +0200 schrieb Sylwester Nawrocki: On 25/06/15 12:01, Philipp Zabel wrote: Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/v4l2-core/v4l2-mem2mem.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c index dc853e5..511caaa 100644 --- a/drivers/media/v4l2-core/v4l2-mem2mem.c +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c @@ -357,9 +357,16 @@ int v4l2_m2m_reqbufs(struct file *file, struct v4l2_m2m_ctx *m2m_ctx, struct v4l2_requestbuffers *reqbufs) { struct vb2_queue *vq; + int ret; vq = v4l2_m2m_get_vq(m2m_ctx, reqbufs-type); - return vb2_reqbufs(vq, reqbufs); + ret = vb2_reqbufs(vq, reqbufs); + /* If count == 0, then the owner has released all buffers and he + is no longer owner of the queue. Otherwise we have a new owner. */ + if (ret == 0) + vq-owner = reqbufs-count ? file-private_data : NULL; + + return ret; } How about modifying v4l2_m2m_ioctl_reqbufs() instead ? The coda, gsc-m2m, m2m-deinterlace, mx2_emmaprp, and sh_veu drivers all have their own implementation of vidioc_reqbufs that call v4l2_m2m_reqbufs directly. Maybe this should be moved into v4l2_m2m_ioctl_reqbufs after all drivers are updated to use it instead of v4l2_m2m_reqbufs. In case of some of the above listed drivers it shouldn't be difficult and would be nice to convert to the generic v4l2_m2m_ioctl* callbacks. Anyway, I guess your code change makes sense, just the comment might be a little bit misleading. vq-owner will always be one and the same file handle, unless I'm missing something. Moreover, does it really makes sense when a new m2m device context is being created during each video device open()? Having the queue owner's device minor in the trace output is very useful when tracing a single stream across multiple devices. To discern events from multiple simultaneous contexts I have added the context id to the coda driver specific trace events. OK, I understand now, you are just using this stored file handle for traces. -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] x86/mm/pat, drivers/infiniband/ipath: replace WARN() with pr_warn()
* Luis R. Rodriguez mcg...@suse.com wrote: On Thu, Jun 25, 2015 at 08:49:22AM +0200, Ingo Molnar wrote: * Luis R. Rodriguez mcg...@do-not-panic.com wrote: From: Luis R. Rodriguez mcg...@suse.com WARN() may confuse users, fix that. ipath_init_one() is part the device's probe so this would only be triggered if a corresponding device was found. Signed-off-by: Luis R. Rodriguez mcg...@suse.com --- drivers/infiniband/hw/ipath/ipath_driver.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c b/drivers/infiniband/hw/ipath/ipath_driver.c index 2d7e503..871dbe5 100644 --- a/drivers/infiniband/hw/ipath/ipath_driver.c +++ b/drivers/infiniband/hw/ipath/ipath_driver.c @@ -31,6 +31,8 @@ * SOFTWARE. */ +#define pr_fmt(fmt) KBUILD_MODNAME : fmt + #include linux/sched.h #include linux/spinlock.h #include linux/idr.h @@ -399,8 +401,8 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) u32 bar0 = 0, bar1 = 0; #ifdef CONFIG_X86_64 - if (WARN(pat_enabled(), - ipath needs PAT disabled, boot with nopat kernel parameter\n)) { + if (pat_enabled()) { + pr_warn(ipath needs PAT disabled, boot with nopat kernel parameter\n); ret = -ENODEV; goto bail; } So driver init will always fail with this on modern kernels. Nope, I double checked this, ipath_init_one() is the PCI probe routine, not the module init call. It should probably be renamed. Btw., on a second thought, ipath uses MTRRs to enable WC: ret = ipath_enable_wc(dd); if (ret) ret = 0; Note how it ignores any failures - the driver still works even if WC was not enabled. Ah, well WC strategy requires a split of the MMIO registers and the desired WC area, right now they are combined for some type of ipath devices. There are two things to consider when thinking about whether or not we want to do the work required to do the split: But ... why doing the 'split'? With my suggested approach the driver will behave in two ways: - if booted with 'nopat' it will behave as always and have the WC MTRR entries added - if booted with a modern kernel without 'nopat' then instead of getting WC MTRR entries it will not get them - we'll fall back to UC. No 'split' or any other change is needed to the driver AFAICS: it might be slower, but it will still be functional. It will _not_ get PAT WC mappings - it will fall back to UC - which is still much better for any potential user than not working at all. Same suggestion for the other affected driver. what am I missing? Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
* Luis R. Rodriguez mcg...@suse.com wrote: On Thu, Jun 25, 2015 at 08:51:47AM +0200, Ingo Molnar wrote: * Luis R. Rodriguez mcg...@do-not-panic.com wrote: From: Luis R. Rodriguez mcg...@suse.com On built-in kernels this warning will always splat as this is part of the module init. Fix that by shifting the PAT requirement check out under the code that does the quasi-probe for the device. This device driver relies on an existing driver to find its own devices, it looks for that device driver and its own found devices, then uses driver_for_each_device() to try to see if it can probe each of those devices as a frambuffer device with ivtvfb_init_card(). We tuck the PAT requiremenet check then on the ivtvfb_init_card() call making the check at least require an ivtv device present before complaining. Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot] Signed-off-by: Luis R. Rodriguez mcg...@suse.com --- drivers/media/pci/ivtv/ivtvfb.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c index 4cb365d..8b95eef 100644 --- a/drivers/media/pci/ivtv/ivtvfb.c +++ b/drivers/media/pci/ivtv/ivtvfb.c @@ -38,6 +38,8 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +#define pr_fmt(fmt) KBUILD_MODNAME : fmt + #include linux/module.h #include linux/kernel.h #include linux/fb.h @@ -1171,6 +1173,13 @@ static int ivtvfb_init_card(struct ivtv *itv) { int rc; +#ifdef CONFIG_X86_64 + if (pat_enabled()) { + pr_warn(ivtvfb needs PAT disabled, boot with nopat kernel parameter\n); + return -ENODEV; + } +#endif + if (itv-osd_info) { IVTVFB_ERR(Card %d already initialised\n, ivtvfb_card_id); return -EBUSY; Same argument as for ipath: why not make arch_phys_wc_add() fail on PAT and return -1, and check it in arch_phys_wc_del()? The arch_phys_wc_add() is a no-op for PAT systems but for PAT to work we need not only need to add this in where we replace the MTRR call but we also need to convert ioremap_nocache() calls to ioremap_wc() but only if things were split up already. We don't need to do that: for such legacy drivers we can fall back to UC just fine, and inform the user that by booting with 'nopat' the old behavior will be back... Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 1/2] v4l2-utils: add support for reduced fps in cvt modeline
Hi Prashant, On 06/24/2015 03:59 PM, Prashant Laddha wrote: Added reduced fps option in cvt timings calculation. In this case, pixel clock is slowed down by a factor of 1000 / 1001 and all other timing parameters are unchanged. With reduced fps option one could generate timings for refresh rates like 29.97 or 59.94. Pixel clock in this case needs better precision, in the order of 0.001 Khz and hence reduced fps option can be supported only when reduced blanking V2 is enabled. Reduced fps is applicable only to nominal refresh rates which are integer multiple of 6, say 24, 30, 60 etc. Cc: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Prashant Laddha prlad...@cisco.com --- utils/v4l2-ctl/v4l2-ctl-modes.cpp | 7 ++- utils/v4l2-ctl/v4l2-ctl-stds.cpp | 2 +- utils/v4l2-ctl/v4l2-ctl.h | 3 ++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/utils/v4l2-ctl/v4l2-ctl-modes.cpp b/utils/v4l2-ctl/v4l2-ctl-modes.cpp index 88f7b6a..9439b51 100644 --- a/utils/v4l2-ctl/v4l2-ctl-modes.cpp +++ b/utils/v4l2-ctl/v4l2-ctl-modes.cpp @@ -122,6 +122,7 @@ static int v_sync_from_aspect_ratio(int width, int height) * @reduced_blanking: This value, if greater than 0, indicates that * reduced blanking is to be used and value indicates the version. * @interlaced: whether to compute an interlaced mode + * @reduced_fps: reduce fps by factor of 1000 / 1001 * @cvt: stores results of cvt timing calculation * * Returns: @@ -131,7 +132,8 @@ static int v_sync_from_aspect_ratio(int width, int height) bool calc_cvt_modeline(int image_width, int image_height, int refresh_rate, int reduced_blanking, -bool interlaced, struct v4l2_bt_timings *cvt) +bool interlaced, bool reduced_fps, +struct v4l2_bt_timings *cvt) { int h_sync; int v_sync; @@ -295,6 +297,9 @@ bool calc_cvt_modeline(int image_width, int image_height, pixel_clock = v_refresh * total_h_pixel * (2 * total_v_lines + interlace) / 2; + if (reduced_fps v_refresh % 6 == 0) + pixel_clock = ((long long)pixel_clock * 1000) / 1001; + I merged this patch a bit too quickly since this is wrong. If reduced fps is required, then the pixelclock in this struct remains at the nominal frequency and all that happens is that the V4L2_DV_FL_REDUCED_FPS flag is set. I fixed this in a follow-up patch. Regards, Hans pixel_clock -= pixel_clock % clk_gran; } diff --git a/utils/v4l2-ctl/v4l2-ctl-stds.cpp b/utils/v4l2-ctl/v4l2-ctl-stds.cpp index aea46c9..e969d08 100644 --- a/utils/v4l2-ctl/v4l2-ctl-stds.cpp +++ b/utils/v4l2-ctl/v4l2-ctl-stds.cpp @@ -241,7 +241,7 @@ static void get_cvt_gtf_timings(char *subopt, int standard, if (standard == V4L2_DV_BT_STD_CVT) { timings_valid = calc_cvt_modeline(width, height, fps, r_blank, - interlaced == 1 ? true : false, bt); + interlaced == 1 ? true : false, false, bt); } else { timings_valid = calc_gtf_modeline(width, height, fps, r_blank, interlaced == 1 ? true : false, bt); diff --git a/utils/v4l2-ctl/v4l2-ctl.h b/utils/v4l2-ctl/v4l2-ctl.h index de65900..113f348 100644 --- a/utils/v4l2-ctl/v4l2-ctl.h +++ b/utils/v4l2-ctl/v4l2-ctl.h @@ -351,7 +351,8 @@ void edid_get(int fd); /* v4l2-ctl-modes.cpp */ bool calc_cvt_modeline(int image_width, int image_height, int refresh_rate, int reduced_blanking, -bool interlaced, struct v4l2_bt_timings *cvt); +bool interlaced, bool reduced_fps, +struct v4l2_bt_timings *cvt); bool calc_gtf_modeline(int image_width, int image_height, int refresh_rate, int reduced_blanking, -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/1] V4L2: platform: Renesas R-Car JPEG codec driver
Hi, Thanks everybody for comments. 2015-06-22 17:54 GMT+03:00 Kamil Debski ka...@wypas.org: Hi, I am adding Jacek Anaszewski to CC loop. He was working with the s5p-jpeg driver some time ago. I've spoken with him about questions in this email recently. Jacek, thank you for your comments :) On 18 June 2015 at 20:48, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Mikhail, (CC'ing Kamil Debski) On Wednesday 06 May 2015 01:03:10 Mikhail Ulianov wrote: On Mon, 04 May 2015 02:32:05 +0300 Laurent Pinchart wrote: [snip] [snip] +/* + * + * Queue operations + * + */ +static int jpu_queue_setup(struct vb2_queue *vq, + const struct v4l2_format *fmt, + unsigned int *nbuffers, unsigned int *nplanes, + unsigned int sizes[], void *alloc_ctxs[]) +{ + struct jpu_ctx *ctx = vb2_get_drv_priv(vq); + struct jpu_q_data *q_data; + unsigned int count = *nbuffers; + unsigned int i; + + q_data = jpu_get_q_data(ctx, vq-type); + + *nplanes = q_data-format.num_planes; + + /* + * Header is parsed during decoding and parsed information stored + * in the context so we do not allow another buffer to overwrite it. + * For now it works this way, but planned for alternation. It shouldn't be difficult to create a jpu_buffer structure that inherits from vb2_buffer and store the information there instead of in the context. You are absolutely right. But for this version i want to keep it simple and also at this moment not everything clear for me with this format autodetection feature we want to have e.g. for decoder if user requested 2 output buffers and then queue first with some valid JPEG with format -1-(so we setup queues format here), after that another one with format -2-... should we discard second one or just change format of queues? what about same situation if user already requested capture buffers. I mean relations with buf_prepare and queue_setup. AFAIU format should remain the same for all requested buffers. I see only one solid solution here - get rid of autodetection feature and rely only on format setted by user, so in this case we can just discard queued buffers with inappropriate format(kind of format validation in kernel). This solution will also work well with NV61, NV21, and semiplanar formats we want to add in next version. *But* with this solution header parsing must be done twice(in user and kernel spaces). I'm a little bit frustrated here :) Yes, it's a bit frustrating indeed. I'm not sure what to advise, I'm not too familiar with the m2m API for JPEG. Kamil, do you have a comment on that ? I am not sure whether it is good to get rid of header parsing by the driver/hardware option. I agree that the buffers should have a consistent format and size. Maybe the way to go would be to allow header parsing by the hardware, but to stop processing when the format has changed? Other solution would be to use the V4L2_EVENT_SOURCE_CHANGE event to inform user space about the change. User space then could check whether the buffers are sufficient or reallocate them. Similar to what happens in MFC when format changes. For me implementing resolution change in JPEG seems like an overkill, but maybe you have a use case that would benefit from this. Initially the JPEG decoder was designed and written as a one shot device. Could you give an example of such use case? The possible use case I can imagine is having an M-JPEG stream where all JPEGs have the same dimensions and format. There I can see some benefits from having more than one buffer on the queues. Then there would be no change in the buffer parameters, so this should work. That's correct. It's exactly our use case, but we have few cameras. So we serialize buffers in user space and sometimes one(or more) cameras have different configuration. I think i should go with stop processing option. [snip] [snip] Best wishes, Kamil Debski -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/1] V4L2: platform: Renesas R-Car JPEG codec driver
Hi Mikhail, On 26 June 2015 at 12:34, Mikhail Ulyanov mikhail.ulya...@cogentembedded.com wrote: Hi, Thanks everybody for comments. 2015-06-22 17:54 GMT+03:00 Kamil Debski ka...@wypas.org: Hi, I am adding Jacek Anaszewski to CC loop. He was working with the s5p-jpeg driver some time ago. I've spoken with him about questions in this email recently. Jacek, thank you for your comments :) On 18 June 2015 at 20:48, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Mikhail, (CC'ing Kamil Debski) On Wednesday 06 May 2015 01:03:10 Mikhail Ulianov wrote: On Mon, 04 May 2015 02:32:05 +0300 Laurent Pinchart wrote: [snip] [snip] +/* + * + * Queue operations + * + */ +static int jpu_queue_setup(struct vb2_queue *vq, + const struct v4l2_format *fmt, + unsigned int *nbuffers, unsigned int *nplanes, + unsigned int sizes[], void *alloc_ctxs[]) +{ + struct jpu_ctx *ctx = vb2_get_drv_priv(vq); + struct jpu_q_data *q_data; + unsigned int count = *nbuffers; + unsigned int i; + + q_data = jpu_get_q_data(ctx, vq-type); + + *nplanes = q_data-format.num_planes; + + /* + * Header is parsed during decoding and parsed information stored + * in the context so we do not allow another buffer to overwrite it. + * For now it works this way, but planned for alternation. It shouldn't be difficult to create a jpu_buffer structure that inherits from vb2_buffer and store the information there instead of in the context. You are absolutely right. But for this version i want to keep it simple and also at this moment not everything clear for me with this format autodetection feature we want to have e.g. for decoder if user requested 2 output buffers and then queue first with some valid JPEG with format -1-(so we setup queues format here), after that another one with format -2-... should we discard second one or just change format of queues? what about same situation if user already requested capture buffers. I mean relations with buf_prepare and queue_setup. AFAIU format should remain the same for all requested buffers. I see only one solid solution here - get rid of autodetection feature and rely only on format setted by user, so in this case we can just discard queued buffers with inappropriate format(kind of format validation in kernel). This solution will also work well with NV61, NV21, and semiplanar formats we want to add in next version. *But* with this solution header parsing must be done twice(in user and kernel spaces). I'm a little bit frustrated here :) Yes, it's a bit frustrating indeed. I'm not sure what to advise, I'm not too familiar with the m2m API for JPEG. Kamil, do you have a comment on that ? I am not sure whether it is good to get rid of header parsing by the driver/hardware option. I agree that the buffers should have a consistent format and size. Maybe the way to go would be to allow header parsing by the hardware, but to stop processing when the format has changed? Other solution would be to use the V4L2_EVENT_SOURCE_CHANGE event to inform user space about the change. User space then could check whether the buffers are sufficient or reallocate them. Similar to what happens in MFC when format changes. For me implementing resolution change in JPEG seems like an overkill, but maybe you have a use case that would benefit from this. Initially the JPEG decoder was designed and written as a one shot device. Could you give an example of such use case? The possible use case I can imagine is having an M-JPEG stream where all JPEGs have the same dimensions and format. There I can see some benefits from having more than one buffer on the queues. Then there would be no change in the buffer parameters, so this should work. That's correct. It's exactly our use case, but we have few cameras. So we serialize buffers in user space and sometimes one(or more) cameras have different configuration. I think i should go with stop processing option. It could be possible to have frames from each camera decoded in different contexts. This way the configuration should be consistent for all frames in a context. It will require more memory (more buffers) though... [snip] [snip] Best wishes, Kamil Debski -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL]: dma-buf changes for 4.2
Hi Linus, Very small pull request on dma-buf for 4.2 merge window. May I request you to please pull? The following changes since commit 5ebe6afaf0057ac3eaeb98defd5456894b446d22: Linux 4.1-rc2 (2015-05-03 19:22:23 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/sumits/dma-buf.git tags/dma-buf-for-4.2 for you to fetch changes up to 5136629dc5a19701746abd7c8ad98ce0b84dda1d: dma-buf: Minor coding style fixes (2015-05-21 11:29:59 +0530) Minor changes for 4.2 - add ref-counting for kernel modules as exporters - minor code style fixes Jagan Teki (1): dma-buf: Minor coding style fixes Sumit Semwal (1): dma-buf: add ref counting for module as exporter drivers/dma-buf/dma-buf.c | 19 --- drivers/dma-buf/reservation.c | 9 ++--- drivers/dma-buf/seqno-fence.c | 8 +++- include/linux/dma-buf.h | 10 -- 4 files changed, 37 insertions(+), 9 deletions(-) Thanks and best regards, Sumit. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XC5000C 0x14b4 status
An image of the top of the tuner clearly showing any manufacturing markings would be welcome - assuming its accessible. It's a best picture I could find: http://www.reviews.ru/clause/over/T7_2/image41.jpg Thanks. -- Steven Toth - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XC5000C 0x14b4 status
Here is a link on a chinese site with datasheet for xc5100. http://wenku.baidu.com/view/7f92f3fe700abb68a982fb96.html If you look at it, after reading Product ID we also should receive 0x14b4 (5300 decimal) I'll try extract a FW from a Windows driver and will share results, in case of success. Best regards. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Enumeration of Media Interfaces
Hi Hans, Em Fri, 26 Jun 2015 13:23:01 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: Introduction This RFC is a proposal that will hopefully fix the impasse w.r.t. DVB and MC. This proposal was the result of a private irc discussion between Laurent and myself. We were both on opposite sides of the earlier discussions and we decided to try to come to a solution between just the two of us. Sakari read an earlier version of this RFC and is OK with it as well, so now it is time to post it publicly. Hopefully Mauro likes it as well. Yeah, the general concept of the RFC is OK to me. We need to dig into the details. This proposal is fairly high-level, and it does not attempt to go into details. Before doing that we need to know if all parties agree with the basic concept. Laurent, Sakari and myself are considering to meet in Finland for 2-3 days in July to hammer out the details. A meeting to discuss would be good. I should be participating on it, but we should try a closer venue. We could do that in Brazil ;) Or on some place in the middle, that would have more direct routes from here. London is a good place, as I can get some space for our meeting at Samsung Research UK, with would make easier to have other Samsung guys working on DVB to join us. Let's try to schedule this via IRC, and then announce though the ML to see if others can join as well. Proposal There are two types of drivers: the first (true for all non-MC drivers available today) controls the full pipeline through DVB/V4L/ALSA/etc. interfaces (generally these interfaces are device nodes). Applications for these drivers do not need to know which entities there are, knowing the interfaces is enough. The second type (the current set of MC drivers) exposes the hardware through media entities, and each entity can be controlled through a single device node: v4l-subdevX for subdevices, and videoX for DMA engines. Applications for these drivers will need to know the entities and how they are linked in order to configure the hardware correctly. The only interfaces they need to know are those that control each entity. The first type of driver will be called an 'interface-centric' driver, the second type will be called an 'entity-centric' driver in this document. Better names are welcome, but I can't talk about MC and non-MC drivers anymore since we want to use the MC for all drivers, so we need to come up with another name. So interface vs entity centric is the best I came up with. So we want to extend the use of the MC for all driver types, but we ran into a major difference of opinion of how to represent device nodes (are these media entities or properties of media entities?) and how to represent which entities a device node controls (through links, properties, something else?). This RFC presents a solution that I hope is acceptable to all. Media Interfaces The key difference of this proposal is that interfaces such as device nodes (but this can be an e.g. network interface as well) are represented by a new struct: media_interface. And that the list of interfaces that a media device has can be obtained by a new ioctl: MEDIA_IOC_G_INTERFACES. The ioctl will get all interfaces in a single call for efficiency and atomicity. So an interface is different from an entity, but both are first class citizens in the media controller. The kernel implementation will be that the media_entity in struct video_device will be replaced by a struct media_interface. And any media_interface that is created will link into a linked list maintained by the media_device struct. As a consequence of this change there now is no longer a media_entity for DMA engines to use: this will have to be created in the DMA engine driver itself, just as subdevice drivers have their own media_entity. This design is a lot cleaner as well since today the video_device struct used to represent a v4l-subdevX device node doesn't use the media_entity field, which is weird and it indicates that there is a problem with the datastructure design. However, with the introduction of a media_interface struct any video_device struct will always use the media_interface field. So device nodes are media interfaces, hardware blocks are media entities. In rare cases software blocks are allowed as media entities (e.g. the DVB software demux), but there should be very good reasons for doing so. This solves the problem of how to represent device nodes: these are no longer entities (which several developers were strongly opposed to), but neither are they properties (which the other camp was strongly opposed to). Instead they are now a new object: the media interface. The existing DVB/V4L/ etc. applications can use the new ioctl to obtain all interfaces exposed by the MC and open the device nodes that are relevant to
about adding TBS 5980 driver to dw2102
hello all, I am new to dvb kernel driver development and have some questions. the hardware is explained here, http://www.linuxtv.org/wiki/index.php/TBS_Qbox_DVB-S2_CI_USB2.0 Whats the difference between dvb-usb and dvb-usb-v2, what is added or why are there 2 versions? I am trying to implement the tbs5980 driver into the dw2102 driver. It uses the stv090X driver instead of the stv0900 driver used in the dw2102 driver. So my question is , what are the differences between the drivers? and can i make the driver work woth the stv0900 instead of the stv090x version(to avoid adding another header file in the driver). also i want to later add support for the integrated Ci cam slot to the driver, is this ok to add also? i have seen much of the inner functions of the ci slot are already implemented in the ci slot driver, and do not need to be copied like in the original driver. The question is , do i only attach the ci slot to the right driver in the dw2102 driver and let it handle it? or do i need to do the same as the original one? the (not working progress) so far is included in the patch1.patch.orig file, so you can have a see what i have done so far. thanks for your time and anwsers, Greetings, Steven Vanden Branden /* * TurboSight (TBS) Qbox DVB-S2 CI driver * * Copyright (c) 2010 Konstantin Dimitrov kosio.dimit...@gmail.com * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the * Free Software Foundation, version 2. * */ #include linux/version.h #include tbs-qbox2ci.h #include stv6110x.h #include stv090x.h #include stb6100.h #include stb6100_cfg.h #include dvb_ca_en50221.h #define TBSQBOX_READ_MSG 0 #define TBSQBOX_WRITE_MSG 1 #define TBSQBOX_LED_CTRL (0x1b00) /* on my own*/ #define TBSQBOX_VOLTAGE_CTRL (0x1800) #define TBSQBOX_RC_QUERY (0x1a00) struct tbsqbox2ci_state { struct dvb_ca_en50221 ca; struct mutex ca_mutex; u32 last_key_pressed; }; /*struct tbsqbox2ci_rc_keys { u32 keycode; u32 event; };*/ /* debug */ static int dvb_usb_tbsqbox2ci_debug; module_param_named(debug, dvb_usb_tbsqbox2ci_debug, int, 0644); MODULE_PARM_DESC(debug, set debugging level (1=info 2=xfer (or-able)). DVB_USB_DEBUG_STATUS); DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); static int tbsqbox2ci_op_rw(struct usb_device *dev, u8 request, u16 value, u16 index, u8 * data, u16 len, int flags) { int ret; u8 u8buf[len]; unsigned int pipe = (flags == TBSQBOX_READ_MSG) ? usb_rcvctrlpipe(dev, 0) : usb_sndctrlpipe(dev, 0); u8 request_type = (flags == TBSQBOX_READ_MSG) ? USB_DIR_IN : USB_DIR_OUT; if (flags == TBSQBOX_WRITE_MSG) memcpy(u8buf, data, len); ret = usb_control_msg(dev, pipe, request, request_type | USB_TYPE_VENDOR, value, index , u8buf, len, 2000); if (flags == TBSQBOX_READ_MSG) memcpy(data, u8buf, len); return ret; } static int tbsqbox2ci_read_attribute_mem(struct dvb_ca_en50221 *ca, int slot, int address) { struct dvb_usb_device *d = (struct dvb_usb_device *)ca-data; struct tbsqbox2ci_state *state = (struct tbsqbox2ci_state *)d-priv; u8 buf[4], rbuf[3]; int ret; if (0 != slot) return -EINVAL; buf[0] = 1; buf[1] = 0; buf[2] = (address 8) 0x0f; buf[3] = address; //msleep(10); mutex_lock(state-ca_mutex); ret = tbsqbox2ci_op_rw(d-udev, 0xa4, 0, 0, buf, 4, TBSQBOX_WRITE_MSG); //msleep(1); ret = tbsqbox2ci_op_rw(d-udev, 0xa5, 0, 0, rbuf, 1, TBSQBOX_READ_MSG); mutex_unlock(state-ca_mutex); if (ret 0) return ret; return rbuf[0]; } static int tbsqbox2ci_write_attribute_mem(struct dvb_ca_en50221 *ca, int slot, int address, u8 value) { struct dvb_usb_device *d = (struct dvb_usb_device *)ca-data; struct tbsqbox2ci_state *state = (struct tbsqbox2ci_state *)d-priv; u8 buf[5];//, rbuf[1]; int ret; if (0 != slot) return -EINVAL; buf[0] = 1; buf[1] = 0; buf[2] = (address 8) 0x0f; buf[3] = address; buf[4] = value; mutex_lock(state-ca_mutex); ret = tbsqbox2ci_op_rw(d-udev, 0xa2, 0, 0, buf, 5, TBSQBOX_WRITE_MSG); //msleep(1); //ret = tbsqbox2ci_op_rw(d-udev, 0xa5, 0, 0, // rbuf, 1, TBSQBOX_READ_MSG); mutex_unlock(state-ca_mutex); if (ret 0) return ret; return 0; } static int tbsqbox2ci_read_cam_control(struct dvb_ca_en50221 *ca, int slot, u8 address) { struct dvb_usb_device *d = (struct dvb_usb_device *)ca-data; struct tbsqbox2ci_state *state = (struct tbsqbox2ci_state *)d-priv; u8 buf[4], rbuf[1]; int ret; if (0 != slot) return -EINVAL; buf[0] = 1; buf[1] = 1; buf[2] = (address 8) 0x0f; buf[3] = address; mutex_lock(state-ca_mutex); ret = tbsqbox2ci_op_rw(d-udev, 0xa4, 0, 0, buf, 4, TBSQBOX_WRITE_MSG); //msleep(10); ret = tbsqbox2ci_op_rw(d-udev, 0xa5, 0, 0, rbuf, 1, TBSQBOX_READ_MSG); mutex_unlock(state-ca_mutex); if (ret 0) return ret; return rbuf[0]; } static int
[PATCH -next] media/pci/cobalt: fix Kconfig and build when SND is not enabled
From: Randy Dunlap rdun...@infradead.org Fix build errors in cobalt driver when CONFIG_SND is not enabled. Fixes these build errors: ERROR: snd_pcm_period_elapsed [drivers/media/pci/cobalt/cobalt.ko] undefined! ERROR: _snd_pcm_stream_lock_irqsave [drivers/media/pci/cobalt/cobalt.ko] undefined! ERROR: snd_pcm_hw_constraint_integer [drivers/media/pci/cobalt/cobalt.ko] undefined! ERROR: snd_pcm_set_ops [drivers/media/pci/cobalt/cobalt.ko] undefined! ERROR: snd_pcm_stream_unlock_irqrestore [drivers/media/pci/cobalt/cobalt.ko] undefined! ERROR: snd_pcm_lib_ioctl [drivers/media/pci/cobalt/cobalt.ko] undefined! ERROR: snd_card_new [drivers/media/pci/cobalt/cobalt.ko] undefined! ERROR: snd_card_free [drivers/media/pci/cobalt/cobalt.ko] undefined! ERROR: snd_card_register [drivers/media/pci/cobalt/cobalt.ko] undefined! ERROR: snd_pcm_new [drivers/media/pci/cobalt/cobalt.ko] undefined! Signed-off-by: Randy Dunlap rdun...@infradead.org Cc: Hans Verkuil hans.verk...@cisco.com --- drivers/media/pci/cobalt/Kconfig |1 + 1 file changed, 1 insertion(+) --- linux-next-20150626.orig/drivers/media/pci/cobalt/Kconfig +++ linux-next-20150626/drivers/media/pci/cobalt/Kconfig @@ -2,6 +2,7 @@ config VIDEO_COBALT tristate Cisco Cobalt support depends on VIDEO_V4L2 I2C MEDIA_CONTROLLER depends on PCI_MSI MTD_COMPLEX_MAPPINGS GPIOLIB + depends on SND select I2C_ALGOBIT select VIDEO_ADV7604 select VIDEO_ADV7511 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH -next] media/dvb: fix ts2020.c Kconfig and build
From: Randy Dunlap rdun...@infradead.org Fix kconfig warning that is caused by DVB_TS2020: warning: (DVB_TS2020 SND_SOC_ADAU1761_I2C SND_SOC_ADAU1781_I2C SND_SOC_ADAU1977_I2C SND_SOC_RT5677 EXTCON_MAX14577 EXTCON_MAX77693 EXTCON_MAX77843) selects REGMAP_I2C which has unmet direct dependencies (I2C) This fixes many subsequent build errors. Signed-off-by: Randy Dunlap rdun...@infradead.org Cc: Mauro Carvalho Chehab mche...@osg.samsung.com Cc: Konstantin Dimitrov kosio.dimit...@gmail.com --- drivers/media/dvb-frontends/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- linux-next-20150626.orig/drivers/media/dvb-frontends/Kconfig +++ linux-next-20150626/drivers/media/dvb-frontends/Kconfig @@ -240,7 +240,7 @@ config DVB_SI21XX config DVB_TS2020 tristate Montage Tehnology TS2020 based tuners - depends on DVB_CORE + depends on DVB_CORE I2C select REGMAP_I2C default m if !MEDIA_SUBDRV_AUTOSELECT help -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] [media] v4l2-mem2mem: set the queue owner field just as vb2_ioctl_reqbufs does
The queue owner will be used by videobuf2 trace events to determine and record the device minor number. It is set in v4l2_m2m_reqbufs instead of v4l2_m2m_ioctl_reqbufs because several drivers implement their own vidioc_reqbufs handlers that still call v4l2_m2m_reqbufs directly. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/v4l2-core/v4l2-mem2mem.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c index dc853e5..af8d6b4 100644 --- a/drivers/media/v4l2-core/v4l2-mem2mem.c +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c @@ -357,9 +357,16 @@ int v4l2_m2m_reqbufs(struct file *file, struct v4l2_m2m_ctx *m2m_ctx, struct v4l2_requestbuffers *reqbufs) { struct vb2_queue *vq; + int ret; vq = v4l2_m2m_get_vq(m2m_ctx, reqbufs-type); - return vb2_reqbufs(vq, reqbufs); + ret = vb2_reqbufs(vq, reqbufs); + /* If count == 0, then the owner has released all buffers and he + is no longer owner of the queue. Otherwise we have an owner. */ + if (ret == 0) + vq-owner = reqbufs-count ? file-private_data : NULL; + + return ret; } EXPORT_SYMBOL_GPL(v4l2_m2m_reqbufs); -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] [media] videobuf2: add trace events
Add videobuf2 specific vb2_qbuf and vb2_dqbuf trace events that mirror the v4l2_qbuf and v4l2_dqbuf trace events, only they include additional information about queue fill state and are emitted right before the buffer is enqueued in the driver or userspace is woken up. This allows to make sense of the timeline of trace events in combination with others that might be triggered by __enqueue_in_driver. Also two new trace events vb2_buf_queue and vb2_buf_done are added, allowing to trace the handover between videobuf2 framework and driver. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- Changes since v1: - use event class for similar trace events, as requested by Steven Rostedt --- drivers/media/v4l2-core/videobuf2-core.c | 11 include/trace/events/v4l2.h | 97 2 files changed, 108 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 93b3154..b866a6b 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -30,6 +30,8 @@ #include media/v4l2-common.h #include media/videobuf2-core.h +#include trace/events/v4l2.h + static int debug; module_param(debug, int, 0644); @@ -1207,6 +1209,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) atomic_dec(q-owned_by_drv_count); spin_unlock_irqrestore(q-done_lock, flags); + trace_vb2_buf_done(q, vb); + if (state == VB2_BUF_STATE_QUEUED) { if (q-start_streaming_called) __enqueue_in_driver(vb); @@ -1629,6 +1633,8 @@ static void __enqueue_in_driver(struct vb2_buffer *vb) vb-state = VB2_BUF_STATE_ACTIVE; atomic_inc(q-owned_by_drv_count); + trace_vb2_buf_queue(q, vb); + /* sync buffers */ for (plane = 0; plane vb-num_planes; ++plane) call_void_memop(vb, prepare, vb-planes[plane].mem_priv); @@ -1878,6 +1884,8 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b) vb-v4l2_buf.timecode = b-timecode; } + trace_vb2_qbuf(q, vb); + /* * If already streaming, give the buffer to driver for processing. * If not, the buffer will be given to driver on next streamon. @@ -2123,6 +2131,9 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n /* Remove from videobuf queue */ list_del(vb-queued_entry); q-queued_count--; + + trace_vb2_dqbuf(q, vb); + if (!V4L2_TYPE_IS_OUTPUT(q-type) vb-v4l2_buf.flags V4L2_BUF_FLAG_LAST) q-last_buffer_dequeued = true; diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h index 4c88a32..d9e9aeb 100644 --- a/include/trace/events/v4l2.h +++ b/include/trace/events/v4l2.h @@ -174,6 +174,103 @@ DEFINE_EVENT(v4l2_event_class, v4l2_qbuf, TP_ARGS(minor, buf) ); +DECLARE_EVENT_CLASS(vb2_event_class, + TP_PROTO(struct vb2_queue *q, struct vb2_buffer *vb), + TP_ARGS(q, vb), + + TP_STRUCT__entry( + __field(int, minor) + __field(u32, queued_count) + __field(int, owned_by_drv_count) + __field(u32, index) + __field(u32, type) + __field(u32, bytesused) + __field(u32, flags) + __field(u32, field) + __field(s64, timestamp) + __field(u32, timecode_type) + __field(u32, timecode_flags) + __field(u8, timecode_frames) + __field(u8, timecode_seconds) + __field(u8, timecode_minutes) + __field(u8, timecode_hours) + __field(u8, timecode_userbits0) + __field(u8, timecode_userbits1) + __field(u8, timecode_userbits2) + __field(u8, timecode_userbits3) + __field(u32, sequence) + ), + + TP_fast_assign( + __entry-minor = q-owner ? q-owner-vdev-minor : -1; + __entry-queued_count = q-queued_count; + __entry-owned_by_drv_count = + atomic_read(q-owned_by_drv_count); + __entry-index = vb-v4l2_buf.index; + __entry-type = vb-v4l2_buf.type; + __entry-bytesused = vb-v4l2_buf.bytesused; + __entry-flags = vb-v4l2_buf.flags; + __entry-field = vb-v4l2_buf.field; + __entry-timestamp = timeval_to_ns(vb-v4l2_buf.timestamp); + __entry-timecode_type = vb-v4l2_buf.timecode.type; + __entry-timecode_flags = vb-v4l2_buf.timecode.flags; + __entry-timecode_frames = vb-v4l2_buf.timecode.frames; + __entry-timecode_seconds = vb-v4l2_buf.timecode.seconds; + __entry-timecode_minutes = vb-v4l2_buf.timecode.minutes; + __entry-timecode_hours =
[PATCH v2 1/3] [media] v4l2: use event class to deduplicate v4l2 trace events
Trace events with exactly the same parameters and trace output, such as v4l2_qbuf and v4l2_dqbuf, are supposed to use the DECLARE_EVENT_CLASS and DEFINE_EVENT macros instead of duplicated TRACE_EVENT macro calls. Suggested-by: Steven Rostedt rost...@goodmis.org Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- include/trace/events/v4l2.h | 160 +--- 1 file changed, 78 insertions(+), 82 deletions(-) diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h index 89d0497..4c88a32 100644 --- a/include/trace/events/v4l2.h +++ b/include/trace/events/v4l2.h @@ -93,90 +93,86 @@ SHOW_FIELD { V4L2_TC_USERBITS_USERDEFINED, USERBITS_USERDEFINED }, \ { V4L2_TC_USERBITS_8BITCHARS, USERBITS_8BITCHARS }) -#define V4L2_TRACE_EVENT(event_name) \ - TRACE_EVENT(event_name, \ - TP_PROTO(int minor, struct v4l2_buffer *buf), \ - \ - TP_ARGS(minor, buf),\ - \ - TP_STRUCT__entry( \ - __field(int, minor) \ - __field(u32, index) \ - __field(u32, type) \ - __field(u32, bytesused) \ - __field(u32, flags) \ - __field(u32, field) \ - __field(s64, timestamp) \ - __field(u32, timecode_type) \ - __field(u32, timecode_flags)\ - __field(u8, timecode_frames)\ - __field(u8, timecode_seconds) \ - __field(u8, timecode_minutes) \ - __field(u8, timecode_hours) \ - __field(u8, timecode_userbits0) \ - __field(u8, timecode_userbits1) \ - __field(u8, timecode_userbits2) \ - __field(u8, timecode_userbits3) \ - __field(u32, sequence) \ - ), \ - \ - TP_fast_assign( \ - __entry-minor = minor; \ - __entry-index = buf-index;\ - __entry-type = buf-type; \ - __entry-bytesused = buf-bytesused;\ - __entry-flags = buf-flags;\ - __entry-field = buf-field;\ - __entry-timestamp =\ - timeval_to_ns(buf-timestamp); \ - __entry-timecode_type = buf-timecode.type;\ - __entry-timecode_flags = buf-timecode.flags; \ - __entry-timecode_frames = \ - buf-timecode.frames; \ - __entry-timecode_seconds = \ - buf-timecode.seconds; \ - __entry-timecode_minutes = \ - buf-timecode.minutes; \ - __entry-timecode_hours = buf-timecode.hours; \ - __entry-timecode_userbits0 = \ - buf-timecode.userbits[0]; \ - __entry-timecode_userbits1 = \ - buf-timecode.userbits[1]; \ - __entry-timecode_userbits2 = \ - buf-timecode.userbits[2]; \ - __entry-timecode_userbits3 = \ - buf-timecode.userbits[3]; \ - __entry-sequence = buf-sequence; \ - ), \ - \ - TP_printk(minor = %d, index = %u, type = %s, \ -
Re: [PATCH 1/2] [media] v4l2-mem2mem: set the queue owner field just as vb2_ioctl_reqbufs does
On 26/06/15 11:02, Philipp Zabel wrote: Am Freitag, den 26.06.2015, 10:28 +0200 schrieb Sylwester Nawrocki: [...] How about modifying v4l2_m2m_ioctl_reqbufs() instead ? The coda, gsc-m2m, m2m-deinterlace, mx2_emmaprp, and sh_veu drivers all have their own implementation of vidioc_reqbufs that call v4l2_m2m_reqbufs directly. Maybe this should be moved into v4l2_m2m_ioctl_reqbufs after all drivers are updated to use it instead of v4l2_m2m_reqbufs. In case of some of the above listed drivers it shouldn't be difficult and would be nice to convert to the generic v4l2_m2m_ioctl* callbacks. Anyway, I guess your code change makes sense, just the comment might be a little bit misleading. vq-owner will always be one and the same file handle, unless I'm missing something. True. Since the m2m_ctx containing the vb2_queue is attached to the file handle, this will only ever get called with the same file handle for a given queue. s/we have a new owner/we have an owner/ ? Sounds good enough to me. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] [media] v4l2-mem2mem: set the queue owner field just as vb2_ioctl_reqbufs does
Am Freitag, den 26.06.2015, 10:28 +0200 schrieb Sylwester Nawrocki: [...] How about modifying v4l2_m2m_ioctl_reqbufs() instead ? The coda, gsc-m2m, m2m-deinterlace, mx2_emmaprp, and sh_veu drivers all have their own implementation of vidioc_reqbufs that call v4l2_m2m_reqbufs directly. Maybe this should be moved into v4l2_m2m_ioctl_reqbufs after all drivers are updated to use it instead of v4l2_m2m_reqbufs. In case of some of the above listed drivers it shouldn't be difficult and would be nice to convert to the generic v4l2_m2m_ioctl* callbacks. Anyway, I guess your code change makes sense, just the comment might be a little bit misleading. vq-owner will always be one and the same file handle, unless I'm missing something. True. Since the m2m_ctx containing the vb2_queue is attached to the file handle, this will only ever get called with the same file handle for a given queue. s/we have a new owner/we have an owner/ ? [...] Having the queue owner's device minor in the trace output is very useful when tracing a single stream across multiple devices. To discern events from multiple simultaneous contexts I have added the context id to the coda driver specific trace events. OK, I understand now, you are just using this stored file handle for traces. I should mention this in the patch description. regards Philipp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XC5000C 0x14b4 status
Em Fri, 26 Jun 2015 03:58:48 +0700 Unembossed Name severe.siberian@mail.ru escreveu: Hi, I was working on a Linux driver for a hybrid TV-tuner with SAA7134 PCI bridge, XC5000C RF tuner and Si2168 DVB demodulator by a combining all existent at that time drivers together. During that work, I had an issue with XC5000C. Episodically, after attaching to DVB and reading XREG_PRODUCT_ID register, it was possible to receive 0x14b4 instead of usual 0x1388 status. And as a result get a xc5000: Device not found at addr 0x%02x (0x%x)\n in dmesg. To workaround these, I added a few strings to a source of a driver to make it's behaviour the same for 0x14b4, as for 0x1388. After that RF tuner identification became always successful. I had a conversation with a hardware vendor. Now I can say, that such behaviour, most likely, because of early firmware for XC5000C. This hardware vendor is using for his Windows driver a latest firmware, and reading Product ID register always gives 0x14b4 status. As he says, 0x1388 status is only for previous XC5000 IC. (Without C at end of a P/N) Is this possible to patch xc5000.c with something like this: #define XC_PRODUCT_ID_FW_LOADED 0x1388 +#define XC_PRODUCT_ID_FW_LOADED_XC5000C 0x14b4 case XC_PRODUCT_ID_FW_LOADED: + case XC_PRODUCT_ID_FW_LOADED_XC5000C: printk(KERN_INFO xc5000: Successfully identified at address 0x%02x\n, Or to try to get a chip vendor's permission for using a latest firmware for XC5000C in Linux, and then anyway, patch the driver? IMHO, the best is to get the latest firmware licensed is the best thing to do. Does that new xc5000c come with a firmware pre-loaded already? Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: OK
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Sat Jun 27 04:00:29 CEST 2015 git branch: test git hash: faebbd8f134f0c054f372982c8ddd1bbcc41b440 gcc version:i686-linux-gcc (GCC) 5.1.0 sparse version: v0.5.0-44-g40791b9 smatch version: 0.4.1-3153-g7d56ab3 host hardware: x86_64 host os:4.0.0-3.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12.23-i686: OK linux-3.13.11-i686: OK linux-3.14.9-i686: OK linux-3.15.2-i686: OK linux-3.16.7-i686: OK linux-3.17.8-i686: OK linux-3.18.7-i686: OK linux-3.19-i686: OK linux-4.0-i686: OK linux-4.1-rc1-i686: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12.23-x86_64: OK linux-3.13.11-x86_64: OK linux-3.14.9-x86_64: OK linux-3.15.2-x86_64: OK linux-3.16.7-x86_64: OK linux-3.17.8-x86_64: OK linux-3.18.7-x86_64: OK linux-3.19-x86_64: OK linux-4.0-x86_64: OK linux-4.1-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS smatch: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XC5000C 0x14b4 status
Correct. These are not parts that have any form of default firmware in their ROM mask (i.e. not like the silabs or micronas parts which have a default firmware and the ability to patch the ROM via a software loaded code update). The firmware must be loaded every time the chip is brought out of reset or it won't work at all. An image of the top of the tuner clearly showing any manufacturing markings would be welcome - assuming its accessible. -- Steven Toth - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XC5000C 0x14b4 status
It's not new IC. It's XC5000C. Maybe i was interpreted wrong. As I have understood, such behaviour can depends from FW version. HW vendor says, that with his latest FW he always gets response 0x14b4. Ah, so you're running a completely different firmware image? Well in that case that would explain the different response for the firmware loaded indication. No no no. I used a FW image, which was taken from KernelLabs site, and it was intended for use with XC5000C. And partnumber of IC was XC5000C ( I have opened RF shild to check it) Not a 0x1388. And I think, that these ICs still come without pre-loaded FW. HW vendor also didn't says anything about FW pre-load possibility. Correct. These are not parts that have any form of default firmware in their ROM mask (i.e. not like the silabs or micronas parts which have a default firmware and the ability to patch the ROM via a software loaded code update). The firmware must be loaded every time the chip is brought out of reset or it won't work at all. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XC5000C 0x14b4 status
Correct. These are not parts that have any form of default firmware in their ROM mask (i.e. not like the silabs or micronas parts which have a default firmware and the ability to patch the ROM via a software loaded code update). The firmware must be loaded every time the chip is brought out of reset or it won't work at all. An image of the top of the tuner clearly showing any manufacturing markings would be welcome - assuming its accessible. It's a best picture I could find: http://www.reviews.ru/clause/over/T7_2/image41.jpg Also, at least 2 users was successful with using Behold TV T7 tuner with my Linux driver for it, then I shared it with others a year ago. HW vendor also says, that Linux FW 4.1.30.7 for XC5000C (from KernelLabs) reminds him an old 4.1 numeration scheme from 2010 year. But he was unable to understand it's date. Also he says, that for XC5000C they are already long time using 0.6.30.5, and it's always gives a 0x14b4. // Filename : Xc5000_firmwares.h // Generated : 2012/3/5 ¤W¤È 08:34:27 // Firmware version : 0.6; Release Number: 30.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] Enumeration of Media Interfaces
Introduction This RFC is a proposal that will hopefully fix the impasse w.r.t. DVB and MC. This proposal was the result of a private irc discussion between Laurent and myself. We were both on opposite sides of the earlier discussions and we decided to try to come to a solution between just the two of us. Sakari read an earlier version of this RFC and is OK with it as well, so now it is time to post it publicly. Hopefully Mauro likes it as well. This proposal is fairly high-level, and it does not attempt to go into details. Before doing that we need to know if all parties agree with the basic concept. Laurent, Sakari and myself are considering to meet in Finland for 2-3 days in July to hammer out the details. Proposal There are two types of drivers: the first (true for all non-MC drivers available today) controls the full pipeline through DVB/V4L/ALSA/etc. interfaces (generally these interfaces are device nodes). Applications for these drivers do not need to know which entities there are, knowing the interfaces is enough. The second type (the current set of MC drivers) exposes the hardware through media entities, and each entity can be controlled through a single device node: v4l-subdevX for subdevices, and videoX for DMA engines. Applications for these drivers will need to know the entities and how they are linked in order to configure the hardware correctly. The only interfaces they need to know are those that control each entity. The first type of driver will be called an 'interface-centric' driver, the second type will be called an 'entity-centric' driver in this document. Better names are welcome, but I can't talk about MC and non-MC drivers anymore since we want to use the MC for all drivers, so we need to come up with another name. So interface vs entity centric is the best I came up with. So we want to extend the use of the MC for all driver types, but we ran into a major difference of opinion of how to represent device nodes (are these media entities or properties of media entities?) and how to represent which entities a device node controls (through links, properties, something else?). This RFC presents a solution that I hope is acceptable to all. Media Interfaces The key difference of this proposal is that interfaces such as device nodes (but this can be an e.g. network interface as well) are represented by a new struct: media_interface. And that the list of interfaces that a media device has can be obtained by a new ioctl: MEDIA_IOC_G_INTERFACES. The ioctl will get all interfaces in a single call for efficiency and atomicity. So an interface is different from an entity, but both are first class citizens in the media controller. The kernel implementation will be that the media_entity in struct video_device will be replaced by a struct media_interface. And any media_interface that is created will link into a linked list maintained by the media_device struct. As a consequence of this change there now is no longer a media_entity for DMA engines to use: this will have to be created in the DMA engine driver itself, just as subdevice drivers have their own media_entity. This design is a lot cleaner as well since today the video_device struct used to represent a v4l-subdevX device node doesn't use the media_entity field, which is weird and it indicates that there is a problem with the datastructure design. However, with the introduction of a media_interface struct any video_device struct will always use the media_interface field. So device nodes are media interfaces, hardware blocks are media entities. In rare cases software blocks are allowed as media entities (e.g. the DVB software demux), but there should be very good reasons for doing so. This solves the problem of how to represent device nodes: these are no longer entities (which several developers were strongly opposed to), but neither are they properties (which the other camp was strongly opposed to). Instead they are now a new object: the media interface. The existing DVB/V4L/ etc. applications can use the new ioctl to obtain all interfaces exposed by the MC and open the device nodes that are relevant to them. They don't care about entities (some might in the future, but for now this is not something they need), they just want the interfaces. On the other hand applications written for MC devices need to enumerate the entities and entities may have an interface. For both backwards compatibility and as a shortcut the interface that controls the entity will remain available through the media_entity, but only if there is a one-to-one mapping between the entity and interface. This is always the case today. Both entities and interfaces will likely need properties, and we should use the same property API for both. Relationship between Entities and Interfaces This leaves the final missing piece: how to tell userspace
Re: [PATCH v3 1/1] V4L2: platform: Renesas R-Car JPEG codec driver
2015-06-26 15:14 GMT+03:00 Kamil Debski ka...@wypas.org: Hi Mikhail, On 26 June 2015 at 12:34, Mikhail Ulyanov mikhail.ulya...@cogentembedded.com wrote: Hi, Thanks everybody for comments. 2015-06-22 17:54 GMT+03:00 Kamil Debski ka...@wypas.org: Hi, I am adding Jacek Anaszewski to CC loop. He was working with the s5p-jpeg driver some time ago. I've spoken with him about questions in this email recently. Jacek, thank you for your comments :) On 18 June 2015 at 20:48, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Mikhail, (CC'ing Kamil Debski) On Wednesday 06 May 2015 01:03:10 Mikhail Ulianov wrote: On Mon, 04 May 2015 02:32:05 +0300 Laurent Pinchart wrote: [snip] [snip] +/* + * + * Queue operations + * + */ +static int jpu_queue_setup(struct vb2_queue *vq, + const struct v4l2_format *fmt, + unsigned int *nbuffers, unsigned int *nplanes, + unsigned int sizes[], void *alloc_ctxs[]) +{ + struct jpu_ctx *ctx = vb2_get_drv_priv(vq); + struct jpu_q_data *q_data; + unsigned int count = *nbuffers; + unsigned int i; + + q_data = jpu_get_q_data(ctx, vq-type); + + *nplanes = q_data-format.num_planes; + + /* + * Header is parsed during decoding and parsed information stored + * in the context so we do not allow another buffer to overwrite it. + * For now it works this way, but planned for alternation. It shouldn't be difficult to create a jpu_buffer structure that inherits from vb2_buffer and store the information there instead of in the context. You are absolutely right. But for this version i want to keep it simple and also at this moment not everything clear for me with this format autodetection feature we want to have e.g. for decoder if user requested 2 output buffers and then queue first with some valid JPEG with format -1-(so we setup queues format here), after that another one with format -2-... should we discard second one or just change format of queues? what about same situation if user already requested capture buffers. I mean relations with buf_prepare and queue_setup. AFAIU format should remain the same for all requested buffers. I see only one solid solution here - get rid of autodetection feature and rely only on format setted by user, so in this case we can just discard queued buffers with inappropriate format(kind of format validation in kernel). This solution will also work well with NV61, NV21, and semiplanar formats we want to add in next version. *But* with this solution header parsing must be done twice(in user and kernel spaces). I'm a little bit frustrated here :) Yes, it's a bit frustrating indeed. I'm not sure what to advise, I'm not too familiar with the m2m API for JPEG. Kamil, do you have a comment on that ? I am not sure whether it is good to get rid of header parsing by the driver/hardware option. I agree that the buffers should have a consistent format and size. Maybe the way to go would be to allow header parsing by the hardware, but to stop processing when the format has changed? Other solution would be to use the V4L2_EVENT_SOURCE_CHANGE event to inform user space about the change. User space then could check whether the buffers are sufficient or reallocate them. Similar to what happens in MFC when format changes. For me implementing resolution change in JPEG seems like an overkill, but maybe you have a use case that would benefit from this. Initially the JPEG decoder was designed and written as a one shot device. Could you give an example of such use case? The possible use case I can imagine is having an M-JPEG stream where all JPEGs have the same dimensions and format. There I can see some benefits from having more than one buffer on the queues. Then there would be no change in the buffer parameters, so this should work. That's correct. It's exactly our use case, but we have few cameras. So we serialize buffers in user space and sometimes one(or more) cameras have different configuration. I think i should go with stop processing option. It could be possible to have frames from each camera decoded in different contexts. This way the configuration should be consistent for all frames in a context. It will require more memory (more buffers) though... Yes, that's an option. Anyway in latest (v4) version i decided to drop frames with different format and make driver more strict in general. I think it's more robust in any case. Nobody knows what user will pass to kernel :) [snip] [snip] Best wishes, Kamil Debski -- W.B.R, Mikhail. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: XC5000C 0x14b4 status
IMHO, the best is to get the latest firmware licensed is the best thing to do. Does that new xc5000c come with a firmware pre-loaded already? I've got firmware here that is indicated as being for the xc5300 (i.e. 0x14b4). That said, I am not sure if it's the same as the original 5000c firmware. Definitely makes sense to do an I2C dump and compare the firmware images since using the wrong firmware can damage the part. I'm not against an additional #define for the 0x14b4 part ID, but it shouldn't be accepted upstream until we have corresponding firmware and have seen the tuner working. Do you have digital signal lock working with this device under Linux and the issue is strictly with part identification? Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XC5000C 0x14b4 status
From: Mauro Carvalho Chehab To: Unembossed Name Cc: linux-media@vger.kernel.org; Devin Heitmueller Sent: Friday, June 26, 2015 4:22 PM Subject: Re: XC5000C 0x14b4 status After that RF tuner identification became always successful. I had a conversation with a hardware vendor. Now I can say, that such behaviour, most likely, because of early firmware for XC5000C. This hardware vendor is using for his Windows driver a latest firmware, and reading Product ID register always gives 0x14b4 status. As he says, 0x1388 status is only for previous XC5000 IC. (Without C at end of a P/N) Is this possible to patch xc5000.c with something like this: #define XC_PRODUCT_ID_FW_LOADED 0x1388 +#define XC_PRODUCT_ID_FW_LOADED_XC5000C 0x14b4 case XC_PRODUCT_ID_FW_LOADED: + case XC_PRODUCT_ID_FW_LOADED_XC5000C: printk(KERN_INFO xc5000: Successfully identified at address 0x%02x\n, Or to try to get a chip vendor's permission for using a latest firmware for XC5000C in Linux, and then anyway, patch the driver? IMHO, the best is to get the latest firmware licensed is the best thing to do. Agreed. If that possible of course. Does that new xc5000c come with a firmware pre-loaded already? It's not new IC. It's XC5000C. Maybe i was interpreted wrong. As I have understood, such behaviour can depends from FW version. HW vendor says, that with his latest FW he always gets response 0x14b4. Not a 0x1388. And I think, that these ICs still come without pre-loaded FW. HW vendor also didn't says anything about FW pre-load possibility. Best regards. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XC5000C 0x14b4 status
It's not new IC. It's XC5000C. Maybe i was interpreted wrong. As I have understood, such behaviour can depends from FW version. HW vendor says, that with his latest FW he always gets response 0x14b4. Ah, so you're running a completely different firmware image? Well in that case that would explain the different response for the firmware loaded indication. Not a 0x1388. And I think, that these ICs still come without pre-loaded FW. HW vendor also didn't says anything about FW pre-load possibility. Correct. These are not parts that have any form of default firmware in their ROM mask (i.e. not like the silabs or micronas parts which have a default firmware and the ability to patch the ROM via a software loaded code update). The firmware must be loaded every time the chip is brought out of reset or it won't work at all. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/1] V4L2: platform: Add Renesas R-Car JPEG codec driver.
Here's the driver for the Renesas R-Car JPEG processing unit. The driver is implemented within the V4L2 framework as a memory-to-memory device. It presents two video nodes to userspace, one for the encoding part, and one for the decoding part. It was found that the only working mode for encoding is no markers output, so we generate markers with software. In the current version of driver we also use software JPEG header parsing because with hardware parsing performance is lower than desired. From a userspace point of view the process is typical (S_FMT, REQBUF, optionally QUERYBUF, QBUF, STREAMON, DQBUF) for both the source and destination queues. STREAMON can return -EINVAL in case of mismatch of output and capture queues format. Also during decoding driver can return buffers if queued buffer with JPEG image contains image with inappropriate subsampling (e.g. 4:2:0 in JPEG and 4:2:2 in capture). If JPEG image and queue format dimensions differ driver will return buffer on QBUF with VB2_BUF_STATE_ERROR flag. During encoding the available formats are: V4L2_PIX_FMT_NV12M, V4L2_PIX_FMT_NV12, V4L2_PIX_FMT_NV16, V4L2_PIX_FMT_NV16M for source and V4L2_PIX_FMT_JPEG for destination. During decoding the available formats are: V4L2_PIX_FMT_JPEG for source and V4L2_PIX_FMT_NV12M, V4L2_PIX_FMT_NV16M, V4L2_PIX_FMT_NV12, V4L2_PIX_FMT_NV16 for destination. Performance of current version: 1280x800 NV12 image encoding/decoding decoding ~122 FPS encoding ~191 FPS Signed-off-by: Mikhail Ulyanov mikhail.ulya...@cogentembedded.com --- Changes since v3: - driver file renamed to rcar_jpu.c - semiplanar formats NV12 and NV16 support - new callbacks streamon, job_abort and stop_streaming - extra processing error information printout irq handler - fill in JPEG header for encoded buffer in buf_finish - wrapped reading/writing to registers - vb2_set_plane_payload only for necessary buffer in buf_prepare - multiple buffers now supported - removed format setup with parsed info; rely only on users info - JPEG header parser redesigned - video_device structs embedded - video_device_alloc/release removed - name filed in format description removed - remove g_selection - start_streaming removed Changes since v2: - Kconfig entry reordered - unnecessary clk_disable_unprepare(jpu-clk) removed - ref_count fixed in jpu_resume - enable DMABUF in src_vq-io_modes - remove jpu_s_priority jpu_g_priority - jpu_g_selection fixed - timeout in jpu_reset added and hardware reset reworked - remove unused macros - JPEG header parsing now is software because of performance issues based on s5p-jpeg code - JPEG header generation redesigned: JPEG header(s) pre-generated and memcpy'ed on encoding we only fill the necessary fields more transparent header format description - S_FMT, G_FMT and TRY_FMT hooks redesigned partially inspired by VSP1 driver code - some code was reformatted - image formats handling redesigned - multi-planar V4L2 API now in use - now passes v4l2-compliance tool check Cnanges since v1: - s/g_fmt function simplified - default format for queues added - dumb vidioc functions added to be in compliance with standard api: jpu_s_priority, jpu_g_priority - standard v4l2_ctrl_subscribe_event and v4l2_event_unsubscribe now in use by the same reason drivers/media/platform/Kconfig| 11 + drivers/media/platform/Makefile |1 + drivers/media/platform/rcar_jpu.c | 1753 + 3 files changed, 1765 insertions(+) create mode 100644 drivers/media/platform/rcar_jpu.c diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 765bffb..33a457c 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -220,6 +220,17 @@ config VIDEO_SH_VEU Support for the Video Engine Unit (VEU) on SuperH and SH-Mobile SoCs. +config VIDEO_RENESAS_JPU + tristate Renesas JPEG Processing Unit + depends on VIDEO_DEV VIDEO_V4L2 + select VIDEOBUF2_DMA_CONTIG + select V4L2_MEM2MEM_DEV + ---help--- + This is a V4L2 driver for the Renesas JPEG Processing Unit. + + To compile this driver as a module, choose M here: the module + will be called jpu. + config VIDEO_RENESAS_VSP1 tristate Renesas VSP1 Video Processing Engine depends on VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API HAS_DMA diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index a49936b..3879f23 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -42,6 +42,7 @@ obj-$(CONFIG_VIDEO_SH_VOU)+= sh_vou.o obj-$(CONFIG_SOC_CAMERA) += soc_camera/ +obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o obj-$(CONFIG_VIDEO_RENESAS_VSP1) += vsp1/ obj-y += omap/
Re: XC5000C 0x14b4 status
IMHO, the best is to get the latest firmware licensed is the best thing to do. Does that new xc5000c come with a firmware pre-loaded already? I've got firmware here that is indicated as being for the xc5300 (i.e. 0x14b4). That said, I am not sure if it's the same as the original 5000c firmware. Definitely makes sense to do an I2C dump and compare the firmware images since using the wrong firmware can damage the part. As I have understood, reading Poduct ID register (together with reading checksum register) is primarily for check integrity of uploaded FW. It's not indicates IC's P/N which FW should belong. I'm not against an additional #define for the 0x14b4 part ID, but it shouldn't be accepted upstream until we have corresponding firmware and have seen the tuner working. Do you have digital signal lock working with this device under Linux and the issue is strictly with part identification? With that RF tuner IC, Linux driver and public available FW for XC5000C (from Kernel Labs) I successfully received analog and digital transmissions over an air. Didn't checked it with DVC-C though. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
On Fri, 2015-06-26 at 10:45 +0200, Ingo Molnar wrote: * Luis R. Rodriguez mcg...@suse.com wrote: On Thu, Jun 25, 2015 at 08:51:47AM +0200, Ingo Molnar wrote: * Luis R. Rodriguez mcg...@do-not-panic.com wrote: From: Luis R. Rodriguez mcg...@suse.com On built-in kernels this warning will always splat as this is part of the module init. Fix that by shifting the PAT requirement check out under the code that does the quasi-probe for the device. This device driver relies on an existing driver to find its own devices, it looks for that device driver and its own found devices, then uses driver_for_each_device() to try to see if it can probe each of those devices as a frambuffer device with ivtvfb_init_card(). We tuck the PAT requiremenet check then on the ivtvfb_init_card() call making the check at least require an ivtv device present before complaining. Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot] Signed-off-by: Luis R. Rodriguez mcg...@suse.com --- drivers/media/pci/ivtv/ivtvfb.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c index 4cb365d..8b95eef 100644 --- a/drivers/media/pci/ivtv/ivtvfb.c +++ b/drivers/media/pci/ivtv/ivtvfb.c @@ -38,6 +38,8 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +#define pr_fmt(fmt) KBUILD_MODNAME : fmt + #include linux/module.h #include linux/kernel.h #include linux/fb.h @@ -1171,6 +1173,13 @@ static int ivtvfb_init_card(struct ivtv *itv) { int rc; +#ifdef CONFIG_X86_64 + if (pat_enabled()) { + pr_warn(ivtvfb needs PAT disabled, boot with nopat kernel parameter\n); + return -ENODEV; + } +#endif + if (itv-osd_info) { IVTVFB_ERR(Card %d already initialised\n, ivtvfb_card_id); return -EBUSY; Same argument as for ipath: why not make arch_phys_wc_add() fail on PAT and return -1, and check it in arch_phys_wc_del()? The arch_phys_wc_add() is a no-op for PAT systems but for PAT to work we need not only need to add this in where we replace the MTRR call but we also need to convert ioremap_nocache() calls to ioremap_wc() but only if things were split up already. Hi Ingo, We don't need to do that: for such legacy drivers we can fall back to UC just fine, and inform the user that by booting with 'nopat' the old behavior will be back... This is really a user experience decision. IMO anyone who is still using ivtvfb and an old conventional PCI PVR-350 to render, at SDTV resolution, an X Desktop display or video playback on a television screen, isn't going to give a hoot about modern things like PAT. The user will simply want the framebuffer performance they are accustomed to having with their system. UC will probably yield unsatisfactory performance for an ivtvfb framebuffer. With that in mind, I would think it better to obviously and clearly disable the ivtvfb framebuffer module with PAT enabled, so the user will check the log and read the steps needed to obtain acceptable performance. Maybe that's me just wanting to head off the poor ivtvfb performance with latest kernel bug reports. Whatever the decision, my stock response to bug reports related to this will always be What do the logs say?. Regards, Andy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html