date:20170608

On Fri, Jun 9, 2017 at 1:43 AM, Sakari Ailus  wrote:
> Hi Tomasz,
>
> On Wed, Jun 07, 2017 at 05:35:13PM +0900, Tomasz Figa wrote:
>> Hi Yong, Tuukka,
>>
>> Continuing from yesterday. Please see comments inline.
>>
>> > On Tue, Jun 6, 2017 at 5:39 AM, Yong Zhi  wrote:
>> [snip]
>> >> +   ptr = ipu3_mmu_alloc_page_table(mmu_dom, false);
>> >> +   if (!ptr)
>> >> +   goto fail_page_table;
>> >> +
>> >> +   /*
>> >> +* We always map the L1 page table (a single page as well as
>> >> +* the L2 page tables).
>> >> +*/
>> >> +   mmu_dom->dummy_l2_tbl = virt_to_phys(ptr) >> IPU3_MMU_PAGE_SHIFT;
>> >> +   mmu_dom->pgtbl = ipu3_mmu_alloc_page_table(mmu_dom, true);
>> >> +   if (!mmu_dom->pgtbl)
>> >> +   goto fail_page_table;
>> >> +
>> >> +   spin_lock_init(_dom->lock);
>> >> +   return _dom->domain;
>> >> +
>> >> +fail_page_table:
>> >> +   free_page((unsigned long)TBL_VIRT_ADDR(mmu_dom->dummy_page));
>> >> +   free_page((unsigned long)TBL_VIRT_ADDR(mmu_dom->dummy_l2_tbl));
>> >> +fail_get_page:
>> >> +   kfree(mmu_dom);
>> >> +   return NULL;
>> >> +}
>> >> +
>> >> +static void ipu3_mmu_domain_free(struct iommu_domain *dom)
>> >> +{
>> >> +   struct ipu3_mmu_domain *mmu_dom =
>> >> +   container_of(dom, struct ipu3_mmu_domain, domain);
>> >> +   uint32_t l1_idx;
>> >> +
>> >> +   for (l1_idx = 0; l1_idx < IPU3_MMU_L1PT_PTES; l1_idx++)
>> >> +   if (mmu_dom->pgtbl[l1_idx] != mmu_dom->dummy_l2_tbl)
>> >> +   free_page((unsigned long)
>> >> + TBL_VIRT_ADDR(mmu_dom->pgtbl[l1_idx]));
>> >> +
>> >> +   free_page((unsigned long)TBL_VIRT_ADDR(mmu_dom->dummy_page));
>> >> +   free_page((unsigned long)TBL_VIRT_ADDR(mmu_dom->dummy_l2_tbl));
>>
>> I might be overly paranoid, but reading back kernel virtual pointers
>> from device accessible memory doesn't seem safe to me. Other drivers
>> keep kernel pointers of page tables in a dedicated array (it's only 8K
>> of memory, but much better safety).
>
> Do you happen to have an example of that?

Hmm, looks like I misread rockchip-iommu code. Let me quietly back off
from this claim, sorry.

>
> All system memory typically is accessible for devices, I think you wanted to
> say that the device is intended to access that memory. Albeit for reading
> only.

Unless you activate DMAR and make only the memory you want to be
accessible to your devices. I know DMAR is a device too, but there is
a difference between a system level fixed function IOMMU and a PCI
device running a closed source firmware. Still, given Robin's reply,
current DMA and IOMMU frameworks might not be able to handle this
easily, so let's temporarily forget about this setup. We might revisit
it later, with incremental patches, anyway.

>
> ...
>
>> >> +static int ipu3_mmu_bus_remove(struct device *dev)
>> >> +{
>> >> +   struct ipu3_mmu *mmu = dev_get_drvdata(dev);
>> >> +
>> >> +   put_iova_domain(>iova_domain);
>> >> +   iova_cache_put();
>>
>> Don't we need to set the L1 table address to something invalid and
>> invalidate the TLB, so that the IOMMU doesn't reference the page freed
>> below anymore?
>
> I think the expectation is that if a device gets removed, its memory is
> unmapped by that time. Unmapping that memory will cause explicit TLB flush.

Right, but that will only mark the L2 entries as invalid. The L1 table
will still point to the L2 tables installed earlier and the MMU page
directory register will still point to the L1 table, despite the call
below freeing all the associated memory.

Best regards,
Tomasz

Re: [PATCH 1/3] media: ngene: Replace semaphore cmd_mutex with mutex

On 8 June 2017 at 20:40, Arnd Bergmann  wrote:
> On Thu, Jun 8, 2017 at 12:04 PM, Binoy Jayan  wrote:
>> The semaphore 'cmd_mutex' is used as a simple mutex, so
>> it should be written as one. Semaphores are going away in the future.
>>
>> Signed-off-by: Binoy Jayan 
>> ---
>
>> @@ -1283,7 +1283,7 @@ static int ngene_load_firm(struct ngene *dev)
>>
>>  static void ngene_stop(struct ngene *dev)
>>  {
>> -   down(>cmd_mutex);
>> +   mutex_lock(>cmd_mutex);
>> i2c_del_adapter(&(dev->channel[0].i2c_adapter));
>> i2c_del_adapter(&(dev->channel[1].i2c_adapter));
>> ngwritel(0, NGENE_INT_ENABLE);
>
> Are you sure about this one? There is only one mutex_lock() and
> then the structure gets freed without a corresponding mutex_unlock().
>
> I suspect this violates some rules of mutexes, either when compile
> testing with "make C=1", or when running with lockdep enabled.
>
> Can we actually have a concurrently held mutex at the time we
> get here? If not, using mutex_destroy() in place of the down()
> may be the right answer.

I noticed the missing 'up' here, but may be semaphores do not have
to adhere to that rule? Thank you for pointing out that. I'll check the
concurrency part. By the way why do we need mutex_destoy?
To debug an aberrate condition?

Thanks,
Binoy

Re: [PATCH v2 2/4] [media] davinci: vpif_capture: get subdevs from DT when available

2017-06-08 Thread Kevin Hilman

On Wed, Jun 7, 2017 at 11:29 PM, Hans Verkuil  wrote:
> On 07/06/17 01:37, Kevin Hilman wrote:
>> Enable  getting of subdevs from DT ports and endpoints.
>>
>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>> am437x-vpfe.c
>>
>> Signed-off-by: Kevin Hilman 
>> ---
>>  drivers/media/platform/davinci/vpif_capture.c | 126 
>> +-
>>  drivers/media/platform/davinci/vpif_display.c |   5 +
>>  include/media/davinci/vpif_types.h|   9 +-
>>  3 files changed, 134 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c 
>> b/drivers/media/platform/davinci/vpif_capture.c
>> index fc5c7622660c..b9d927d1e5a8 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -22,6 +22,8 @@
>>  #include 
>>
>>  #include 
>> +#include 
>
> v4l2-of.h no longer exists, so this v2 is wrong. Unfortunately this patch has
> already been merged in our master. I'm not sure how this could have slipped 
> past
> both my and Mauro's patch testing (and yours, for that matter).

I have that file in the various trees I tested agains.

> Can you fix this and post a patch on top of the media master that makes this
> compile again?

Sorry for the dumb question, but what tree are you referring to?  I
tried the master branch of both [1] and [2] and both seem to have that
include.

Kevin

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git
[2] git://linuxtv.org/mchehab/media-next.git

[media-af9013] question about return value in function af9013_wr_regs()

2017-06-08 Thread Gustavo A. R. Silva



Hello everybody,

While looking into Coverity ID 1227035 I ran into the following piece  
of code at drivers/media/dvb-frontends/af9013.c:595:


595/* program CFOE coefficients */
596if (c->bandwidth_hz != state->bandwidth_hz) {
597for (i = 0; i < ARRAY_SIZE(coeff_lut); i++) {
598if (coeff_lut[i].clock == state->config.clock &&
599coeff_lut[i].bandwidth_hz ==  
c->bandwidth_hz) {

600break;
601}
602}
603
604/* Return an error if can't find bandwidth or the  
right clock */

605if (i == ARRAY_SIZE(coeff_lut))
606return -EINVAL;
608ret = af9013_wr_regs(state, 0xae00, coeff_lut[i].val,
609sizeof(coeff_lut[i].val));
610}
611
612/* program frequency control */
613if (c->bandwidth_hz != state->bandwidth_hz || state->first_tune) {
614/* get used IF frequency */
615if (fe->ops.tuner_ops.get_if_frequency)
616fe->ops.tuner_ops.get_if_frequency(fe,  
_frequency);

617else
618if_frequency = state->config.if_frequency;
619
620dev_dbg(>i2c->dev, "%s: if_frequency=%d\n",
621__func__, if_frequency);
622
623sampling_freq = if_frequency;
624
625while (sampling_freq > (state->config.clock / 2))
626sampling_freq -= state->config.clock;
627
628if (sampling_freq < 0) {
629sampling_freq *= -1;
630spec_inv = state->config.spec_inv;
631} else {
632spec_inv = !state->config.spec_inv;
633}
634
635freq_cw = af9013_div(state, sampling_freq,  
state->config.clock,

63623);
637
638if (spec_inv)
639freq_cw = 0x80 - freq_cw;
640
641buf[0] = (freq_cw >>  0) & 0xff;
642buf[1] = (freq_cw >>  8) & 0xff;
643buf[2] = (freq_cw >> 16) & 0x7f;
644
645freq_cw = 0x80 - freq_cw;
646
647buf[3] = (freq_cw >>  0) & 0xff;
648buf[4] = (freq_cw >>  8) & 0xff;
649buf[5] = (freq_cw >> 16) & 0x7f;
650
651ret = af9013_wr_regs(state, 0xd140, buf, 3);
652if (ret)
653goto err;
654
655ret = af9013_wr_regs(state, 0x9be7, buf, 6);
656if (ret)
657goto err;
658}
659
660/* clear TPS lock flag */
661ret = af9013_wr_reg_bits(state, 0xd330, 3, 1, 1);
662if (ret)
663goto err;
664
665/* clear MPEG2 lock flag */
666ret = af9013_wr_reg_bits(state, 0xd507, 6, 1, 0);
667if (ret)
668goto err;
669
670/* empty channel function */
671ret = af9013_wr_reg_bits(state, 0x9bfe, 0, 1, 0);
672if (ret)
673goto err;
674
675/* empty DVB-T channel function */
676ret = af9013_wr_reg_bits(state, 0x9bc2, 0, 1, 0);
677if (ret)
678goto err;
679

The issue here is that the value stored in variable _ret_ at line 608,  
is not being evaluated as it happens at line 662, 667, 672 and 677.  
Then after looking into function af9013_wr_regs(), I noticed that this  
function always returns zero, no matter what, as you can see below:


121static int af9013_wr_regs(struct af9013_state *priv, u16 reg, const  
u8 *val,

122int len)
123{
124int ret, i;
125u8 mbox = (0 << 7)|(0 << 6)|(1 << 1)|(1 << 0);
126
127if ((priv->config.ts_mode == AF9013_TS_USB) &&
128((reg & 0xff00) != 0xff00) && ((reg & 0xff00) != 0xae00)) {
129mbox |= ((len - 1) << 2);
130ret = af9013_wr_regs_i2c(priv, mbox, reg, val, len);
131} else {
132for (i = 0; i < len; i++) {
133ret = af9013_wr_regs_i2c(priv, mbox, reg+i,  
val+i, 1);

134if (ret)
135goto err;
136}
137}
138
139err:
140return 0;
141}

So I am wondering if such function is really intended to ignore the  
return value of af9013_wr_regs_i2c()?

If that is the case maybe it should be refactored as follows:

--- a/drivers/media/dvb-frontends/af9013.c
+++ b/drivers/media/dvb-frontends/af9013.c
@@ -118,26 +118,21 @@ static int af9013_rd_regs_i2c(struct  
af9013_state *priv, u8 mbox, u16 reg,

 }

 /* write multiple registers */
-static int af9013_wr_regs(struct af9013_state *priv, u16 reg, const u8 *val,
+static void af9013_wr_regs(struct af9013_state *priv, u16 reg, const u8 *val,
int len)
 {
-

Re: [RFC 00/10] V4L2 explicit synchronization support

2017-06-08 Thread Shuah Khan

Hi Gustavo,

On Thu, Jun 8, 2017 at 2:17 PM, Mauro Carvalho Chehab
 wrote:
> Hi Gustavo,
>
> Em Wed, 24 May 2017 21:31:01 -0300
> Gustavo Padovan  escreveu:
>
>> Hi all,
>>
>> I've been working on the v2 of this series, but I think I hit a blocker
>> when trying to cover the case where the driver asks to requeue the
>> buffer. It is related to the out-fence side.
>>
>> In the current implementation we return on QBUF an out-fence fd that is not
>> tied to any buffer, because we don't know the queueing order until the
>> buffer is queued to the driver. Then when the buffer is queued we use
>> the BUF_QUEUED event to notify userspace of the index of the buffer,
>> so now userspace knows the buffer associated to the out-fence fd
>> received earlier.
>>
>> Userspace goes ahead and send a DRM Atomic Request to the kernel to
>> display that buffer on the screen once the fence signals. If it is
>> a nonblocking request the fence waiting is past the check phase, thus
>> it isn't allowed to fail anymore.
>>
>> But now, what happens if the V4L2 driver calls buffer_done() asking
>> to requeue the buffer. That means the operation failed and can't
>> signal the fence, starving the DRM side.
>>
>> We need to fix that. The only way I can see is to guarantee ordering of
>> buffers when out-fences are used. Ordering is something that HAL3 needs
>> to so maybe there is more than one reason to do it like this. I'm not
>> a V4L2 expert, so I don't know all the consequences of such a change.
>>
>> Any other ideas?
>>
>> The current patchset is at:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/padovan/linux.git/log/?h=v4l2-fences

Do you plan to send the v2 out? I did a quick review and have a few comments.

[media] vb2: split out queueing from vb_core_qbuf()

It changes the sequence a bit.

/* Fill buffer information for the userspace */
  if (pb)
  call_void_bufop(q, fill_user_buffer, vb, pb);

With the changes - user information is filled before __enqueue_in_driver(vb);

Anyway, it might be a good idea to send the v2 out for review and we can review
patches in detail. I am hoping to test your patch series on odroid-xu4
next week.
Could you please add me to the thread as well as include me when you send
v2 and subsequent versions.

thanks,
-- Shuah

Re: [PATCH v8 14/34] ARM: dts: imx6-sabreauto: add the ADV7180 video decoder

2017-06-08 Thread Steve Longerbeam




On 06/08/2017 01:25 PM, Tim Harvey wrote:



Steve,

You need to remove the fim node now that you've moved this to V4L2 controls.



Yep, I caught this just after sending the v8 patchset. I'll send
a v9 of this patch.

Steve

Re: [PATCH v8 14/34] ARM: dts: imx6-sabreauto: add the ADV7180 video decoder

2017-06-08 Thread Tim Harvey

On Wed, Jun 7, 2017 at 11:33 AM, Steve Longerbeam  wrote:
> Enables the ADV7180 decoder sensor. The ADV7180 connects to the
> parallel-bus mux input on ipu1_csi0_mux.
>
> The ADV7180 power pin is via max7310_b port expander.
>
> Signed-off-by: Steve Longerbeam 
>
> - Use IRQ_TYPE_LEVEL_LOW instead of 0x8 for interrupt type for clarity.
> - For 8-bit parallel IPU1-CSI0 bus connection only data[12-19] are used.
>
> Signed-off-by: Tim Harvey 
> ---
>  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 50 
> 
>  1 file changed, 50 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi 
> b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> index 1212f82..c24af28 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> @@ -124,6 +124,21 @@
> #size-cells = <0>;
> reg = <1>;
>
> +   adv7180: camera@21 {
> +   compatible = "adi,adv7180";
> +   reg = <0x21>;
> +   powerdown-gpios = <_b 2 
> GPIO_ACTIVE_LOW>;
> +   interrupt-parent = <>;
> +   interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> +
> +   port {
> +   adv7180_to_ipu1_csi0_mux: endpoint {
> +   remote-endpoint = 
> <_csi0_mux_from_parallel_sensor>;
> +   bus-width = <8>;
> +   };
> +   };
> +   };
> +
> max7310_a: gpio@30 {
> compatible = "maxim,max7310";
> reg = <0x30>;
> @@ -151,6 +166,25 @@
> };
>  };
>
> +_csi0_from_ipu1_csi0_mux {
> +   bus-width = <8>;
> +};
> +
> +_csi0_mux_from_parallel_sensor {
> +   remote-endpoint = <_to_ipu1_csi0_mux>;
> +   bus-width = <8>;
> +};
> +
> +_csi0 {
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_ipu1_csi0>;
> +
> +   /* enable frame interval monitor on this port */
> +   fim {
> +   status = "okay";
> +   };

Steve,

You need to remove the fim node now that you've moved this to V4L2 controls.

Regards,

Tim

Re: [RFC 00/10] V4L2 explicit synchronization support

Hi Gustavo,

Em Wed, 24 May 2017 21:31:01 -0300
Gustavo Padovan  escreveu:

> Hi all,
> 
> I've been working on the v2 of this series, but I think I hit a blocker
> when trying to cover the case where the driver asks to requeue the
> buffer. It is related to the out-fence side.
> 
> In the current implementation we return on QBUF an out-fence fd that is not
> tied to any buffer, because we don't know the queueing order until the
> buffer is queued to the driver. Then when the buffer is queued we use
> the BUF_QUEUED event to notify userspace of the index of the buffer,
> so now userspace knows the buffer associated to the out-fence fd
> received earlier.
> 
> Userspace goes ahead and send a DRM Atomic Request to the kernel to
> display that buffer on the screen once the fence signals. If it is 
> a nonblocking request the fence waiting is past the check phase, thus
> it isn't allowed to fail anymore.
> 
> But now, what happens if the V4L2 driver calls buffer_done() asking
> to requeue the buffer. That means the operation failed and can't
> signal the fence, starving the DRM side.
> 
> We need to fix that. The only way I can see is to guarantee ordering of
> buffers when out-fences are used. Ordering is something that HAL3 needs
> to so maybe there is more than one reason to do it like this. I'm not
> a V4L2 expert, so I don't know all the consequences of such a change.
> 
> Any other ideas?
> 
> The current patchset is at:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/padovan/linux.git/log/?h=v4l2-fences

Currently, nothing warrants that buffers will be returned in order,
but that should be the case of most drivers. I guess the main
exception would be mem2mem codec drivers. On those, the driver
or the hardware may opt to reorder the buffers.

If this is a mandatory requirement for explicit fences to work, then
we'll need to be able to explicitly enable it, per driver, and
clearly document that drivers using it *should* warrant that the
dequeued buffer will follow the queued order.

We may need to modify VB2 in order to enforce it or return an
error if the order doesn't match.

-- 
Thanks,
Mauro

Re: [PATCH v4] v4l: subdev: tolerate null in media_entity_to_v4l2_subdev

Em Thu, 8 Jun 2017 22:32:10 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> On Thu, Jun 08, 2017 at 03:00:22PM -0300, Mauro Carvalho Chehab wrote:
> > Em Wed,  7 Jun 2017 10:52:07 +0100
> > Kieran Bingham  escreveu:
> >   
> > > From: Kieran Bingham 
> > > 
> > > Return NULL, if a null entity is parsed for it's v4l2_subdev
> > > 
> > > Signed-off-by: Kieran Bingham   
> > 
> > Could you please improve this patch description?
> > 
> > I'm unsure if this is a bug fix, or some sort of feature...
> > 
> > On what situations would a null entity be passed to this function?  
> 
> I actually proposed this patch. This change is simply for convenience ---
> the caller doesn't need to make sure the subdev is non-NULL, possibly
> obtained from e.g. media_entity_remote_pad() which returns NULL all links to
> the pad are disabled. This is a recurring pattern, and making this change
> avoids an additional check.
> 
> Having something along these lines in the patch description wouldn't hurt.

Patch added, with a description based on the above.

Thanks!

Mauro

[PATCH 1/1] v4l: ctrls: Add a control for digital gain

Add V4L2_CID_DIGITAL_GAIN to control explicitly digital gain.

We already have analogue gain control which the digital gain control
complements. Typically higher quality images are obtained using analogue
gain only as the digital gain does not add information to the image
(rather it may remove it).

Signed-off-by: Sakari Ailus 
---
 Documentation/media/uapi/v4l/extended-controls.rst | 7 +++
 drivers/media/v4l2-core/v4l2-ctrls.c   | 1 +
 include/uapi/linux/v4l2-controls.h | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/v4l/extended-controls.rst 
b/Documentation/media/uapi/v4l/extended-controls.rst
index abb1057..60b73b0 100644
--- a/Documentation/media/uapi/v4l/extended-controls.rst
+++ b/Documentation/media/uapi/v4l/extended-controls.rst
@@ -3021,6 +3021,13 @@ Image Process Control IDs
 The video deinterlacing mode (such as Bob, Weave, ...). The menu items are
 driver specific and are documented in :ref:`v4l-drivers`.
 
+``V4L2_CID_DIGITAL_GAIN (integer)``
+Digital gain is the value by which all colour components
+are multiplied by. Typically the digital gain applied is the
+control value divided by e.g. 0x100, meaning that to get no
+digital gain the control value needs to be 0x100. This is also
+typically the default.
+
 
 .. _dv-controls:
 
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index ec42872..6318365 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -886,6 +886,7 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_PIXEL_RATE:   return "Pixel Rate";
case V4L2_CID_TEST_PATTERN: return "Test Pattern";
case V4L2_CID_DEINTERLACING_MODE:   return "Deinterlacing Mode";
+   case V4L2_CID_DIGITAL_GAIN: return "Digital Gain";
 
/* DV controls */
/* Keep the order of the 'case's the same as in v4l2-controls.h! */
diff --git a/include/uapi/linux/v4l2-controls.h 
b/include/uapi/linux/v4l2-controls.h
index 0d2e1e0..0cdb8eb 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -893,7 +893,7 @@ enum v4l2_jpeg_chroma_subsampling {
 #define V4L2_CID_PIXEL_RATE(V4L2_CID_IMAGE_PROC_CLASS_BASE 
+ 2)
 #define V4L2_CID_TEST_PATTERN  (V4L2_CID_IMAGE_PROC_CLASS_BASE 
+ 3)
 #define V4L2_CID_DEINTERLACING_MODE(V4L2_CID_IMAGE_PROC_CLASS_BASE 
+ 4)
-
+#define V4L2_CID_DIGITAL_GAIN  (V4L2_CID_IMAGE_PROC_CLASS_BASE 
+ 5)
 
 /*  DV-class control IDs defined by V4L2 */
 #define V4L2_CID_DV_CLASS_BASE (V4L2_CTRL_CLASS_DV | 0x900)
-- 
2.1.4

Re: [PATCH v4] v4l: subdev: tolerate null in media_entity_to_v4l2_subdev

Hi Mauro,

On Thu, Jun 08, 2017 at 03:00:22PM -0300, Mauro Carvalho Chehab wrote:
> Em Wed,  7 Jun 2017 10:52:07 +0100
> Kieran Bingham  escreveu:
> 
> > From: Kieran Bingham 
> > 
> > Return NULL, if a null entity is parsed for it's v4l2_subdev
> > 
> > Signed-off-by: Kieran Bingham 
> 
> Could you please improve this patch description?
> 
> I'm unsure if this is a bug fix, or some sort of feature...
> 
> On what situations would a null entity be passed to this function?

I actually proposed this patch. This change is simply for convenience ---
the caller doesn't need to make sure the subdev is non-NULL, possibly
obtained from e.g. media_entity_remote_pad() which returns NULL all links to
the pad are disabled. This is a recurring pattern, and making this change
avoids an additional check.

Having something along these lines in the patch description wouldn't hurt.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH 01/11] [media] dvb-core/dvb_ca_en50221.c: Rename STATUSREG_??

2017-06-08 Thread Jasmin J.


Hello Mauro!

> The idea behind patch 04/11 makes sense to me. I'll review it carefully
> after having everything applied.
Please don't do that now!
The first series will then not be easily changed, because it changes the state
machine. I will provide the first series and when it is applied the second
series again, now that I know what you like.

BR,
   Jasmin

[PATCH] media/cec.h: use IS_REACHABLE instead of IS_ENABLED

Fix messages like this:

adv7842.c:(.text+0x2edadd): undefined reference to `cec_unregister_adapter'

when CEC_CORE=m but the driver including media/cec.h is built-in. In that case
the static inlines provided in media/cec.h should be used by that driver.

Signed-off-by: Hans Verkuil 
---
diff --git a/include/media/cec.h b/include/media/cec.h
index bfa88d4d67e1..201f060978da 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -206,7 +206,7 @@ static inline bool cec_is_sink(const struct cec_adapter 
*adap)
 #define cec_phys_addr_exp(pa) \
((pa) >> 12), ((pa) >> 8) & 0xf, ((pa) >> 4) & 0xf, (pa) & 0xf

-#if IS_ENABLED(CONFIG_CEC_CORE)
+#if IS_REACHABLE(CONFIG_CEC_CORE)
 struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
void *priv, const char *name, u32 caps, u8 available_las);
 int cec_register_adapter(struct cec_adapter *adap, struct device *parent);

Re: [PATCH 03/12] intel-ipu3: Add DMA API implementation

2017-06-08 Thread Robin Murphy

On 08/06/17 15:35, Tomasz Figa wrote:
> On Thu, Jun 8, 2017 at 10:22 PM, Robin Murphy  wrote:
>> On 07/06/17 10:47, Tomasz Figa wrote:
>>> Hi Yong,
>>>
>>> +Robin, Joerg, IOMMU ML
>>>
>>> Please see my comments inline.
>>>
>>> On Tue, Jun 6, 2017 at 5:39 AM, Yong Zhi  wrote:
> [snip]
 +
 +/* End of things adapted from arch/arm/mm/dma-mapping.c */
 +static void ipu3_dmamap_sync_single_for_cpu(struct device *dev,
 +   dma_addr_t dma_handle, size_t 
 size,
 +   enum dma_data_direction dir)
 +{
 +   struct ipu3_mmu *mmu = to_ipu3_mmu(dev);
 +   dma_addr_t daddr = iommu_iova_to_phys(mmu->domain, dma_handle);
 +
 +   clflush_cache_range(phys_to_virt(daddr), size);
>>>
>>> You might need to consider another IOMMU on the way here. Generally,
>>> given that daddr is your MMU DMA address (not necessarily CPU physical
>>> address), you should be able to call
>>>
>>> dma_sync_single_for_cpu(, daddr, size, dir)
>>
>> I'd hope that this IPU complex is some kind of embedded endpoint thing
>> that bypasses the VT-d IOMMU or is always using its own local RAM,
>> because it would be pretty much unworkable otherwise.
> 
> It uses system RAM and, as far as my understanding goes, by default it
> operates without the VT-d IOMMU and that's how it's used right now.

OK, if it *is* behind a DMAR unit then booting with "iommu=force" (or
whatever the exact incantation for intel-iommu is) should be fun...

> I'm suggesting VT-d IOMMU as a way to further strengthen the security
> and error resilience in future (due to the IPU complex being
> non-coherent and also running a closed source firmware).

TBH, doing DMA remapping through *two* IOMMUS will add horrible hardware
overhead, increase the scope for kernel-side bugs, and not much more. If
we don't trust this IOMMU to behave, why are we trying to drive it in
the first place? If we do, then a second IOMMU behind it won't protect
anything that the first one doesn't already.

>> The whole
>> infrastructure isn't really capable of dealing with nested IOMMUs, and
>> nested DMA ops would be an equally horrible idea.
> 
> Could you elaborate a bit more on this? I think we should be able to
> deal with this in a way I suggested before:
> 
> a) the PCI device would use the system DMA ops,
> b) the PCI device would implement a secondary bus for which it would
> provide its own DMA and IOMMU ops.
> c) a secondary device would be registered on the secondary bus,
> d) all memory for the IPU would be managed on behalf of the secondary device.
> 
> In fact, the driver already is designed in a way that all the points
> above are true. If I'm not missing something, the only significant
> missing point is calling into system DMA ops from IPU DMA ops.

I don't believe x86 has any non-coherent DMA ops, therefore the IPU DMA
ops would still probably have to do all their own cache maintenance.
Allocation/mapping, though, would have to be done with the parent DMA
ops first (in case DMA address != physical address), *then* mapped at
the IPU MMU, which is the real killer - if the PCI DMA ops are from
intel-iommu, then there's little need for the IPU MMU mapping to be
anything other than 1:1, so you may as well not bother. If the PCI DMA
ops are from SWIOTLB, then the constraints of having to go through that
first eliminate all the scatter-gather benefit of the IPU MMU.

The IOMMU API ops would have to be handled similarly, by checking for
ops on the parent bus, calling those first if present, then running the
intermediate results through the IPU MMU's own functions. Sure, it's not
impossible, but it's really really grim. Not to mention that all the IPU
MMU's page tables/control structures/etc. would also have to be
DMA-allocated/mapped because it may or may not be operating in physical
address space.

The reasonable option - assuming the topology really is this way - would
seem to be special-casing the IPU in intel-iommu in a similar manner to
integrated graphics, to make sure it gets a passthrough domain for DMA
ops, but still allowing the whole PCI device to be passed through to a
guest VM via VFIO if desired (which is really the only case where nested
translation does start to make sense).

Robin.

> 
> Best regards,
> Tomasz
>

Re: [PATCH v4] v4l: subdev: tolerate null in media_entity_to_v4l2_subdev

Em Wed,  7 Jun 2017 10:52:07 +0100
Kieran Bingham  escreveu:

> From: Kieran Bingham 
> 
> Return NULL, if a null entity is parsed for it's v4l2_subdev
> 
> Signed-off-by: Kieran Bingham 

Could you please improve this patch description?

I'm unsure if this is a bug fix, or some sort of feature...

On what situations would a null entity be passed to this function?

Regards,
Mauro

> 
> ---
> Not sure if this patch ever made it out of my mailbox:
> 
> Here's the respin with the parameter evaluated only once.
> 
> v4:
>  - Improve macro usage to evaluate ent only once
> 
>  include/media/v4l2-subdev.h | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index a40760174797..0f92ebd2d710 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -826,8 +826,15 @@ struct v4l2_subdev {
>   struct v4l2_subdev_platform_data *pdata;
>  };
>  
> -#define media_entity_to_v4l2_subdev(ent) \
> - container_of(ent, struct v4l2_subdev, entity)
> +#define media_entity_to_v4l2_subdev(ent) \
> +({   \
> + typeof(ent) __me_sd_ent = (ent);\
> + \
> + __me_sd_ent ?   \
> + container_of(__me_sd_ent, struct v4l2_subdev, entity) : \
> + NULL;   \
> +})
> +
>  #define vdev_to_v4l2_subdev(vdev) \
>   ((struct v4l2_subdev *)video_get_drvdata(vdev))
>  



Thanks,
Mauro

[linuxtv-media:master 1719/1759] arch/arm/mach-davinci/pdata-quirks.c:87:13: warning: initialization discards 'const' qualifier from pointer target type

2017-06-08 Thread kbuild test robot

tree:   git://linuxtv.org/media_tree.git master
head:   703ecba4dbfb8de9cad7420021dfcbd11007de3b
commit: 4a5f8ae50b66a40cd9cec8f72f5f64611504ddc1 [1719/1759] [media] davinci: 
vpif_capture: get subdevs from DT when available
config: arm-davinci_all_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 4a5f8ae50b66a40cd9cec8f72f5f64611504ddc1
# save the attached .config to linux build tree
make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

>> arch/arm/mach-davinci/pdata-quirks.c:87:13: warning: initialization discards 
>> 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  .inputs = da850_ch0_inputs,
^~~~
   arch/arm/mach-davinci/pdata-quirks.c:97:13: warning: initialization discards 
'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  .inputs = da850_ch1_inputs,
^~~~
--
>> arch/arm/mach-davinci/board-dm646x-evm.c:680:13: warning: initialization 
>> discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  .inputs = dm6467_ch0_inputs,
^
   arch/arm/mach-davinci/board-dm646x-evm.c:690:13: warning: initialization 
discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  .inputs = dm6467_ch1_inputs,
^

vim +/const +87 arch/arm/mach-davinci/pdata-quirks.c

7ee77194 Kevin Hilman 2017-02-28  71.platform_data = 
_pdata,
7ee77194 Kevin Hilman 2017-02-28  72},
7ee77194 Kevin Hilman 2017-02-28  73},
7ee77194 Kevin Hilman 2017-02-28  74{
7ee77194 Kevin Hilman 2017-02-28  75.name = TVP5147_CH1,
7ee77194 Kevin Hilman 2017-02-28  76.board_info = {
7ee77194 Kevin Hilman 2017-02-28  77
I2C_BOARD_INFO("tvp5146", 0x5c),
7ee77194 Kevin Hilman 2017-02-28  78.platform_data = 
_pdata,
7ee77194 Kevin Hilman 2017-02-28  79},
7ee77194 Kevin Hilman 2017-02-28  80},
7ee77194 Kevin Hilman 2017-02-28  81  };
7ee77194 Kevin Hilman 2017-02-28  82  
7ee77194 Kevin Hilman 2017-02-28  83  static struct vpif_capture_config 
da850_vpif_capture_config = {
7ee77194 Kevin Hilman 2017-02-28  84.subdev_info = 
da850_vpif_capture_sdev_info,
7ee77194 Kevin Hilman 2017-02-28  85.subdev_count = 
ARRAY_SIZE(da850_vpif_capture_sdev_info),
7ee77194 Kevin Hilman 2017-02-28  86.chan_config[0] = {
7ee77194 Kevin Hilman 2017-02-28 @87.inputs = da850_ch0_inputs,
7ee77194 Kevin Hilman 2017-02-28  88.input_count = 
ARRAY_SIZE(da850_ch0_inputs),
7ee77194 Kevin Hilman 2017-02-28  89.vpif_if = {
7ee77194 Kevin Hilman 2017-02-28  90.if_type = 
VPIF_IF_BT656,
7ee77194 Kevin Hilman 2017-02-28  91.hd_pol  = 1,
7ee77194 Kevin Hilman 2017-02-28  92.vd_pol  = 1,
7ee77194 Kevin Hilman 2017-02-28  93.fid_pol = 0,
7ee77194 Kevin Hilman 2017-02-28  94},
7ee77194 Kevin Hilman 2017-02-28  95},

:: The code at line 87 was first introduced by commit
:: 7ee77194143ba7cee8d55956adc85914ce49a277 ARM: davinci: da8xx: add 
pdata-quirks for VPIF capture

:: TO: Kevin Hilman 
:: CC: Sekhar Nori 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] [media] media-ioc-g-topology.rst: fix typos

Em Wed,  7 Jun 2017 18:33:02 +0900
Alexandre Courbot  escreveu:

> Fix what seems to be a few typos induced by copy/paste.
> 
> Signed-off-by: Alexandre Courbot 
> ---
>  Documentation/media/uapi/mediactl/media-ioc-g-topology.rst | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst 
> b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> index 48c9531f4db0..5f2d82756033 100644
> --- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> +++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
> @@ -241,7 +241,7 @@ desired arrays with the media graph elements.
>  
>  .. c:type:: media_v2_intf_devnode
>  
> -.. flat-table:: struct media_v2_interface
> +.. flat-table:: struct media_v2_devnode
>  :header-rows:  0
>  :stub-columns: 0
>  :widths: 1 2 8

Actually the fix is wrong here :-)

I'll just fold the following diff to your patch:

diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst 
b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index 5f2d82756033..add8281494f8 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -241,7 +241,7 @@ desired arrays with the media graph elements.
 
 .. c:type:: media_v2_intf_devnode
 
-.. flat-table:: struct media_v2_devnode
+.. flat-table:: struct media_v2_intf_devnode
 :header-rows:  0
 :stub-columns: 0
 :widths: 1 2 8

Thanks,
Mauro

adv7842.c:undefined reference to `cec_unregister_adapter'

2017-06-08 Thread kbuild test robot

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   b29794ec95c6856b316c2295904208bf11ffddd9
commit: 9177e51d1434076a91f9bfb693deae8b955d6d57 [media] cec: select CEC_CORE 
instead of depend on it
date:   4 days ago
config: i386-randconfig-b0-06090003 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
git checkout 9177e51d1434076a91f9bfb693deae8b955d6d57
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `adv7842_unregistered':
>> adv7842.c:(.text+0x2edadd): undefined reference to `cec_unregister_adapter'
   drivers/built-in.o: In function `adv7842_registered':
>> adv7842.c:(.text+0x2edb2a): undefined reference to `cec_register_adapter'
>> adv7842.c:(.text+0x2edb57): undefined reference to `cec_delete_adapter'
   drivers/built-in.o: In function `edid_write_hdmi_segment':
>> adv7842.c:(.text+0x2ee49f): undefined reference to `cec_get_edid_phys_addr'
>> adv7842.c:(.text+0x2ee4c0): undefined reference to `cec_phys_addr_validate'
>> adv7842.c:(.text+0x2ee736): undefined reference to `cec_s_phys_addr'
   drivers/built-in.o: In function `vivid_remove':
>> vivid-core.c:(.text+0x3fc9fb): undefined reference to 
>> `cec_unregister_adapter'
   vivid-core.c:(.text+0x3fca19): undefined reference to 
`cec_unregister_adapter'
   drivers/built-in.o: In function `vivid_create_instance':
   vivid-core.c:(.text+0x3ffecf): undefined reference to 
`cec_unregister_adapter'
   vivid-core.c:(.text+0x3ffef7): undefined reference to 
`cec_unregister_adapter'
   drivers/built-in.o: In function `vidioc_g_edid':
>> (.text+0x405a5b): undefined reference to `cec_set_edid_phys_addr'
   drivers/built-in.o: In function `vidioc_s_edid':
>> (.text+0x40e71a): undefined reference to `cec_get_edid_phys_addr'
   drivers/built-in.o: In function `vidioc_s_edid':
>> (.text+0x40e73d): undefined reference to `cec_phys_addr_validate'
   drivers/built-in.o: In function `vidioc_s_edid':
>> (.text+0x40e7ad): undefined reference to `cec_s_phys_addr'
   drivers/built-in.o: In function `vidioc_s_edid':
>> (.text+0x40e7f8): undefined reference to `cec_phys_addr_for_input'
   drivers/built-in.o: In function `vidioc_s_edid':
   (.text+0x40e819): undefined reference to `cec_s_phys_addr'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v8 02/34] [media] dt-bindings: Add bindings for i.MX media driver

2017-06-08 Thread Steve Longerbeam




On 06/08/2017 09:45 AM, Steve Longerbeam wrote:

Hi Rob, Mark,

Are there any remaining technical issues with this
binding doc? At this point an Ack from you is the only
thing holding up merge of the imx-media driver.




Note that the Synopsys core in the i.MX6 is a differently configured
Synopsys core from the core as described in the bindings at [1].

Russell King provided more information on the differences between these
cores at [2]. They are essentially different devices.

So perhaps the "snps,dw-mipi-csi2" compatibility needs to be removed
from this binding, for now, until this driver is moved to drivers/media/
and is made compatible with other MIPI CSI-2 Synopsys cores with
different configurations.


[1] http://patchwork.ozlabs.org/patch/736177/
[2] <20170403150342.gz7...@n2100.armlinux.org.uk>



On 06/07/2017 11:33 AM, Steve Longerbeam wrote:

Add bindings documentation for the i.MX media driver.

Signed-off-by: Steve Longerbeam 
---
  Documentation/devicetree/bindings/media/imx.txt | 47 
+

  1 file changed, 47 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/imx.txt

diff --git a/Documentation/devicetree/bindings/media/imx.txt 
b/Documentation/devicetree/bindings/media/imx.txt

new file mode 100644
index 000..c1e1e2b
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx.txt
@@ -0,0 +1,47 @@
+Freescale i.MX Media Video Device
+=
+
+Video Media Controller node
+---
+
+This is the media controller node for video capture support. It is a
+virtual device that lists the camera serial interface nodes that the
+media device will control.
+
+Required properties:
+- compatible : "fsl,imx-capture-subsystem";
+- ports  : Should contain a list of phandles pointing to camera
+sensor interface ports of IPU devices
+
+example:
+
+capture-subsystem {
+compatible = "fsl,imx-capture-subsystem";
+ports = <_csi0>, <_csi1>;
+};
+
+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 Receiver, required for MIPI
+CSI-2 sensors.
+
+Required properties:
+- compatible: "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";
+- reg   : physical base address and length of the register set;
+- clocks: the MIPI CSI-2 receiver requires three clocks: hsi_tx
+  (the D-PHY clock), video_27m (D-PHY PLL reference
+  clock), and eim_podf;
+- clock-names: must contain "dphy", "ref", "pix";
+- port@*: five port nodes must exist, containing endpoints
+  connecting to the source and sink devices according to
+  of_graph bindings. The first port is an input port,
+  connecting with a MIPI CSI-2 source, and ports 1
+  through 4 are output ports connecting with parallel
+  bus sink endpoint nodes and correspond to the four
+  MIPI CSI-2 virtual channel outputs.
+
+Optional properties:
+- interrupts: must contain two level-triggered interrupts,
+  in order: 100 and 101;

[PATCH v2] [media] v4l2-subdev: check colorimetry in link validate

2017-06-08 Thread Helen Koike

colorspace, ycbcr_enc, quantization and xfer_func must match
across the link.
Check if they match in v4l2_subdev_link_validate_default
unless they are set as _DEFAULT.

Signed-off-by: Helen Koike 

---

Hi,

As discussed previously, I added a warn message instead of returning
error to give drivers some time to adapt.
But the problem is that: as the format is set by userspace, it is
possible that userspace will set the wrong format at pads and see these
messages when there is no error in the driver's code at all (or maybe
this is not a problem, just noise in the log).

Helen
---
 drivers/media/v4l2-core/v4l2-subdev.c | 38 +++
 1 file changed, 38 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
b/drivers/media/v4l2-core/v4l2-subdev.c
index da78497..1a642c7 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -508,6 +508,44 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev 
*sd,
|| source_fmt->format.code != sink_fmt->format.code)
return -EPIPE;
 
+   /*
+* TODO: return -EPIPE instead of printing a warning in the following
+* checks. As colorimetry properties were added after most of the
+* drivers, only a warning was added to avoid potential regressions
+*/
+
+   /* colorspace match. */
+   if (source_fmt->format.colorspace != sink_fmt->format.colorspace)
+   dev_warn(sd->v4l2_dev->dev,
+"colorspace doesn't match in link 
\"%s\":%d->\"%s\":%d\n",
+   link->source->entity->name, link->source->index,
+   link->sink->entity->name, link->sink->index);
+
+   /* Colorimetry must match if they are not set to DEFAULT */
+   if (source_fmt->format.ycbcr_enc != V4L2_YCBCR_ENC_DEFAULT
+   && sink_fmt->format.ycbcr_enc != V4L2_YCBCR_ENC_DEFAULT
+   && source_fmt->format.ycbcr_enc != sink_fmt->format.ycbcr_enc)
+   dev_warn(sd->v4l2_dev->dev,
+"YCbCr encoding doesn't match in link 
\"%s\":%d->\"%s\":%d\n",
+   link->source->entity->name, link->source->index,
+   link->sink->entity->name, link->sink->index);
+
+   if (source_fmt->format.quantization != V4L2_QUANTIZATION_DEFAULT
+   && sink_fmt->format.quantization != V4L2_QUANTIZATION_DEFAULT
+   && source_fmt->format.quantization != sink_fmt->format.quantization)
+   dev_warn(sd->v4l2_dev->dev,
+"quantization doesn't match in link 
\"%s\":%d->\"%s\":%d\n",
+   link->source->entity->name, link->source->index,
+   link->sink->entity->name, link->sink->index);
+
+   if (source_fmt->format.xfer_func != V4L2_XFER_FUNC_DEFAULT
+   && sink_fmt->format.xfer_func != V4L2_XFER_FUNC_DEFAULT
+   && source_fmt->format.xfer_func != sink_fmt->format.xfer_func)
+   dev_warn(sd->v4l2_dev->dev,
+"transfer function doesn't match in link 
\"%s\":%d->\"%s\":%d\n",
+   link->source->entity->name, link->source->index,
+   link->sink->entity->name, link->sink->index);
+
/* The field order must match, or the sink field order must be NONE
 * to support interlaced hardware connected to bridges that support
 * progressive formats only.
-- 
2.7.4

[PATCH] atomisp: use correct dialect to disable warnings

There's a Macro that checks if gcc supports a warning before
disabling it. Use it, in order to avoid warnings when building
with older gcc versions.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/media/atomisp/i2c/Makefile| 6 --
 drivers/staging/media/atomisp/i2c/imx/Makefile| 6 --
 drivers/staging/media/atomisp/i2c/ov5693/Makefile | 6 --
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/Makefile 
b/drivers/staging/media/atomisp/i2c/Makefile
index a1afca6ec31f..be13fab92175 100644
--- a/drivers/staging/media/atomisp/i2c/Makefile
+++ b/drivers/staging/media/atomisp/i2c/Makefile
@@ -21,5 +21,7 @@ obj-$(CONFIG_VIDEO_LM3554) += lm3554.o
 
 # HACK! While this driver is in bad shape, don't enable several warnings
 #   that would be otherwise enabled with W=1
-ccflags-y += -Wno-unused-but-set-variable -Wno-missing-prototypes \
--Wno-unused-const-variable -Wno-missing-declarations
+ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
+ccflags-y += $(call cc-disable-warning, unused-const-variable)
+ccflags-y += $(call cc-disable-warning, missing-prototypes)
+ccflags-y += $(call cc-disable-warning, missing-declarations)
diff --git a/drivers/staging/media/atomisp/i2c/imx/Makefile 
b/drivers/staging/media/atomisp/i2c/imx/Makefile
index 0eceb7374bec..b6578f09546e 100644
--- a/drivers/staging/media/atomisp/i2c/imx/Makefile
+++ b/drivers/staging/media/atomisp/i2c/imx/Makefile
@@ -7,5 +7,7 @@ obj-$(CONFIG_VIDEO_OV8858) += ov8858_driver.o
 
 # HACK! While this driver is in bad shape, don't enable several warnings
 #   that would be otherwise enabled with W=1
-ccflags-y += -Wno-unused-but-set-variable -Wno-missing-prototypes \
- -Wno-unused-const-variable -Wno-missing-declarations
+ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
+ccflags-y += $(call cc-disable-warning, unused-const-variable)
+ccflags-y += $(call cc-disable-warning, missing-prototypes)
+ccflags-y += $(call cc-disable-warning, missing-declarations)
diff --git a/drivers/staging/media/atomisp/i2c/ov5693/Makefile 
b/drivers/staging/media/atomisp/i2c/ov5693/Makefile
index fd2ef2e3c31e..4e3833aaec05 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/Makefile
+++ b/drivers/staging/media/atomisp/i2c/ov5693/Makefile
@@ -2,5 +2,7 @@ obj-$(CONFIG_VIDEO_OV5693) += ov5693.o
 
 # HACK! While this driver is in bad shape, don't enable several warnings
 #   that would be otherwise enabled with W=1
-ccflags-y += -Wno-unused-but-set-variable -Wno-missing-prototypes \
- -Wno-unused-const-variable -Wno-missing-declarations
+ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
+ccflags-y += $(call cc-disable-warning, unused-const-variable)
+ccflags-y += $(call cc-disable-warning, missing-prototypes)
+ccflags-y += $(call cc-disable-warning, missing-declarations)
-- 
2.9.4

Re: [PATCH 03/12] intel-ipu3: Add DMA API implementation

Hi Tomasz and Alan,

On Thu, Jun 08, 2017 at 11:55:18AM +0900, Tomasz Figa wrote:
> Hi Alan,
> 
> On Thu, Jun 8, 2017 at 2:45 AM, Alan Cox  wrote:
> >> > +   struct ipu3_mmu *mmu = to_ipu3_mmu(dev);
> >> > +   dma_addr_t daddr = iommu_iova_to_phys(mmu->domain, dma_handle);
> >> > +
> >> > +   clflush_cache_range(phys_to_virt(daddr), size);
> >>
> >> You might need to consider another IOMMU on the way here. Generally,
> >> given that daddr is your MMU DMA address (not necessarily CPU physical
> >> address), you should be able to call
> >>
> >> dma_sync_single_for_cpu(, daddr, size, dir)
> >
> > Te system IOMMU (if enabled) may be cache coherent - and on x86 would be,
> > so it doesn't think it needs to do anything for cache synchronization
> > and the dma_sync won't actually do any work.
> 
> I'm not very familiar with x86, but typically I found coherency to be
> an attribute of the DMA master (i.e. if it is connected to a coherent
> memory port).
> 
> Looking at all the IPU3 code, it looks like the whole PCI device is
> non-coherent for some reason (e.g. you can see implicit cache flushes
> for page tables). So I would have expected that a non-coherent variant
> of x86 dma_ops is used for the PCI struct device, which would do cache
> maintenance in its dma_sync_* ops.

It can actually do both --- in most cases.

The MMU page tables are an exception so they will still need an explicit
flush.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v8 02/34] [media] dt-bindings: Add bindings for i.MX media driver

2017-06-08 Thread Steve Longerbeam


Hi Rob, Mark,

Are there any remaining technical issues with this
binding doc? At this point an Ack from you is the only
thing holding up merge of the imx-media driver.

Thanks,
Steve


On 06/07/2017 11:33 AM, Steve Longerbeam wrote:

Add bindings documentation for the i.MX media driver.

Signed-off-by: Steve Longerbeam 
---
  Documentation/devicetree/bindings/media/imx.txt | 47 +
  1 file changed, 47 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/imx.txt

diff --git a/Documentation/devicetree/bindings/media/imx.txt 
b/Documentation/devicetree/bindings/media/imx.txt
new file mode 100644
index 000..c1e1e2b
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx.txt
@@ -0,0 +1,47 @@
+Freescale i.MX Media Video Device
+=
+
+Video Media Controller node
+---
+
+This is the media controller node for video capture support. It is a
+virtual device that lists the camera serial interface nodes that the
+media device will control.
+
+Required properties:
+- compatible : "fsl,imx-capture-subsystem";
+- ports  : Should contain a list of phandles pointing to camera
+   sensor interface ports of IPU devices
+
+example:
+
+capture-subsystem {
+   compatible = "fsl,imx-capture-subsystem";
+   ports = <_csi0>, <_csi1>;
+};
+
+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 Receiver, required for MIPI
+CSI-2 sensors.
+
+Required properties:
+- compatible   : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";
+- reg   : physical base address and length of the register set;
+- clocks   : the MIPI CSI-2 receiver requires three clocks: hsi_tx
+ (the D-PHY clock), video_27m (D-PHY PLL reference
+ clock), and eim_podf;
+- clock-names  : must contain "dphy", "ref", "pix";
+- port@*: five port nodes must exist, containing endpoints
+ connecting to the source and sink devices according to
+ of_graph bindings. The first port is an input port,
+ connecting with a MIPI CSI-2 source, and ports 1
+ through 4 are output ports connecting with parallel
+ bus sink endpoint nodes and correspond to the four
+ MIPI CSI-2 virtual channel outputs.
+
+Optional properties:
+- interrupts   : must contain two level-triggered interrupts,
+ in order: 100 and 101;

Re: [PATCH 02/12] intel-ipu3: mmu: implement driver

Hi Tomasz,

On Wed, Jun 07, 2017 at 05:35:13PM +0900, Tomasz Figa wrote:
> Hi Yong, Tuukka,
> 
> Continuing from yesterday. Please see comments inline.
> 
> > On Tue, Jun 6, 2017 at 5:39 AM, Yong Zhi  wrote:
> [snip]
> >> +   ptr = ipu3_mmu_alloc_page_table(mmu_dom, false);
> >> +   if (!ptr)
> >> +   goto fail_page_table;
> >> +
> >> +   /*
> >> +* We always map the L1 page table (a single page as well as
> >> +* the L2 page tables).
> >> +*/
> >> +   mmu_dom->dummy_l2_tbl = virt_to_phys(ptr) >> IPU3_MMU_PAGE_SHIFT;
> >> +   mmu_dom->pgtbl = ipu3_mmu_alloc_page_table(mmu_dom, true);
> >> +   if (!mmu_dom->pgtbl)
> >> +   goto fail_page_table;
> >> +
> >> +   spin_lock_init(_dom->lock);
> >> +   return _dom->domain;
> >> +
> >> +fail_page_table:
> >> +   free_page((unsigned long)TBL_VIRT_ADDR(mmu_dom->dummy_page));
> >> +   free_page((unsigned long)TBL_VIRT_ADDR(mmu_dom->dummy_l2_tbl));
> >> +fail_get_page:
> >> +   kfree(mmu_dom);
> >> +   return NULL;
> >> +}
> >> +
> >> +static void ipu3_mmu_domain_free(struct iommu_domain *dom)
> >> +{
> >> +   struct ipu3_mmu_domain *mmu_dom =
> >> +   container_of(dom, struct ipu3_mmu_domain, domain);
> >> +   uint32_t l1_idx;
> >> +
> >> +   for (l1_idx = 0; l1_idx < IPU3_MMU_L1PT_PTES; l1_idx++)
> >> +   if (mmu_dom->pgtbl[l1_idx] != mmu_dom->dummy_l2_tbl)
> >> +   free_page((unsigned long)
> >> + TBL_VIRT_ADDR(mmu_dom->pgtbl[l1_idx]));
> >> +
> >> +   free_page((unsigned long)TBL_VIRT_ADDR(mmu_dom->dummy_page));
> >> +   free_page((unsigned long)TBL_VIRT_ADDR(mmu_dom->dummy_l2_tbl));
> 
> I might be overly paranoid, but reading back kernel virtual pointers
> from device accessible memory doesn't seem safe to me. Other drivers
> keep kernel pointers of page tables in a dedicated array (it's only 8K
> of memory, but much better safety).

Do you happen to have an example of that?

All system memory typically is accessible for devices, I think you wanted to
say that the device is intended to access that memory. Albeit for reading
only.

...

> >> +static int ipu3_mmu_bus_remove(struct device *dev)
> >> +{
> >> +   struct ipu3_mmu *mmu = dev_get_drvdata(dev);
> >> +
> >> +   put_iova_domain(>iova_domain);
> >> +   iova_cache_put();
> 
> Don't we need to set the L1 table address to something invalid and
> invalidate the TLB, so that the IOMMU doesn't reference the page freed
> below anymore?

I think the expectation is that if a device gets removed, its memory is
unmapped by that time. Unmapping that memory will cause explicit TLB flush.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH] Revert "[media] et8ek8: Export OF device ID as module aliases"

2017-06-08 Thread Pavel Machek

On Thu 2017-06-08 11:01:37, Arnd Bergmann wrote:
> This one got applied twice, causing a build error with clang:
> 
> drivers/media/i2c/et8ek8/et8ek8_driver.c:1499:1: error: redefinition of 
> '__mod_of__et8ek8_of_table_device_table'
> 
> Fixes: 9ae05fd1e791 ("[media] et8ek8: Export OF device ID as module aliases")
> Signed-off-by: Arnd Bergmann 

Acked-by: Pavel Machek 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [RFC PATCH] [media] v4l2-subdev: check colorimetry in link validate

2017-06-08 Thread Helen Koike

On 2017-06-08 08:41 AM, Mauro Carvalho Chehab wrote:

Em Tue, 6 Jun 2017 19:15:34 -0300
Helen Koike escreveu:

Hi Sakari,

Thanks for replying

On 2017-05-31 03:31 AM, Sakari Ailus wrote:

Hi Helen,

On Tue, May 30, 2017 at 04:08:08PM -0300, Helen Koike wrote:

colorspace, ycbcr_enc, quantization and xfer_func must match across the
link.
Check if they match in v4l2_subdev_link_validate_default unless they are
set as _DEFAULT.

Signed-off-by: Helen Koike

---

Hi,

I think we should validate colorimetry as having different colorimetry
across a link doesn't make sense.
But I am confused about what to do when they are set to _DEFAULT, what
do you think?

These fields were added at various points over the course of the past three
years or so. User space code that was written before that will certainly not
set anything and I'm not sure many drivers care about these fields nor they
are relevant for many formats. In practice that means that they are very
likely zero in many cases.

If they are set to zero then they won't be affected by this patch.

Driver changes will probably be necessary for removing the explicit checks
for the default value.

At least in the drivers/media tree I couldn't find many drivers that
implement its own link_validate, there is only
platform/omap3isp/ispccdc.c and platform/omap3isp/ispresizer.c that
implements a custom value, but from a quick look it doesn't seems that
they will be affected.

The same applies to checking the colour space: drivers should enforce using
the correct colour space before the check can be merged. I might move that
to a separate patch.

I am not sure if I got what you mean. If driver don't care about
colourspace then probably it will be set to zero and won't be affected
by this patch, if colourspace is different across the link then the user
space must set both ends to the same colourspace.

I guess what Sakari is concerned about is to avoid regressions.

Colorimetry properties were added after the addition of most of
the drivers. Adding a mandatory link validation may break drivers
that don't set it right. Even on new drivers, this may not be ok.

Just to give you an example, this week I just applied a patch fixing
colorimetry handling at the coda driver:

https://git.linuxtv.org/media_tree.git/commit/?id=b14ac545688d8cc4b2b707d71d106799ad476964

If a change like that would have been applied before such fix, it
could be breaking coda.

If I understand correct, coda doesn't have subdevs or links so it
wouldn't be affected by this patch.

So, before adding such patch, we need to check how existing drivers are
setting colorimetry fields, to be sure that the ones that don't
touch it won't break.

In my view, if they don't touch colorimetry they shouldn't be affected
by this patch as values would be zero, and there are very few drivers
that implement a custom link validation under driver/media tree, and
from a quick look it doesn't seems to change colorimetry values.

Perhaps one alternative would be to write a patchset that would,
instead, print a warning at dmesg, and let it be upstream for a
while, to give people time to check if the colorimetry logic is
ok at the subdevs.

Agreed, lets start with that, I'll re-send this patch which this change.

Thanks
Helen

[linuxtv-media:master 1719/1759] drivers/media//platform/davinci/vpif_capture.c:25:27: fatal error: media/v4l2-of.h: No such file or directory

2017-06-08 Thread kbuild test robot

tree:   git://linuxtv.org/media_tree.git master
head:   703ecba4dbfb8de9cad7420021dfcbd11007de3b
commit: 4a5f8ae50b66a40cd9cec8f72f5f64611504ddc1 [1719/1759] [media] davinci: 
vpif_capture: get subdevs from DT when available
config: arm-davinci_all_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 4a5f8ae50b66a40cd9cec8f72f5f64611504ddc1
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> drivers/media//platform/davinci/vpif_capture.c:25:27: fatal error: 
>> media/v4l2-of.h: No such file or directory
#include 
  ^
   compilation terminated.

vim +25 drivers/media//platform/davinci/vpif_capture.c

 9   *
10   * This program is distributed in the hope that it will be useful,
11   * but WITHOUT ANY WARRANTY; without even the implied warranty of
12   * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
13   * GNU General Public License for more details.
14   *
15   * TODO : add support for VBI & HBI data service
16   *add static buffer allocation
17   */
18  
19  #include 
20  #include 
21  #include 
22  #include 
23  
24  #include 
  > 25  #include 
26  #include 
27  
28  #include "vpif.h"
29  #include "vpif_capture.h"
30  
31  MODULE_DESCRIPTION("TI DaVinci VPIF Capture driver");
32  MODULE_LICENSE("GPL");
33  MODULE_VERSION(VPIF_CAPTURE_VERSION);

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH RFC] v4l2-core: Use kvmalloc() for potentially big allocations

Em Wed, 31 May 2017 16:07:29 +0300
Sakari Ailus  escreveu:

> Hi Tomasz,
> 
> On Wed, May 31, 2017 at 09:46:05PM +0900, Tomasz Figa wrote:
> > On Wed, May 31, 2017 at 9:09 PM, Marek Szyprowski
> >  wrote:  
> > > Hi Tomasz,
> > >
> > >
> > > On 2017-05-31 08:58, Tomasz Figa wrote:  
> > >>
> > >> There are multiple places where arrays or otherwise variable sized
> > >> buffer are allocated through V4L2 core code, including things like
> > >> controls, memory pages, staging buffers for ioctls and so on. Such
> > >> allocations can potentially require an order > 0 allocation from the
> > >> page allocator, which is not guaranteed to be fulfilled and is likely to
> > >> fail on a system with severe memory fragmentation (e.g. a system with
> > >> very long uptime).
> > >>
> > >> Since the memory being allocated is intended to be used by the CPU
> > >> exclusively, we can consider using vmalloc() as a fallback and this is
> > >> exactly what the recently merged kvmalloc() helpers do. A kmalloc() call
> > >> is still attempted, even for order > 0 allocations, but it is done
> > >> with __GFP_NORETRY and __GFP_NOWARN, with expectation of failing if
> > >> requested memory is not available instantly. Only then the vmalloc()
> > >> fallback is used. This should give us fast and more reliable allocations
> > >> even on systems with higher memory pressure and/or more fragmentation,
> > >> while still retaining the same performance level on systems not
> > >> suffering from such conditions.
> > >>
> > >> While at it, replace explicit array size calculations on changed
> > >> allocations with kvmalloc_array().
> > >>
> > >> Signed-off-by: Tomasz Figa 
> > >> ---
> > >>   drivers/media/v4l2-core/v4l2-async.c   |  4 ++--
> > >>   drivers/media/v4l2-core/v4l2-ctrls.c   | 25
> > >> +
> > >>   drivers/media/v4l2-core/v4l2-event.c   |  8 +---
> > >>   drivers/media/v4l2-core/v4l2-ioctl.c   |  6 +++---
> > >>   drivers/media/v4l2-core/v4l2-subdev.c  |  7 ---
> > >>   drivers/media/v4l2-core/videobuf2-dma-sg.c |  8   
> > >
> > >
> > > For vb2:
> > > Acked-by: Marek Szyprowski   
> > 
> > Thanks!
> >   
> > >
> > > There are also a few vmalloc calls in old videobuf (v1) framework, which
> > > might be converted to kvmalloc if you have a few spare minutes to take
> > > a look.  
> > 
> > I was intending to convert those as well, but on the other hand I
> > concluded that it's some very old code, which might be difficult to
> > test and likely to introduce some long undiscovered regressions. If
> > it's desired to update those as well, I can include those changes in
> > the non-RFC version.  
> 
> I think it's better to leave videobuf1 as-is. I'd rather like to see it
> removed instead.

Agreed.

There aren't much VB drivers anymore:

$ git grep -l VIDEOBUF_ |grep Kconfig
drivers/media/common/saa7146/Kconfig
drivers/media/pci/bt8xx/Kconfig
drivers/media/pci/cx18/Kconfig
drivers/media/platform/Kconfig
drivers/media/platform/davinci/Kconfig
drivers/media/platform/omap/Kconfig
drivers/media/usb/cx231xx/Kconfig
drivers/media/usb/tm6000/Kconfig
drivers/media/usb/zr364xx/Kconfig
drivers/media/v4l2-core/Kconfig
drivers/staging/media/atomisp/pci/Kconfig

(at platform/Kconfig, there are two drivers: via-camera and viu)

Not sure how easy/hard would be to convert those remaining ones.


Thanks,
Mauro

Re: Unknown symbol put_vaddr_frames when using media_build

Em Thu, 8 Jun 2017 21:23:19 +1000
Vincent McIntyre  escreveu:

> On Wed, Jun 07, 2017 at 08:12:01PM -0300, Mauro Carvalho Chehab wrote:
> > Em Wed, 7 Jun 2017 22:17:50 +0200
> > Matthias Schwarzott  escreveu:
> >   
> > > Am 07.06.2017 um 20:23 schrieb Mauro Carvalho Chehab:  
> > > > Em Tue, 9 May 2017 06:56:25 +0200
> > > > Matthias Schwarzott  escreveu:
> > > > 
> > > >> Hi!
> > > >>
> > > >> Whenever I compile the media drivers using media_build against a recent
> > > >> kernel, I get this message when loading them:
> > > >>
> > > >> [5.848537] media: Linux media interface: v0.10
> > > >> [5.881440] Linux video capture interface: v2.00
> > > >> [5.881441] WARNING: You are using an experimental version of the
> > > >> media stack.
> > > >> ...
> > > >> [6.166390] videobuf2_memops: Unknown symbol put_vaddr_frames (err 
> > > >> 0)
> > > >> [6.166394] videobuf2_memops: Unknown symbol get_vaddr_frames (err 
> > > >> 0)
> > > >> [6.166396] videobuf2_memops: Unknown symbol frame_vector_destroy 
> > > >> (err 0)
> > > >> [6.166398] videobuf2_memops: Unknown symbol frame_vector_create 
> > > >> (err 0)
> > > >>
> > > >> That means I am not able to load any drivers being based on
> > > >> videobuf2_memops without manual actions.
> > > >>
> > > >> I used kernel 4.11.0, but it does not matter which kernel version
> > > >> exactly is used.
> > > >>
> > > >> My solution for that has been to modify mm/Kconfig of my kernel like
> > > >> this and then enable FRAME_VECTOR in .config
> > > > 
> > > > Well, if you build your Kernel with VB2 compiled, you'll have it.
> > > > 
> > > Sure.
> > > 
> > > So my question is:
> > > How good do the kernel origin vb2 and the media_build vb2 play together?
> > > 
> > > Will modprobe always choose the media_build one?
> > > Or will "make install" just overwrite the original file?  
> > 
> > make install *should* overwrite the old one. If not, then there's a problem
> > at the media-build makefile [1].
> >   
> 
> There is a problem. make install has been broken for at least a week,
> see the thread "media_build: fails to install"
> 
> The reason is that scripts/make_makefile.pl aborts
> 
> make[1]: Entering directory '/home/me/git/clones/media_build/v4l'
> scripts/make_makefile.pl
> Can't handle includes! In 
> ../linux/drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile at
> scripts/  make_makefile.pl line 109,  line 4.
> 
> is because that css2400/Makefile includes another:
> 
> $ cat ../linux/drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile
> 
> ccflags-y += -DISP2400B0
> ISP2400B0 := y
> 
> include $(srctree)/$(src)/../Makefile.common

Hmm...

$ git grep Makefile.common
drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile:include 
$(srctree)/$(src)/../Makefile.common
$ find . -name Makefile.common
$

There's no Makefile.common! This is just a bogus line.
 
> The abort of scripts/make_makefile.pl means that the v4l/Makefile
> does not get completely written out, in particular the rules for
> making the 'media-install' target.
> 
> I am not sure how to fix this. The make_makefile.pl deliberately
> falls over when given an include to deal with, so there must be
> some other mechanism in the media_build framework that handles
> this kind of thing. But I am not aware of it. 

I just fixed it: make install should work again.

I also fixed some recent breakages after fwnode patches.
So, at least for me (built against 4.10.17-200.fc25.x86_64) it works.

Thanks,
Mauro

[PATCH] [media] staging: css2400/Makefile: don't include makefiles

The atomisp css2400/Makefile includes a Makefile.common:

include $(srctree)/$(src)/../Makefile.common

Well, this file doesn't exist at the Kernel tree :-)

So, don't include it.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile
index 04defaafa02c..ee5631b0e635 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile
@@ -1,4 +1,2 @@
 ccflags-y += -DISP2400B0
 ISP2400B0 := y
-
-include $(srctree)/$(src)/../Makefile.common
-- 
2.9.4

Re: linux-next: Tree for Jun 8 (drivers/media/i2c/adv7604.c)

2017-06-08 Thread Randy Dunlap

On 06/08/17 00:50, Stephen Rothwell wrote:
> Hi all,
> 
> Changes since 20170607:
> 

on x86_64: (randconfig)

CONFIG_VIDEO_ADV7604=y
# CONFIG_VIDEO_ADV7604_CEC is not set
CONFIG_COMPILE_TEST=y


drivers/built-in.o: In function `adv76xx_unregistered':
adv7604.c:(.text+0x43ad91): undefined reference to `cec_unregister_adapter'
drivers/built-in.o: In function `adv76xx_registered':
adv7604.c:(.text+0x43adc4): undefined reference to `cec_register_adapter'
adv7604.c:(.text+0x43adef): undefined reference to `cec_delete_adapter'
drivers/built-in.o: In function `adv76xx_set_edid':
adv7604.c:(.text+0x43daab): undefined reference to `cec_get_edid_phys_addr'
adv7604.c:(.text+0x43dabd): undefined reference to `cec_phys_addr_validate'
adv7604.c:(.text+0x43df5d): undefined reference to `cec_s_phys_addr'




-- 
~Randy

[PATCH] [media] platform/Makefile: don't depend on arch to include dirs

Depending on arch configs to include dirs is evil, and makes
harder to change drivers to work with COMPILE_TEST.

Replace them by obj-y.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 231f3c2894c9..c3588d570f5d 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -44,9 +44,9 @@ obj-$(CONFIG_VIDEO_STI_HDMI_CEC)  += sti/cec/
 
 obj-$(CONFIG_VIDEO_STI_DELTA)  += sti/delta/
 
-obj-$(CONFIG_BLACKFIN)  += blackfin/
+obj-y   += blackfin/
 
-obj-$(CONFIG_ARCH_DAVINCI) += davinci/
+obj-y  += davinci/
 
 obj-$(CONFIG_VIDEO_SH_VOU) += sh_vou.o
 
-- 
2.9.4

Re: [PATCH 2/3] media: ngene: Replace semaphore stream_mutex with mutex

On Thu, Jun 8, 2017 at 12:04 PM, Binoy Jayan  wrote:
> The semaphore 'stream_mutex' is used as a simple mutex, so
> it should be written as one. Semaphores are going away in the future.
>
> Signed-off-by: Binoy Jayan 
> ---

Looks correct, though I wonder whether it would be nicer to move the
mutex_lock/unlock() to the caller to avoid repeating the unlock() five
times.

Either way,

Reviewed-by: Arnd Bergmann

Re: [PATCH 3/3] media: ngene: Replace semaphore i2c_switch_mutex with mutex

On Thu, Jun 8, 2017 at 12:04 PM, Binoy Jayan  wrote:
> The semaphore 'i2c_switch_mutex' is used as a simple mutex, so
> it should be written as one. Semaphores are going away in the future.
>
> Signed-off-by: Binoy Jayan 

This one is obviously correct,

Reviewed-by: Arnd Bergmann

Re: [PATCH 1/3] media: ngene: Replace semaphore cmd_mutex with mutex

On Thu, Jun 8, 2017 at 12:04 PM, Binoy Jayan  wrote:
> The semaphore 'cmd_mutex' is used as a simple mutex, so
> it should be written as one. Semaphores are going away in the future.
>
> Signed-off-by: Binoy Jayan 
> ---

> @@ -1283,7 +1283,7 @@ static int ngene_load_firm(struct ngene *dev)
>
>  static void ngene_stop(struct ngene *dev)
>  {
> -   down(>cmd_mutex);
> +   mutex_lock(>cmd_mutex);
> i2c_del_adapter(&(dev->channel[0].i2c_adapter));
> i2c_del_adapter(&(dev->channel[1].i2c_adapter));
> ngwritel(0, NGENE_INT_ENABLE);

Are you sure about this one? There is only one mutex_lock() and
then the structure gets freed without a corresponding mutex_unlock().

I suspect this violates some rules of mutexes, either when compile
testing with "make C=1", or when running with lockdep enabled.

Can we actually have a concurrently held mutex at the time we
get here? If not, using mutex_destroy() in place of the down()
may be the right answer.

   Arnd

[PATCH 1/1] davinci: Switch from V4L2 OF to V4L2 fwnode

The DaVinci VPIF capture driver V4L2 OF support was added after the V4L2
OF framework got removed. Switch VPIF capture driver to V4L2 fwnode.

Signed-off-by: Sakari Ailus 
---
 drivers/media/platform/davinci/Kconfig|  1 +
 drivers/media/platform/davinci/vpif_capture.c | 21 -
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/davinci/Kconfig 
b/drivers/media/platform/davinci/Kconfig
index 554e710..55982e6 100644
--- a/drivers/media/platform/davinci/Kconfig
+++ b/drivers/media/platform/davinci/Kconfig
@@ -22,6 +22,7 @@ config VIDEO_DAVINCI_VPIF_CAPTURE
depends on HAS_DMA
depends on I2C
select VIDEOBUF2_DMA_CONTIG
+   select V4L2_FWNODE
help
  Enables Davinci VPIF module used for capture devices.
  This module is used for capture on TI DM6467/DA850/OMAPL138
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 2735795..d78580f 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -18,11 +18,12 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
+#include 
 #include 
-#include 
 #include 
 #include 
 
@@ -1389,15 +1390,16 @@ static int vpif_async_bound(struct v4l2_async_notifier 
*notifier,
 
for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
-   const struct device_node *node = _asd->match.of.node;
+   const struct fwnode_handle *fwnode = _asd->match.fwnode.fwnode;
 
-   if (node == subdev->of_node) {
+   if (fwnode == subdev->fwnode) {
vpif_obj.sd[i] = subdev;
vpif_obj.config->chan_config->inputs[i].subdev_name =
-   (char *)subdev->of_node->full_name;
+   (char *)to_of_node(subdev->fwnode)->full_name;
vpif_dbg(2, debug,
 "%s: setting input %d subdev_name = %s\n",
-__func__, i, subdev->of_node->full_name);
+__func__, i,
+to_of_node(subdev->fwnode)->full_name);
return 0;
}
}
@@ -1502,7 +1504,7 @@ static struct vpif_capture_config *
 vpif_capture_get_pdata(struct platform_device *pdev)
 {
struct device_node *endpoint = NULL;
-   struct v4l2_of_endpoint bus_cfg;
+   struct v4l2_fwnode_endpoint bus_cfg;
struct vpif_capture_config *pdata;
struct vpif_subdev_info *sdinfo;
struct vpif_capture_chan_config *chan;
@@ -1549,7 +1551,8 @@ vpif_capture_get_pdata(struct platform_device *pdev)
chan->inputs[i].input.std = V4L2_STD_ALL;
chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
 
-   err = v4l2_of_parse_endpoint(endpoint, _cfg);
+   err = v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint),
+_cfg);
if (err) {
dev_err(>dev, "Could not parse the endpoint\n");
goto done;
@@ -1584,8 +1587,8 @@ vpif_capture_get_pdata(struct platform_device *pdev)
goto done;
}
 
-   pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
-   pdata->asd[i]->match.of.node = rem;
+   pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
+   pdata->asd[i]->match.fwnode.fwnode = of_fwnode_handle(rem);
of_node_put(rem);
}
 
-- 
2.7.4

Re: [media_build] regression at 3a17e11 "update v4.10_sched_signal.patch"

On 08/06/17 15:28, Vincent McIntyre wrote:
> I managed to find the failing patch, not sure what the fix is.
> 
> $ cd linux/
> $ patch -f -N -p1 -i ../backports/v4.10_sched_signal.patch
> patching file drivers/media/dvb-core/dvb_ca_en50221.c
> Hunk #1 succeeded at 35 (offset 1 line).
> patching file drivers/media/dvb-core/dvb_demux.c
> Hunk #1 succeeded at 20 with fuzz 1 (offset 1 line).
> patching file drivers/media/dvb-core/dvb_frontend.c
> Hunk #1 succeeded at 30 (offset 1 line).
> patching file drivers/media/pci/cx18/cx18-driver.h
> patching file drivers/media/pci/ivtv/ivtv-driver.c
> patching file drivers/media/pci/ivtv/ivtv-driver.h
> Hunk #1 succeeded at 39 (offset 1 line).
> patching file drivers/media/pci/pt1/pt1.c
> patching file drivers/media/pci/pt3/pt3.c
> patching file drivers/media/pci/solo6x10/solo6x10-i2c.c
> patching file drivers/media/pci/zoran/zoran_device.c
> patching file drivers/media/platform/vivid/vivid-radio-rx.c
> patching file drivers/media/platform/vivid/vivid-radio-tx.c
> patching file drivers/media/rc/lirc_dev.c
> Hunk #1 FAILED at 18.
> 1 out of 1 hunk FAILED -- saving rejects to file 
> drivers/media/rc/lirc_dev.c.rej
> patching file drivers/media/usb/cpia2/cpia2_core.c
> patching file drivers/media/usb/gspca/cpia1.c
> Hunk #1 succeeded at 28 (offset 1 line).
> patching file drivers/media/v4l2-core/videobuf-dma-sg.c
> patching file drivers/staging/media/lirc/lirc_zilog.c
> patching file include/media/v4l2-ioctl.h
> 
> $ cat drivers/media/rc/lirc_dev.c.rej
> --- drivers/media/rc/lirc_dev.c
> +++ drivers/media/rc/lirc_dev.c
> @@ -18,7 +18,7 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> 

Odd, it applies cleanly here.

But don't bother, the media_build totally broke after the latest round of
commits to the master. We're looking into that, but it might take a bit
of time before it is resolved.

Regards,

Hans

Re: [PATCH 03/12] intel-ipu3: Add DMA API implementation

On Thu, Jun 8, 2017 at 10:22 PM, Robin Murphy  wrote:
> On 07/06/17 10:47, Tomasz Figa wrote:
>> Hi Yong,
>>
>> +Robin, Joerg, IOMMU ML
>>
>> Please see my comments inline.
>>
>> On Tue, Jun 6, 2017 at 5:39 AM, Yong Zhi  wrote:
[snip]
>>> +
>>> +/* End of things adapted from arch/arm/mm/dma-mapping.c */
>>> +static void ipu3_dmamap_sync_single_for_cpu(struct device *dev,
>>> +   dma_addr_t dma_handle, size_t 
>>> size,
>>> +   enum dma_data_direction dir)
>>> +{
>>> +   struct ipu3_mmu *mmu = to_ipu3_mmu(dev);
>>> +   dma_addr_t daddr = iommu_iova_to_phys(mmu->domain, dma_handle);
>>> +
>>> +   clflush_cache_range(phys_to_virt(daddr), size);
>>
>> You might need to consider another IOMMU on the way here. Generally,
>> given that daddr is your MMU DMA address (not necessarily CPU physical
>> address), you should be able to call
>>
>> dma_sync_single_for_cpu(, daddr, size, dir)
>
> I'd hope that this IPU complex is some kind of embedded endpoint thing
> that bypasses the VT-d IOMMU or is always using its own local RAM,
> because it would be pretty much unworkable otherwise.

It uses system RAM and, as far as my understanding goes, by default it
operates without the VT-d IOMMU and that's how it's used right now.
I'm suggesting VT-d IOMMU as a way to further strengthen the security
and error resilience in future (due to the IPU complex being
non-coherent and also running a closed source firmware).

> The whole
> infrastructure isn't really capable of dealing with nested IOMMUs, and
> nested DMA ops would be an equally horrible idea.

Could you elaborate a bit more on this? I think we should be able to
deal with this in a way I suggested before:

a) the PCI device would use the system DMA ops,
b) the PCI device would implement a secondary bus for which it would
provide its own DMA and IOMMU ops.
c) a secondary device would be registered on the secondary bus,
d) all memory for the IPU would be managed on behalf of the secondary device.

In fact, the driver already is designed in a way that all the points
above are true. If I'm not missing something, the only significant
missing point is calling into system DMA ops from IPU DMA ops.

Best regards,
Tomasz

Re: [media_build] regression at 3a17e11 "update v4.10_sched_signal.patch"

> 
> $ cat drivers/media/rc/lirc_dev.c.rej
> --- drivers/media/rc/lirc_dev.c
> +++ drivers/media/rc/lirc_dev.c
> @@ -18,7 +18,7 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> 

A bit of staring brings this to light:

The file that is being patched has extra lines relative to the patch

 18 #undef pr_fmt
 19 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 20 
 21 #include 
**   22 #include 
 23 #include 
**   24 #include 
 25 #include 
 26 #include 
 27 #include 
 28 #include 
 29 #include 
 30 #include 

This hunk applies cleanly, and seems to match up with recent kernel
code (eg
   174cd4b1e5fbd0d74c68cf3a74f5bd4923485512
   sched/headers: Prepare to move signal wakeup & sigpending methods
from  into )

@@ -20,7 +20,7 @@
 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 

HTH
Vince

Re: [PATCH 6/7] [media] soc_camera: rcar_vin: use proper name for the R-Car SoC

2017-06-08 Thread Wolfram Sang


> Btw, I'm seeing only patches 6 and 7 here at media ML (and patchwork).
> As those are trivial changes, I'll just apply what I have.

Perfect, thanks!



signature.asc
Description: PGP signature

Re: [media_build] regression at 3a17e11 "update v4.10_sched_signal.patch"

I managed to find the failing patch, not sure what the fix is.

$ cd linux/
$ patch -f -N -p1 -i ../backports/v4.10_sched_signal.patch
patching file drivers/media/dvb-core/dvb_ca_en50221.c
Hunk #1 succeeded at 35 (offset 1 line).
patching file drivers/media/dvb-core/dvb_demux.c
Hunk #1 succeeded at 20 with fuzz 1 (offset 1 line).
patching file drivers/media/dvb-core/dvb_frontend.c
Hunk #1 succeeded at 30 (offset 1 line).
patching file drivers/media/pci/cx18/cx18-driver.h
patching file drivers/media/pci/ivtv/ivtv-driver.c
patching file drivers/media/pci/ivtv/ivtv-driver.h
Hunk #1 succeeded at 39 (offset 1 line).
patching file drivers/media/pci/pt1/pt1.c
patching file drivers/media/pci/pt3/pt3.c
patching file drivers/media/pci/solo6x10/solo6x10-i2c.c
patching file drivers/media/pci/zoran/zoran_device.c
patching file drivers/media/platform/vivid/vivid-radio-rx.c
patching file drivers/media/platform/vivid/vivid-radio-tx.c
patching file drivers/media/rc/lirc_dev.c
Hunk #1 FAILED at 18.
1 out of 1 hunk FAILED -- saving rejects to file drivers/media/rc/lirc_dev.c.rej
patching file drivers/media/usb/cpia2/cpia2_core.c
patching file drivers/media/usb/gspca/cpia1.c
Hunk #1 succeeded at 28 (offset 1 line).
patching file drivers/media/v4l2-core/videobuf-dma-sg.c
patching file drivers/staging/media/lirc/lirc_zilog.c
patching file include/media/v4l2-ioctl.h

$ cat drivers/media/rc/lirc_dev.c.rej
--- drivers/media/rc/lirc_dev.c
+++ drivers/media/rc/lirc_dev.c
@@ -18,7 +18,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include

Re: [PATCH 03/12] intel-ipu3: Add DMA API implementation

2017-06-08 Thread Robin Murphy

On 07/06/17 10:47, Tomasz Figa wrote:
> Hi Yong,
> 
> +Robin, Joerg, IOMMU ML
> 
> Please see my comments inline.
> 
> On Tue, Jun 6, 2017 at 5:39 AM, Yong Zhi  wrote:
>> IPU3 mmu based DMA mapping driver
>>
>> Signed-off-by: Yong Zhi 
>> ---
>>  drivers/media/pci/intel/ipu3/Kconfig   |   6 +
>>  drivers/media/pci/intel/ipu3/Makefile  |   1 +
>>  drivers/media/pci/intel/ipu3/ipu3-dmamap.c | 408 
>> +
>>  drivers/media/pci/intel/ipu3/ipu3-dmamap.h |  20 ++
>>  4 files changed, 435 insertions(+)
>>  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.c
>>  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.h
>>
>> diff --git a/drivers/media/pci/intel/ipu3/Kconfig 
>> b/drivers/media/pci/intel/ipu3/Kconfig
>> index ab2edcb..2030be7 100644
>> --- a/drivers/media/pci/intel/ipu3/Kconfig
>> +++ b/drivers/media/pci/intel/ipu3/Kconfig
>> @@ -26,3 +26,9 @@ config INTEL_IPU3_MMU
>>
>>   Say Y here if you have Skylake/Kaby Lake SoC with IPU3.
>>   Say N if un-sure.
>> +
>> +config INTEL_IPU3_DMAMAP
>> +   bool "Intel ipu3 DMA mapping driver"
>> +   select IOMMU_IOVA
>> +   ---help---
>> + This is IPU3 IOMMU domain specific DMA driver.
>> diff --git a/drivers/media/pci/intel/ipu3/Makefile 
>> b/drivers/media/pci/intel/ipu3/Makefile
>> index 2b669df..2c2a035 100644
>> --- a/drivers/media/pci/intel/ipu3/Makefile
>> +++ b/drivers/media/pci/intel/ipu3/Makefile
>> @@ -1,2 +1,3 @@
>>  obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
>>  obj-$(CONFIG_INTEL_IPU3_MMU) += ipu3-mmu.o
>> +obj-$(CONFIG_INTEL_IPU3_DMAMAP) += ipu3-dmamap.o
>> diff --git a/drivers/media/pci/intel/ipu3/ipu3-dmamap.c 
>> b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
>> new file mode 100644
>> index 000..74704d9
>> --- /dev/null
>> +++ b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
>> @@ -0,0 +1,408 @@
>> +/*
>> + * Copyright (c) 2017 Intel Corporation.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include "ipu3-mmu.h"
>> +
>> +/* Begin of things adapted from arch/arm/mm/dma-mapping.c */
> 
> ARM's DMA ops are not a good example of today's coding standards.
> There are already generic DMA mapping helpers available in
> drivers/iommu/dma-iommu.c and drivers/base/dma-*. (Hmm, I remember
> writing this already, déjà vu maybe...)

Yes, dma-iommu exists for precisely this purpose -
arch/arm64/mm/dma-mapping.c would have been a better point of reference.

>> +static void ipu3_dmamap_clear_buffer(struct page *page, size_t size,
>> +unsigned long attrs)
>> +{
>> +   /*
>> +* Ensure that the allocated pages are zeroed, and that any data
>> +* lurking in the kernel direct-mapped region is invalidated.
>> +*/
>> +   if (PageHighMem(page)) {
>> +   while (size > 0) {
>> +   void *ptr = kmap_atomic(page);
>> +
>> +   memset(ptr, 0, PAGE_SIZE);
>> +   if ((attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
>> +   clflush_cache_range(ptr, PAGE_SIZE);
>> +   kunmap_atomic(ptr);
>> +   page++;
>> +   size -= PAGE_SIZE;
>> +   }
>> +   } else {
>> +   void *ptr = page_address(page);
>> +
>> +   memset(ptr, 0, size);
>> +   if ((attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
>> +   clflush_cache_range(ptr, size);
>> +   }
>> +}
>> +
>> +/**
>> + * ipu3_dmamap_alloc_buffer - allocate buffer based on attributes
>> + * @dev: struct device pointer
>> + * @size: size of buffer in bytes
>> + * @gfp: specify the free page type
>> + * @attrs: defined in linux/dma-attrs.h
>> + *
>> + * This is a helper function for physical page allocation
>> + *
>> + * Return array representing buffer from alloc_pages() on success
>> + * or NULL on failure
>> + *
>> + * Must be freed with ipu3_dmamap_free_buffer.
>> + */
>> +static struct page **ipu3_dmamap_alloc_buffer(struct device *dev, size_t 
>> size,
>> + gfp_t gfp, unsigned long attrs)
>> +{
>> +   struct page **pages;
>> +   int count = size >> PAGE_SHIFT;
>> +   int array_size = count * sizeof(struct page *);
>> +   int i = 0;
>> +
>> +   /* Allocate mem for array of page ptrs */
>> +   if (array_size <= PAGE_SIZE)
>> +   pages = kzalloc(array_size, GFP_KERNEL);

[media_build] regression at 3a17e11 "update v4.10_sched_signal.patch"

Hi

I think the build was broken by this commit.

  3a17e11 "update v4.10_sched_signal.patch"

It's been fun learning about git bisect.

$ cat /etc/lsb-release 
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=16.04
DISTRIB_CODENAME=xenial
DISTRIB_DESCRIPTION="Ubuntu 16.04.2 LTS"

The build script falls over on these kernels, in the same way

 4.4.0-77-generic x86_64
 4.8.0-53-generic x86_64


$ ./build --main-git --depth 1 -v 1
...
**
* Start building *
* **
* $ make allyesconfig
* make -C /home/me/git/clones/media_build/v4l allyesconfig
* make[1]: Entering directory '/home/me/git/clones/media_build/v4l'
* make[2]: Entering directory '/home/me/git/clones/media_build/linux'
* Syncing with dir ../media/
* Applying patches for kernel 4.4.0-77-generic
* patch -s -f -N -p1 -i ../backports/api_version.patch
* patch -s -f -N -p1 -i ../backports/pr_fmt.patch
* patch -s -f -N -p1 -i ../backports/debug.patch
* patch -s -f -N -p1 -i ../backports/drx39xxj.patch
* patch -s -f -N -p1 -i ../backports/v4.10_sched_signal.patch
* 1 out of 1 hunk FAILED
* Makefile:137: recipe for target 'apply_patches' failed
* make[2]: *** [apply_patches] Error 1
* make[2]: Leaving directory '/home/me/git/clones/media_build/linux'
* Makefile:383: recipe for target 'allyesconfig' failed
* make[1]: *** [allyesconfig] Error 2
* make[1]: Leaving directory '/home/me/git/clones/media_build/v4l'
* Makefile:26: recipe for target 'allyesconfig' failed
* make: *** [allyesconfig] Error 2
* can't select all drivers at ./build.new line 521
*

Hopefully this of some use to someone
Vince

Re: [PATCH] Revert "[media] et8ek8: Export OF device ID as module aliases"

On Thu, Jun 08, 2017 at 11:01:37AM +0200, Arnd Bergmann wrote:
> This one got applied twice, causing a build error with clang:
> 
> drivers/media/i2c/et8ek8/et8ek8_driver.c:1499:1: error: redefinition of 
> '__mod_of__et8ek8_of_table_device_table'
> 
> Fixes: 9ae05fd1e791 ("[media] et8ek8: Export OF device ID as module aliases")
> Signed-off-by: Arnd Bergmann 

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [RFC] V4L2 unified low-level decoder API

On Thu, Jun 8, 2017 at 8:11 PM, ayaka  wrote:
>
>
> On 06/08/2017 06:56 PM, Hans Verkuil wrote:
>>
>> Hi Alexandre,
>>
>> On 08/06/17 11:59, Alexandre Courbot wrote:
>>>
>>> On Thu, Jun 8, 2017 at 5:56 PM, Pawel Osciak 
>>> wrote:

 Hi,

 On Fri, May 19, 2017 at 1:08 AM, Hugues FRUCHET 
 wrote:
>
> Before merging this work Hans would like to have feedback from peers,
> in
> order to be sure that this is inline with other SoC vendors drivers
> expectations.
>
> Thomasz, Pawel, could you give your view regarding ChromeOS and
> Rockchip
> driver ?

 The drivers for Rockchip codecs are submitted to the public Chromium OS
 kernel
 and working on our RK-based platforms. We have also since added a VP9
 API as
>
> That driver still lacks a number of feature comparing to the rockchip
> android driver, since google never requests them. Also the performance is
> not as good as the android one.

I'm not sure exactly what features or performance problems you're
mentioning, but that's not the point. It's a reasonably simple driver
without too much complexity, written with good V4L2 codec API
compliance in mind and proven in the industry. I see it as a good
starting point.

> That is why I start to write a new driver myself.

I think it would make sense to work on incrementally adding those
missing features and performance optimizations, instead of spending
time on unnecessary effort of doing the same basic work, which is
already done in our driver.

> Also the rockchip platform is very complex, that driver won't be work on all
> the SoCs without a large of modification(now only two chips are supported)

This is another story. The rockchip-vpu driver in ChromeOS 4.4 should
be actually refactored into generic V4L2 stateless codec helpers (most
of the shared code in current driver) + few separate drivers using
those helpers, one for each physical IP block. Current design is there
only because we thought there is more similarity between those IP
blocks and we didn't have more time to actually clean it up later,
after we realized that the assumption was false.

Best regards,
Tomasz

Re: [RFC PATCH] [media] v4l2-subdev: check colorimetry in link validate

Em Tue, 6 Jun 2017 19:15:34 -0300
Helen Koike  escreveu:

> Hi Sakari,
> 
> 
> Thanks for replying
> 
> On 2017-05-31 03:31 AM, Sakari Ailus wrote:
> > Hi Helen,
> >
> > On Tue, May 30, 2017 at 04:08:08PM -0300, Helen Koike wrote:  
> >> colorspace, ycbcr_enc, quantization and xfer_func must match across the
> >> link.
> >> Check if they match in v4l2_subdev_link_validate_default unless they are
> >> set as _DEFAULT.
> >>
> >> Signed-off-by: Helen Koike 
> >>
> >> ---
> >>
> >> Hi,
> >>
> >> I think we should validate colorimetry as having different colorimetry
> >> across a link doesn't make sense.
> >> But I am confused about what to do when they are set to _DEFAULT, what
> >> do you think?  
> >
> > These fields were added at various points over the course of the past three
> > years or so. User space code that was written before that will certainly not
> > set anything and I'm not sure many drivers care about these fields nor they
> > are relevant for many formats. In practice that means that they are very
> > likely zero in many cases.  
> 
> If they are set to zero then they won't be affected by this patch.
> 
> >
> > Driver changes will probably be necessary for removing the explicit checks
> > for the default value.  
> 
> At least in the drivers/media tree I couldn't find many drivers that 
> implement its own link_validate, there is only 
> platform/omap3isp/ispccdc.c and platform/omap3isp/ispresizer.c that 
> implements a custom value, but from a quick look it doesn't seems that 
> they will be affected.
> 
> >
> > The same applies to checking the colour space: drivers should enforce using
> > the correct colour space before the check can be merged. I might move that
> > to a separate patch.  
> 
> I am not sure if I got what you mean. If driver don't care about 
> colourspace then probably it will be set to zero and won't be affected 
> by this patch, if colourspace is different across the link then the user 
> space must set both ends to the same colourspace.

I guess what Sakari is concerned about is to avoid regressions.

Colorimetry properties were added after the addition of most of
the drivers. Adding a mandatory link validation may break drivers
that don't set it right. Even on new drivers, this may not be ok.

Just to give you an example, this week I just applied a patch fixing
colorimetry handling at the coda driver:

https://git.linuxtv.org/media_tree.git/commit/?id=b14ac545688d8cc4b2b707d71d106799ad476964

If a change like that would have been applied before such fix, it 
could be breaking coda.

So, before adding such patch, we need to check how existing drivers are
setting colorimetry fields, to be sure that the ones that don't
touch it won't break.

Perhaps one alternative would be to write a patchset that would,
instead, print a warning at dmesg, and let it be upstream for a
while, to give people time to check if the colorimetry logic is
ok at the subdevs.

Thanks,
Mauro

Re: [PATCH 6/7] [media] soc_camera: rcar_vin: use proper name for the R-Car SoC

Em Mon, 29 May 2017 14:02:02 +0200
Wolfram Sang  escreveu:

> >Why "soc_camera:" in the subject?  
> 
> I used 'git log $file' and copied the most recent entry. Do I need to
> resend?
> 

No need to resend. I'll fix it when applying the patch.

Btw, I'm seeing only patches 6 and 7 here at media ML (and patchwork).
As those are trivial changes, I'll just apply what I have.

Thanks,
Mauro

Re: Unknown symbol put_vaddr_frames when using media_build

On Wed, Jun 07, 2017 at 08:12:01PM -0300, Mauro Carvalho Chehab wrote:
> Em Wed, 7 Jun 2017 22:17:50 +0200
> Matthias Schwarzott  escreveu:
> 
> > Am 07.06.2017 um 20:23 schrieb Mauro Carvalho Chehab:
> > > Em Tue, 9 May 2017 06:56:25 +0200
> > > Matthias Schwarzott  escreveu:
> > >   
> > >> Hi!
> > >>
> > >> Whenever I compile the media drivers using media_build against a recent
> > >> kernel, I get this message when loading them:
> > >>
> > >> [5.848537] media: Linux media interface: v0.10
> > >> [5.881440] Linux video capture interface: v2.00
> > >> [5.881441] WARNING: You are using an experimental version of the
> > >> media stack.
> > >> ...
> > >> [6.166390] videobuf2_memops: Unknown symbol put_vaddr_frames (err 0)
> > >> [6.166394] videobuf2_memops: Unknown symbol get_vaddr_frames (err 0)
> > >> [6.166396] videobuf2_memops: Unknown symbol frame_vector_destroy 
> > >> (err 0)
> > >> [6.166398] videobuf2_memops: Unknown symbol frame_vector_create (err 
> > >> 0)
> > >>
> > >> That means I am not able to load any drivers being based on
> > >> videobuf2_memops without manual actions.
> > >>
> > >> I used kernel 4.11.0, but it does not matter which kernel version
> > >> exactly is used.
> > >>
> > >> My solution for that has been to modify mm/Kconfig of my kernel like
> > >> this and then enable FRAME_VECTOR in .config  
> > > 
> > > Well, if you build your Kernel with VB2 compiled, you'll have it.
> > >   
> > Sure.
> > 
> > So my question is:
> > How good do the kernel origin vb2 and the media_build vb2 play together?
> > 
> > Will modprobe always choose the media_build one?
> > Or will "make install" just overwrite the original file?
> 
> make install *should* overwrite the old one. If not, then there's a problem
> at the media-build makefile [1].
> 

There is a problem. make install has been broken for at least a week,
see the thread "media_build: fails to install"

The reason is that scripts/make_makefile.pl aborts

make[1]: Entering directory '/home/me/git/clones/media_build/v4l'
scripts/make_makefile.pl
Can't handle includes! In 
../linux/drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile at
scripts/  make_makefile.pl line 109,  line 4.

is because that css2400/Makefile includes another:

$ cat ../linux/drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile

ccflags-y += -DISP2400B0
ISP2400B0 := y

include $(srctree)/$(src)/../Makefile.common

The abort of scripts/make_makefile.pl means that the v4l/Makefile
does not get completely written out, in particular the rules for
making the 'media-install' target.

I am not sure how to fix this. The make_makefile.pl deliberately
falls over when given an include to deal with, so there must be
some other mechanism in the media_build framework that handles
this kind of thing. But I am not aware of it. 

Vince

Re: [RFC] V4L2 unified low-level decoder API

2017-06-08 Thread ayaka




On 06/08/2017 06:56 PM, Hans Verkuil wrote:

Hi Alexandre,

On 08/06/17 11:59, Alexandre Courbot wrote:

On Thu, Jun 8, 2017 at 5:56 PM, Pawel Osciak  wrote:

Hi,

On Fri, May 19, 2017 at 1:08 AM, Hugues FRUCHET  wrote:

Before merging this work Hans would like to have feedback from peers, in
order to be sure that this is inline with other SoC vendors drivers
expectations.

Thomasz, Pawel, could you give your view regarding ChromeOS and Rockchip
driver ?

The drivers for Rockchip codecs are submitted to the public Chromium OS kernel
and working on our RK-based platforms. We have also since added a VP9 API as
That driver still lacks a number of feature comparing to the rockchip 
android driver, since google never requests them. Also the performance 
is not as good as the android one.

That is why I start to write a new driver myself.
Also the rockchip platform is very complex, that driver won't be work on 
all the SoCs without a large of modification(now only two chips are 
supported)

well, which is also working on devices that support it. This gives us
a set of H.264,
VP8 and VP9 APIs on both kernel and userspace side (in the open source
Chromium browser) that are working currently and can be used for
further testing.

We are interested in merging the API patches as well as these drivers upstream
(they were posted on this list two years ago), however we've been blocked by the
progress of request API, which is required for this. Alexandre Courbot
is currently
Well the request API looks fine for me, I just can't understand why it 
is not merged except those are a few functions have a reference problem 
stopping build v4l2 as a module.

looking into creating a minimal version of the request API that would provide
enough functionality for stateless codecs, and also plans to further work on
re-submitting the particular codec API patches, once the request API
is finalized.

Hi everyone,

It is a bit scary to start hacking on V4L with something as disruptive
as the request API, so please bear with me as I will inevitably post
code that will go from cutely clueless to downright offensive.

Yeah, you went straight into the deep end of the pool :-)

I am very, very pleased to see Google picking up this work. We need more
core V4L2 developers, so welcome!


Thankfully Pawel is not too far from my desk, and we (Pawel, Tomasz
and myself) had a very fruitful in-person brainstorming session last
week with Laurent, so this should limit the damage.

In any case, I think that everybody agrees that this needs to be
pushed forward. Chromium OS in particular has a big interest to see
this feature landing upstream, so I will dedicate some cycles to this.

Absolutely!


 From reading the meetings reports (e.g.
https://www.spinics.net/lists/linux-media/msg106699.html) I understand
that we want to support several use-cases with this and we already
have several proposals with code. Chromium in a first time is
interested in stateless codecs support, and this use-case also seems
to be the simplest to implement, so we would like to start pushing in
that direction first. This should give us a reasonably sized code base
to rely upon and implement the other use-cases as we see clearer
through this.

I still need to study a bit the existing proposals and to clearly lay
out the conclusions of our meeting with Laurent, but the general idea
has not changed too much, except maybe that we thought it may be nice
to make state management less explicit to userspace by default. I
would be interested in knowing whether there are updated versions of
the implementations mentioned in the meeting report above, and/or a
merge of these work? Also, if someone is actively working on this at
the moment, we will definitely want to sync on IRC or anywhere else.

Laurent has been the last one working on this, but his code doesn't have
the control handling :-(

My latest (well, December 2015) tree with the control request code
is here:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=requests2

It's AFAIK a slightly newer version from what ChromeOS uses.


Excited to work with you all! :)

Looking forward to your code!

Regards,

Hans

RE: [PATCH v6 3/7] media: i2c: max2175: Add MAX2175 support

2017-06-08 Thread Ramesh Shanmugasundaram


> Em Thu, 8 Jun 2017 09:42:43 +
> Ramesh Shanmugasundaram  escreveu:
> 
> > > Subject: Re: [PATCH v6 3/7] media: i2c: max2175: Add MAX2175 support
> > >
> > > Em Wed, 31 May 2017 09:44:53 +0100
> > > Ramesh Shanmugasundaram 
> escreveu:
> > >
> > > > +++ b/Documentation/media/v4l-drivers/max2175.rst
> > > > @@ -0,0 +1,60 @@
> > > > +Maxim Integrated MAX2175 RF to bits tuner driver
> > > > +
> > > > +
> > > > +The MAX2175 driver implements the following driver-specific
> controls:
> > > > +
> > > > +``V4L2_CID_MAX2175_I2S_ENABLE``
> > > > +---
> > > > +Enable/Disable I2S output of the tuner.
> > > > +
> > > > +.. flat-table::
> > > > +:header-rows:  0
> > > > +:stub-columns: 0
> > > > +:widths:   1 4
> > > > +
> > > > +* - ``(0)``
> > > > +  - I2S output is disabled.
> > > > +* - ``(1)``
> > > > +  - I2S output is enabled.
> > >
> > > Hmm... There are other drivers at the subsystem that use I2S (for
> > > audio - not for SDR - but I guess the issue is similar).
> > >
> > > On such drivers, the bridge driver controls it directly, being sure
> > > that I2S is enabled when it is expecting some data coming from the I2S
> bus.
> > >
> > > On some drivers, there are both I2S and A/D inputs at the bridge
> chipset.
> > > On such drivers, enabling/disabling I2S is done via VIDIOC_S_INPUT
> > > (and optionally via ALSA mixer), being transparent to the user if
> > > the stream comes from a tuner via I2S or from a directly connected A/D
> input.
> > >
> > > I don't think it is a good idea to enable it via a control, as, if
> > > the bridge driver is expecting data via I2S, disabling it will cause
> > > timeouts at the videobuf handling.
> >
> > The MAX2175 device is exposed as a v4l2 subdev with tuner ops and can
> interface with an SDR device. When the tuner is configured, the I2S output
> is enabled by default. From an independent tuner device perspective, this
> default behaviour is enough and this control may not be needed/used.
> >
> > However, for the use case here, the R-Car DRIF device acts as the main
> SDR device and the Maxim MAX2175 provides a sub-dev interface with tuner
> ops.
> >
> > +-++-+
> > | |-SCK--->|CLK  |
> > |   Master|-SS>|SYNC  DRIFn (slave)  |
> > |  (MAX2175)  |-SD0--->|D0   |
> > | |-SD1--->|D1   |
> > +-++-+
> >
> > The DRIF device design is such that it involves separate register writes
> to enable Rx on each of the data line. To keep both the data lines in sync
> it expects the master device to enable output after both the data line Rx
> are enabled.
> >
> > This level of control is exposed as a feature in the MAX2175 using this
> control. When interfaced with DRIF this control is used to achieve the
> desired functionality. When not interfaced with DRIF, the MAX2175 default
> behaviour does not have to change because of DRIF and hence this I2S
> control may be unused. Like MAX2175, DRIF is also an independent device
> and can interface with a different third party tuner.
> >
> > Hence, this I2S enable/disable is exposed as a user control. The end
> user application (knowing both these devices) is expected to use these
> controls appropriately. Please let me know if I need to explain anything
> in further detail.
> 
> 
> The usecase is clear. That's exactly what other drivers with I2S do,
> except that, on those other drivers, they pass I2S control info via
> platform_data (they're not platform drivers).
> 
> With those drivers, generic applications work as-is via the standard
> video, radio or sdr devnodes, without knowing about I2S.
> 
> The main difference here is that you're requiring an specialized
> application for this device to work, 

Specialized application may be needed for this specific combination of devices 
(DRIF + MAX2175) only. For MAX2175 alone, a generic application itself would be 
enough. The MAX2175 will have I2S output enabled by default and the SDR device 
have to just enable/disable its own RX during .start_streaming/.stop_streaming.

as a generic one won't be aware of
> this device-specific control, and may end by exposing this "internal"
> control to the end user. That is OK for embedded usage, but, as soon as
> this is used on some non-embedded usecase (with is likely, as there are
> several PC consumer products using other chips from Maxim), we'll have
> problems.
> 
> I guess the solution here is to make such control visible only via the
> subdev interface.
> 

Did you mean using v4l2_subdev_tuner_ops? It does not have a method as of today 
to support this. The video, audio subdev ops supports s_stream method.

Re: [RFC] V4L2 unified low-level decoder API

Hi Alexandre,

On 08/06/17 11:59, Alexandre Courbot wrote:
> On Thu, Jun 8, 2017 at 5:56 PM, Pawel Osciak  wrote:
>> Hi,
>>
>> On Fri, May 19, 2017 at 1:08 AM, Hugues FRUCHET  
>> wrote:
>>> Before merging this work Hans would like to have feedback from peers, in
>>> order to be sure that this is inline with other SoC vendors drivers
>>> expectations.
>>>
>>> Thomasz, Pawel, could you give your view regarding ChromeOS and Rockchip
>>> driver ?
>>
>> The drivers for Rockchip codecs are submitted to the public Chromium OS 
>> kernel
>> and working on our RK-based platforms. We have also since added a VP9 API as
>> well, which is also working on devices that support it. This gives us
>> a set of H.264,
>> VP8 and VP9 APIs on both kernel and userspace side (in the open source
>> Chromium browser) that are working currently and can be used for
>> further testing.
>>
>> We are interested in merging the API patches as well as these drivers 
>> upstream
>> (they were posted on this list two years ago), however we've been blocked by 
>> the
>> progress of request API, which is required for this. Alexandre Courbot
>> is currently
>> looking into creating a minimal version of the request API that would provide
>> enough functionality for stateless codecs, and also plans to further work on
>> re-submitting the particular codec API patches, once the request API
>> is finalized.
> 
> Hi everyone,
> 
> It is a bit scary to start hacking on V4L with something as disruptive
> as the request API, so please bear with me as I will inevitably post
> code that will go from cutely clueless to downright offensive.

Yeah, you went straight into the deep end of the pool :-)

I am very, very pleased to see Google picking up this work. We need more
core V4L2 developers, so welcome!

> Thankfully Pawel is not too far from my desk, and we (Pawel, Tomasz
> and myself) had a very fruitful in-person brainstorming session last
> week with Laurent, so this should limit the damage.
> 
> In any case, I think that everybody agrees that this needs to be
> pushed forward. Chromium OS in particular has a big interest to see
> this feature landing upstream, so I will dedicate some cycles to this.

Absolutely!

> From reading the meetings reports (e.g.
> https://www.spinics.net/lists/linux-media/msg106699.html) I understand
> that we want to support several use-cases with this and we already
> have several proposals with code. Chromium in a first time is
> interested in stateless codecs support, and this use-case also seems
> to be the simplest to implement, so we would like to start pushing in
> that direction first. This should give us a reasonably sized code base
> to rely upon and implement the other use-cases as we see clearer
> through this.
> 
> I still need to study a bit the existing proposals and to clearly lay
> out the conclusions of our meeting with Laurent, but the general idea
> has not changed too much, except maybe that we thought it may be nice
> to make state management less explicit to userspace by default. I
> would be interested in knowing whether there are updated versions of
> the implementations mentioned in the meeting report above, and/or a
> merge of these work? Also, if someone is actively working on this at
> the moment, we will definitely want to sync on IRC or anywhere else.

Laurent has been the last one working on this, but his code doesn't have
the control handling :-(

My latest (well, December 2015) tree with the control request code
is here:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=requests2

It's AFAIK a slightly newer version from what ChromeOS uses.

> Excited to work with you all! :)

Looking forward to your code!

Regards,

Hans

Re: [PATCH v6 3/7] media: i2c: max2175: Add MAX2175 support

Em Thu, 8 Jun 2017 09:42:43 +
Ramesh Shanmugasundaram  escreveu:

> > Subject: Re: [PATCH v6 3/7] media: i2c: max2175: Add MAX2175 support
> > 
> > Em Wed, 31 May 2017 09:44:53 +0100
> > Ramesh Shanmugasundaram  escreveu:
> >   
> > > +++ b/Documentation/media/v4l-drivers/max2175.rst
> > > @@ -0,0 +1,60 @@
> > > +Maxim Integrated MAX2175 RF to bits tuner driver
> > > +
> > > +
> > > +The MAX2175 driver implements the following driver-specific controls:
> > > +
> > > +``V4L2_CID_MAX2175_I2S_ENABLE``
> > > +---
> > > +Enable/Disable I2S output of the tuner.
> > > +
> > > +.. flat-table::
> > > +:header-rows:  0
> > > +:stub-columns: 0
> > > +:widths:   1 4
> > > +
> > > +* - ``(0)``
> > > +  - I2S output is disabled.
> > > +* - ``(1)``
> > > +  - I2S output is enabled.  
> > 
> > Hmm... There are other drivers at the subsystem that use I2S (for audio -
> > not for SDR - but I guess the issue is similar).
> > 
> > On such drivers, the bridge driver controls it directly, being sure that
> > I2S is enabled when it is expecting some data coming from the I2S bus.
> > 
> > On some drivers, there are both I2S and A/D inputs at the bridge chipset.
> > On such drivers, enabling/disabling I2S is done via VIDIOC_S_INPUT (and
> > optionally via ALSA mixer), being transparent to the user if the stream
> > comes from a tuner via I2S or from a directly connected A/D input.
> > 
> > I don't think it is a good idea to enable it via a control, as, if the
> > bridge driver is expecting data via I2S, disabling it will cause timeouts
> > at the videobuf handling.  
> 
> The MAX2175 device is exposed as a v4l2 subdev with tuner ops and can 
> interface with an SDR device. When the tuner is configured, the I2S output is 
> enabled by default. From an independent tuner device perspective, this 
> default behaviour is enough and this control may not be needed/used.
> 
> However, for the use case here, the R-Car DRIF device acts as the main SDR 
> device and the Maxim MAX2175 provides a sub-dev interface with tuner ops.
> 
> +-++-+
> | |-SCK--->|CLK  |   
> |   Master|-SS>|SYNC  DRIFn (slave)  |
> |  (MAX2175)  |-SD0--->|D0   |   
> | |-SD1--->|D1   |   
> +-++-+
> 
> The DRIF device design is such that it involves separate register writes to 
> enable Rx on each of the data line. To keep both the data lines in sync it 
> expects the master device to enable output after both the data line Rx are 
> enabled.
> 
> This level of control is exposed as a feature in the MAX2175 using this 
> control. When interfaced with DRIF this control is used to achieve the 
> desired functionality. When not interfaced with DRIF, the MAX2175 default 
> behaviour does not have to change because of DRIF and hence this I2S control 
> may be unused. Like MAX2175, DRIF is also an independent device and can 
> interface with a different third party tuner. 
> 
> Hence, this I2S enable/disable is exposed as a user control. The end user 
> application (knowing both these devices) is expected to use these controls 
> appropriately. Please let me know if I need to explain anything in further 
> detail.


The usecase is clear. That's exactly what other drivers with I2S do,
except that, on those other drivers, they pass I2S control info via
platform_data (they're not platform drivers).

With those drivers, generic applications work as-is via the standard
video, radio or sdr devnodes, without knowing about I2S.

The main difference here is that you're requiring an specialized
application for this device to work, as a generic one won't be
aware of this device-specific control, and may end by exposing this
"internal" control to the end user. That is OK for embedded usage,
but, as soon as this is used on some non-embedded usecase (with
is likely, as there are several PC consumer products using other
chips from Maxim), we'll have problems.

I guess the solution here is to make such control visible only via the
subdev interface.

Thanks,
Mauro

[PATCH 2/3] media: ngene: Replace semaphore stream_mutex with mutex

The semaphore 'stream_mutex' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---
 drivers/media/pci/ngene/ngene-core.c | 14 +++---
 drivers/media/pci/ngene/ngene.h  |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index dfbd1e0..59f2e5f 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -560,7 +560,7 @@ static int ngene_command_stream_control(struct ngene *dev, 
u8 stream,
u16 BsSPI = ((stream & 1) ? 0x9800 : 0x9700);
u16 BsSDO = 0x9B00;
 
-   down(>stream_mutex);
+   mutex_lock(>stream_mutex);
memset(, 0, sizeof(com));
com.cmd.hdr.Opcode = CMD_CONTROL;
com.cmd.hdr.Length = sizeof(struct FW_STREAM_CONTROL) - 2;
@@ -587,16 +587,16 @@ static int ngene_command_stream_control(struct ngene 
*dev, u8 stream,
chan->HWState = HWSTATE_STOP;
spin_unlock_irq(>state_lock);
if (ngene_command(dev, ) < 0) {
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return -1;
}
/* clear_buffers(chan); */
flush_buffers(chan);
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return 0;
}
spin_unlock_irq(>state_lock);
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return 0;
}
 
@@ -693,10 +693,10 @@ static int ngene_command_stream_control(struct ngene 
*dev, u8 stream,
spin_unlock_irq(>state_lock);
 
if (ngene_command(dev, ) < 0) {
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return -1;
}
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return 0;
 }
 
@@ -1347,7 +1347,7 @@ static int ngene_start(struct ngene *dev)
init_waitqueue_head(>tx_wq);
init_waitqueue_head(>rx_wq);
mutex_init(>cmd_mutex);
-   sema_init(>stream_mutex, 1);
+   mutex_init(>stream_mutex);
sema_init(>pll_mutex, 1);
sema_init(>i2c_switch_mutex, 1);
spin_lock_init(>cmd_lock);
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index e600b70..0dd15d6 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -763,7 +763,7 @@ struct ngene {
wait_queue_head_t cmd_wq;
int   cmd_done;
struct mutex  cmd_mutex;
-   struct semaphore  stream_mutex;
+   struct mutex  stream_mutex;
struct semaphore  pll_mutex;
struct semaphore  i2c_switch_mutex;
int   i2c_current_channel;
-- 
Binoy Jayan

[PATCH 1/3] media: ngene: Replace semaphore cmd_mutex with mutex

The semaphore 'cmd_mutex' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---
 drivers/media/pci/ngene/ngene-core.c | 12 ++--
 drivers/media/pci/ngene/ngene.h  |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index ce69e64..dfbd1e0 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -336,9 +336,9 @@ int ngene_command(struct ngene *dev, struct ngene_command 
*com)
 {
int result;
 
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
result = ngene_command_mutex(dev, com);
-   up(>cmd_mutex);
+   mutex_unlock(>cmd_mutex);
return result;
 }
 
@@ -1283,7 +1283,7 @@ static int ngene_load_firm(struct ngene *dev)
 
 static void ngene_stop(struct ngene *dev)
 {
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
i2c_del_adapter(&(dev->channel[0].i2c_adapter));
i2c_del_adapter(&(dev->channel[1].i2c_adapter));
ngwritel(0, NGENE_INT_ENABLE);
@@ -1346,7 +1346,7 @@ static int ngene_start(struct ngene *dev)
init_waitqueue_head(>cmd_wq);
init_waitqueue_head(>tx_wq);
init_waitqueue_head(>rx_wq);
-   sema_init(>cmd_mutex, 1);
+   mutex_init(>cmd_mutex);
sema_init(>stream_mutex, 1);
sema_init(>pll_mutex, 1);
sema_init(>i2c_switch_mutex, 1);
@@ -1606,10 +1606,10 @@ static void ngene_unlink(struct ngene *dev)
com.in_len = 3;
com.out_len = 1;
 
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
ngwritel(0, NGENE_INT_ENABLE);
ngene_command_mutex(dev, );
-   up(>cmd_mutex);
+   mutex_unlock(>cmd_mutex);
 }
 
 void ngene_shutdown(struct pci_dev *pdev)
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index 10d8f74..e600b70 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -762,7 +762,7 @@ struct ngene {
 
wait_queue_head_t cmd_wq;
int   cmd_done;
-   struct semaphore  cmd_mutex;
+   struct mutex  cmd_mutex;
struct semaphore  stream_mutex;
struct semaphore  pll_mutex;
struct semaphore  i2c_switch_mutex;
-- 
Binoy Jayan

[PATCH 3/3] media: ngene: Replace semaphore i2c_switch_mutex with mutex

The semaphore 'i2c_switch_mutex' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---
 drivers/media/pci/ngene/ngene-core.c | 2 +-
 drivers/media/pci/ngene/ngene-i2c.c  | 6 +++---
 drivers/media/pci/ngene/ngene.h  | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index 59f2e5f..ca0c0f8 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -1349,7 +1349,7 @@ static int ngene_start(struct ngene *dev)
mutex_init(>cmd_mutex);
mutex_init(>stream_mutex);
sema_init(>pll_mutex, 1);
-   sema_init(>i2c_switch_mutex, 1);
+   mutex_init(>i2c_switch_mutex);
spin_lock_init(>cmd_lock);
for (i = 0; i < MAX_STREAM; i++)
spin_lock_init(>channel[i].state_lock);
diff --git a/drivers/media/pci/ngene/ngene-i2c.c 
b/drivers/media/pci/ngene/ngene-i2c.c
index cf39fcf..fbf3635 100644
--- a/drivers/media/pci/ngene/ngene-i2c.c
+++ b/drivers/media/pci/ngene/ngene-i2c.c
@@ -118,7 +118,7 @@ static int ngene_i2c_master_xfer(struct i2c_adapter 
*adapter,
(struct ngene_channel *)i2c_get_adapdata(adapter);
struct ngene *dev = chan->dev;
 
-   down(>i2c_switch_mutex);
+   mutex_lock(>i2c_switch_mutex);
ngene_i2c_set_bus(dev, chan->number);
 
if (num == 2 && msg[1].flags & I2C_M_RD && !(msg[0].flags & I2C_M_RD))
@@ -136,11 +136,11 @@ static int ngene_i2c_master_xfer(struct i2c_adapter 
*adapter,
msg[0].buf, msg[0].len, 0))
goto done;
 
-   up(>i2c_switch_mutex);
+   mutex_unlock(>i2c_switch_mutex);
return -EIO;
 
 done:
-   up(>i2c_switch_mutex);
+   mutex_unlock(>i2c_switch_mutex);
return num;
 }
 
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index 0dd15d6..7c7cd21 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -765,7 +765,7 @@ struct ngene {
struct mutex  cmd_mutex;
struct mutex  stream_mutex;
struct semaphore  pll_mutex;
-   struct semaphore  i2c_switch_mutex;
+   struct mutex  i2c_switch_mutex;
int   i2c_current_channel;
int   i2c_current_bus;
spinlock_tcmd_lock;
-- 
Binoy Jayan

[PATCH 0/3] ngene: Replace semaphores with mutexes

These are a set of patches which removes semaphores from ngene.
These are part of a bigger effort to eliminate unwanted semaphores
from the linux kernel.

Binoy Jayan (3):
  media: ngene: Replace semaphore cmd_mutex with mutex
  media: ngene: Replace semaphore stream_mutex with mutex
  media: ngene: Replace semaphore i2c_switch_mutex with mutex

 drivers/media/pci/ngene/ngene-core.c | 28 ++--
 drivers/media/pci/ngene/ngene-i2c.c  |  6 +++---
 drivers/media/pci/ngene/ngene.h  |  6 +++---
 3 files changed, 20 insertions(+), 20 deletions(-)

-- 
Binoy Jayan

Re: [RFC] V4L2 unified low-level decoder API

2017-06-08 Thread Alexandre Courbot

On Thu, Jun 8, 2017 at 5:56 PM, Pawel Osciak  wrote:
> Hi,
>
> On Fri, May 19, 2017 at 1:08 AM, Hugues FRUCHET  wrote:
>> Before merging this work Hans would like to have feedback from peers, in
>> order to be sure that this is inline with other SoC vendors drivers
>> expectations.
>>
>> Thomasz, Pawel, could you give your view regarding ChromeOS and Rockchip
>> driver ?
>
> The drivers for Rockchip codecs are submitted to the public Chromium OS kernel
> and working on our RK-based platforms. We have also since added a VP9 API as
> well, which is also working on devices that support it. This gives us
> a set of H.264,
> VP8 and VP9 APIs on both kernel and userspace side (in the open source
> Chromium browser) that are working currently and can be used for
> further testing.
>
> We are interested in merging the API patches as well as these drivers upstream
> (they were posted on this list two years ago), however we've been blocked by 
> the
> progress of request API, which is required for this. Alexandre Courbot
> is currently
> looking into creating a minimal version of the request API that would provide
> enough functionality for stateless codecs, and also plans to further work on
> re-submitting the particular codec API patches, once the request API
> is finalized.

Hi everyone,

It is a bit scary to start hacking on V4L with something as disruptive
as the request API, so please bear with me as I will inevitably post
code that will go from cutely clueless to downright offensive.

Thankfully Pawel is not too far from my desk, and we (Pawel, Tomasz
and myself) had a very fruitful in-person brainstorming session last
week with Laurent, so this should limit the damage.

In any case, I think that everybody agrees that this needs to be
pushed forward. Chromium OS in particular has a big interest to see
this feature landing upstream, so I will dedicate some cycles to this.

>From reading the meetings reports (e.g.
https://www.spinics.net/lists/linux-media/msg106699.html) I understand
that we want to support several use-cases with this and we already
have several proposals with code. Chromium in a first time is
interested in stateless codecs support, and this use-case also seems
to be the simplest to implement, so we would like to start pushing in
that direction first. This should give us a reasonably sized code base
to rely upon and implement the other use-cases as we see clearer
through this.

I still need to study a bit the existing proposals and to clearly lay
out the conclusions of our meeting with Laurent, but the general idea
has not changed too much, except maybe that we thought it may be nice
to make state management less explicit to userspace by default. I
would be interested in knowing whether there are updated versions of
the implementations mentioned in the meeting report above, and/or a
merge of these work? Also, if someone is actively working on this at
the moment, we will definitely want to sync on IRC or anywhere else.

Excited to work with you all! :)

Cheers,
Alex.

RE: [PATCH v6 3/7] media: i2c: max2175: Add MAX2175 support

2017-06-08 Thread Ramesh Shanmugasundaram

> Subject: Re: [PATCH v6 3/7] media: i2c: max2175: Add MAX2175 support
> 
> Em Wed, 31 May 2017 09:44:53 +0100
> Ramesh Shanmugasundaram  escreveu:
> 
> > +++ b/Documentation/media/v4l-drivers/max2175.rst
> > @@ -0,0 +1,60 @@
> > +Maxim Integrated MAX2175 RF to bits tuner driver
> > +
> > +
> > +The MAX2175 driver implements the following driver-specific controls:
> > +
> > +``V4L2_CID_MAX2175_I2S_ENABLE``
> > +---
> > +Enable/Disable I2S output of the tuner.
> > +
> > +.. flat-table::
> > +:header-rows:  0
> > +:stub-columns: 0
> > +:widths:   1 4
> > +
> > +* - ``(0)``
> > +  - I2S output is disabled.
> > +* - ``(1)``
> > +  - I2S output is enabled.
> 
> Hmm... There are other drivers at the subsystem that use I2S (for audio -
> not for SDR - but I guess the issue is similar).
> 
> On such drivers, the bridge driver controls it directly, being sure that
> I2S is enabled when it is expecting some data coming from the I2S bus.
> 
> On some drivers, there are both I2S and A/D inputs at the bridge chipset.
> On such drivers, enabling/disabling I2S is done via VIDIOC_S_INPUT (and
> optionally via ALSA mixer), being transparent to the user if the stream
> comes from a tuner via I2S or from a directly connected A/D input.
> 
> I don't think it is a good idea to enable it via a control, as, if the
> bridge driver is expecting data via I2S, disabling it will cause timeouts
> at the videobuf handling.

The MAX2175 device is exposed as a v4l2 subdev with tuner ops and can interface 
with an SDR device. When the tuner is configured, the I2S output is enabled by 
default. From an independent tuner device perspective, this default behaviour 
is enough and this control may not be needed/used.

However, for the use case here, the R-Car DRIF device acts as the main SDR 
device and the Maxim MAX2175 provides a sub-dev interface with tuner ops.

+-++-+
| |-SCK--->|CLK  |   
|   Master|-SS>|SYNC  DRIFn (slave)  |
|  (MAX2175)  |-SD0--->|D0   |   
| |-SD1--->|D1   |   
+-++-+

The DRIF device design is such that it involves separate register writes to 
enable Rx on each of the data line. To keep both the data lines in sync it 
expects the master device to enable output after both the data line Rx are 
enabled.

This level of control is exposed as a feature in the MAX2175 using this 
control. When interfaced with DRIF this control is used to achieve the desired 
functionality. When not interfaced with DRIF, the MAX2175 default behaviour 
does not have to change because of DRIF and hence this I2S control may be 
unused. Like MAX2175, DRIF is also an independent device and can interface with 
a different third party tuner. 

Hence, this I2S enable/disable is exposed as a user control. The end user 
application (knowing both these devices) is expected to use these controls 
appropriately. Please let me know if I need to explain anything in further 
detail.

Thanks,
Ramesh

[PATCH] [media] coda: ctx->codec is not NULL in coda_alloc_framebuffers

This fixes a smatch warning:

drivers/media/platform/coda/coda-bit.c:415 coda_alloc_framebuffers()
error: we previously assumed 'ctx->codec' could be null (see line 396)

coda_alloc_framebuffers() is called from coda_start_encoding() and
__coda_start_decoding(). Both dereference ctx->codec before calling
coda_alloc_framebuffers() in lines 935 and 1649, so ctx->codec can not
be NULL.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda/coda-bit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c 
b/drivers/media/platform/coda/coda-bit.c
index 795b6d7584320..bba1eb43b5d83 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -393,8 +393,8 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
int ret;
int i;
 
-   if (ctx->codec && (ctx->codec->src_fourcc == V4L2_PIX_FMT_H264 ||
-ctx->codec->dst_fourcc == V4L2_PIX_FMT_H264)) {
+   if (ctx->codec->src_fourcc == V4L2_PIX_FMT_H264 ||
+   ctx->codec->dst_fourcc == V4L2_PIX_FMT_H264) {
width = round_up(q_data->width, 16);
height = round_up(q_data->height, 16);
} else {
-- 
2.11.0

Re: [PATCH 8/8] omapdrm: hdmi4: hook up the HDMI CEC support

2017-06-08 Thread Tomi Valkeinen

On 08/06/17 10:34, Hans Verkuil wrote:

>> Peter is about to send hotplug-interrupt-handling series, I think the
>> HPD function work should be done on top of that, as otherwise it'll just
>> conflict horribly.
> 
> Has that been merged yet? And if so, what git repo/branch should I base
> my next version of this patch series on? If not, do you know when it is
> expected?

No, still pending review. The patches ("[PATCH v2 0/3] drm/omap: Support
for hotplug detection") apply on top of latest drm-next, if you want to try.

 Tomi



signature.asc
Description: OpenPGP digital signature

[PATCH] Revert "[media] et8ek8: Export OF device ID as module aliases"

This one got applied twice, causing a build error with clang:

drivers/media/i2c/et8ek8/et8ek8_driver.c:1499:1: error: redefinition of 
'__mod_of__et8ek8_of_table_device_table'

Fixes: 9ae05fd1e791 ("[media] et8ek8: Export OF device ID as module aliases")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/i2c/et8ek8/et8ek8_driver.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c 
b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index 6e313d5243a0..f39f5179dd95 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -1496,7 +1496,6 @@ MODULE_DEVICE_TABLE(i2c, et8ek8_id_table);
 static const struct dev_pm_ops et8ek8_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(et8ek8_suspend, et8ek8_resume)
 };
-MODULE_DEVICE_TABLE(of, et8ek8_of_table);
 
 static struct i2c_driver et8ek8_i2c_driver = {
.driver = {
-- 
2.9.0

Re: [RFC] V4L2 unified low-level decoder API

2017-06-08 Thread Pawel Osciak

Hi,

On Fri, May 19, 2017 at 1:08 AM, Hugues FRUCHET  wrote:
> Before merging this work Hans would like to have feedback from peers, in
> order to be sure that this is inline with other SoC vendors drivers
> expectations.
>
> Thomasz, Pawel, could you give your view regarding ChromeOS and Rockchip
> driver ?

The drivers for Rockchip codecs are submitted to the public Chromium OS kernel
and working on our RK-based platforms. We have also since added a VP9 API as
well, which is also working on devices that support it. This gives us
a set of H.264,
VP8 and VP9 APIs on both kernel and userspace side (in the open source
Chromium browser) that are working currently and can be used for
further testing.

We are interested in merging the API patches as well as these drivers upstream
(they were posted on this list two years ago), however we've been blocked by the
progress of request API, which is required for this. Alexandre Courbot
is currently
looking into creating a minimal version of the request API that would provide
enough functionality for stateless codecs, and also plans to further work on
re-submitting the particular codec API patches, once the request API
is finalized.

Thanks,
Pawel

[PATCH v2 1/3] [media] coda: use correct offset for mvcol buffer

From: Lucas Stach 

The mvcol buffer needs to be placed behind the chroma plane(s), so
use the real offset including any required rounding.

Signed-off-by: Lucas Stach 
Reviewed-by: Philipp Zabel 
Signed-off-by: Philipp Zabel 
---
No changes since v1 [1].

[1] https://patchwork.linuxtv.org/patch/40604
---
 drivers/media/platform/coda/coda-bit.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c 
b/drivers/media/platform/coda/coda-bit.c
index 2ec41375a896f..325035bb0a777 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -427,14 +427,16 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
 
/* Register frame buffers in the parameter buffer */
for (i = 0; i < ctx->num_internal_frames; i++) {
-   u32 y, cb, cr;
+   u32 y, cb, cr, mvcol;
 
/* Start addresses of Y, Cb, Cr planes */
y = ctx->internal_frames[i].paddr;
cb = y + ysize;
cr = y + ysize + ysize/4;
+   mvcol = y + ysize + ysize/4 + ysize/4;
if (ctx->tiled_map_type == GDI_TILED_FRAME_MB_RASTER_MAP) {
cb = round_up(cb, 4096);
+   mvcol = cb + ysize/2;
cr = 0;
/* Packed 20-bit MSB of base addresses */
/* YCCC, CCyc,  */
@@ -448,9 +450,7 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
/* mvcol buffer for h.264 */
if (ctx->codec->src_fourcc == V4L2_PIX_FMT_H264 &&
dev->devtype->product != CODA_DX6)
-   coda_parabuf_write(ctx, 96 + i,
-  ctx->internal_frames[i].paddr +
-  ysize + ysize/4 + ysize/4);
+   coda_parabuf_write(ctx, 96 + i, mvcol);
}
 
/* mvcol buffer for mpeg4 */
-- 
2.11.0

[PATCH v2 2/3] [media] coda: first step at error recovery

From: Lucas Stach 

This implements a simple handler for the case where decode did not finish
successfully. This might be helpful during normal streaming, but for now it
only handles the case where the context would deadlock with userspace,
i.e. userspace issued DEC_CMD_STOP and waits for EOS, but after the failed
decode run we would hold the context and wait for userspace to queue more
buffers.

Signed-off-by: Lucas Stach 
Reviewed-by: Philipp Zabel 
[p.za...@pengutronix.de: renamed error_decode/run to run/decode_timeout]
Signed-off-by: Philipp Zabel 
---
Changes since v1 [1]:
- Rename error_run/decode callback to run/decode_timeout, as
  this error handler is called on device_run timeouts only.

[1] https://patchwork.linuxtv.org/patch/40603
---
 drivers/media/platform/coda/coda-bit.c| 20 
 drivers/media/platform/coda/coda-common.c |  3 +++
 drivers/media/platform/coda/coda.h|  1 +
 3 files changed, 24 insertions(+)

diff --git a/drivers/media/platform/coda/coda-bit.c 
b/drivers/media/platform/coda/coda-bit.c
index 325035bb0a777..795b6d7584320 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -2198,12 +2198,32 @@ static void coda_finish_decode(struct coda_ctx *ctx)
ctx->display_idx = display_idx;
 }
 
+static void coda_decode_timeout(struct coda_ctx *ctx)
+{
+   struct vb2_v4l2_buffer *dst_buf;
+
+   /*
+* For now this only handles the case where we would deadlock with
+* userspace, i.e. userspace issued DEC_CMD_STOP and waits for EOS,
+* but after a failed decode run we would hold the context and wait for
+* userspace to queue more buffers.
+*/
+   if (!(ctx->bit_stream_param & CODA_BIT_STREAM_END_FLAG))
+   return;
+
+   dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
+   dst_buf->sequence = ctx->qsequence - 1;
+
+   coda_m2m_buf_done(ctx, dst_buf, VB2_BUF_STATE_ERROR);
+}
+
 const struct coda_context_ops coda_bit_decode_ops = {
.queue_init = coda_decoder_queue_init,
.reqbufs = coda_decoder_reqbufs,
.start_streaming = coda_start_decoding,
.prepare_run = coda_prepare_decode,
.finish_run = coda_finish_decode,
+   .run_timeout = coda_decode_timeout,
.seq_end_work = coda_seq_end_work,
.release = coda_bit_release,
 };
diff --git a/drivers/media/platform/coda/coda-common.c 
b/drivers/media/platform/coda/coda-common.c
index 78bd9a4ace0e4..829c7895a98a2 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1163,6 +1163,9 @@ static void coda_pic_run_work(struct work_struct *work)
ctx->hold = true;
 
coda_hw_reset(ctx);
+
+   if (ctx->ops->run_timeout)
+   ctx->ops->run_timeout(ctx);
} else if (!ctx->aborting) {
ctx->ops->finish_run(ctx);
}
diff --git a/drivers/media/platform/coda/coda.h 
b/drivers/media/platform/coda/coda.h
index 76d059431ca13..c5f504d8cf67f 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -183,6 +183,7 @@ struct coda_context_ops {
int (*start_streaming)(struct coda_ctx *ctx);
int (*prepare_run)(struct coda_ctx *ctx);
void (*finish_run)(struct coda_ctx *ctx);
+   void (*run_timeout)(struct coda_ctx *ctx);
void (*seq_end_work)(struct work_struct *work);
void (*release)(struct coda_ctx *ctx);
 };
-- 
2.11.0

[PATCH v2 3/3] [media] coda/imx-vdoa: always wait for job completion

From: Lucas Stach 

As long as only one CODA context is running we get alternating
device_run() and wait_for_completion() calls, but when more then one
CODA context is active, other VDOA slots can be inserted between those
calls for one context.

Make sure to wait on job completion before running a different context
and before destroying the currently active context.

Signed-off-by: Lucas Stach 
Reviewed-by: Philipp Zabel 
Signed-off-by: Philipp Zabel 
---
No changes since v1 [1].

[1] https://patchwork.linuxtv.org/patch/40605
---
 drivers/media/platform/coda/imx-vdoa.c | 49 +++---
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/coda/imx-vdoa.c 
b/drivers/media/platform/coda/imx-vdoa.c
index 669a4c82f1ffa..df9b71621420c 100644
--- a/drivers/media/platform/coda/imx-vdoa.c
+++ b/drivers/media/platform/coda/imx-vdoa.c
@@ -101,6 +101,8 @@ struct vdoa_ctx {
struct vdoa_data*vdoa;
struct completion   completion;
struct vdoa_q_data  q_data[2];
+   unsigned intsubmitted_job;
+   unsigned intcompleted_job;
 };
 
 static irqreturn_t vdoa_irq_handler(int irq, void *data)
@@ -114,7 +116,7 @@ static irqreturn_t vdoa_irq_handler(int irq, void *data)
 
curr_ctx = vdoa->curr_ctx;
if (!curr_ctx) {
-   dev_dbg(vdoa->dev,
+   dev_warn(vdoa->dev,
"Instance released before the end of transaction\n");
return IRQ_HANDLED;
}
@@ -127,19 +129,44 @@ static irqreturn_t vdoa_irq_handler(int irq, void *data)
} else if (!(val & VDOAIST_EOT)) {
dev_warn(vdoa->dev, "Spurious interrupt\n");
}
+   curr_ctx->completed_job++;
complete(_ctx->completion);
 
return IRQ_HANDLED;
 }
 
+int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
+{
+   struct vdoa_data *vdoa = ctx->vdoa;
+
+   if (ctx->submitted_job == ctx->completed_job)
+   return 0;
+
+   if (!wait_for_completion_timeout(>completion,
+msecs_to_jiffies(300))) {
+   dev_err(vdoa->dev,
+   "Timeout waiting for transfer result\n");
+   return -ETIMEDOUT;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(vdoa_wait_for_completion);
+
 void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst, dma_addr_t src)
 {
struct vdoa_q_data *src_q_data, *dst_q_data;
struct vdoa_data *vdoa = ctx->vdoa;
u32 val;
 
+   if (vdoa->curr_ctx)
+   vdoa_wait_for_completion(vdoa->curr_ctx);
+
vdoa->curr_ctx = ctx;
 
+   reinit_completion(>completion);
+   ctx->submitted_job++;
+
src_q_data = >q_data[V4L2_M2M_SRC];
dst_q_data = >q_data[V4L2_M2M_DST];
 
@@ -177,21 +204,6 @@ void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst, 
dma_addr_t src)
 }
 EXPORT_SYMBOL(vdoa_device_run);
 
-int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
-{
-   struct vdoa_data *vdoa = ctx->vdoa;
-
-   if (!wait_for_completion_timeout(>completion,
-msecs_to_jiffies(300))) {
-   dev_err(vdoa->dev,
-   "Timeout waiting for transfer result\n");
-   return -ETIMEDOUT;
-   }
-
-   return 0;
-}
-EXPORT_SYMBOL(vdoa_wait_for_completion);
-
 struct vdoa_ctx *vdoa_context_create(struct vdoa_data *vdoa)
 {
struct vdoa_ctx *ctx;
@@ -218,6 +230,11 @@ void vdoa_context_destroy(struct vdoa_ctx *ctx)
 {
struct vdoa_data *vdoa = ctx->vdoa;
 
+   if (vdoa->curr_ctx == ctx) {
+   vdoa_wait_for_completion(vdoa->curr_ctx);
+   vdoa->curr_ctx = NULL;
+   }
+
clk_disable_unprepare(vdoa->vdoa_clk);
kfree(ctx);
 }
-- 
2.11.0

Re: [PATCH 0/7] Add block read/write to en50221 CAM functions

Em Thu, 8 Jun 2017 09:31:25 +0200
"Jasmin J."  escreveu:

> Hello Mauro!
> 
>  > It should be, instead:
>  >From: Ralph Metzler   
> I thought it is enough to write him in the Signed-off-by as first.

No, it isn't. It is possible to have a patch authored by someone that
didn't sign. We don't usually accept it at the Kernel (on exceptional
cases, we might end accepting patches without SOB), but other projects
that use git may not require SOB.

> This From line is automatically generated by Git with Format Patch
> and then used by send. 

Git should take it from the author's name at your git tree. You
can change it with git filter-branch. Something like:

git filter-branch -f --commit-filter '
if [ "$GIT_COMMITTER_NAME" = "My Name" ];
then
GIT_AUTHOR_NAME="Other Name";
GIT_AUTHOR_EMAIL="other@email";
GIT_COMMITTER_EMAIL="my@email";
GIT_AUTHOR_DATE=$GIT_COMMITTER_DATE;
git commit-tree "$@";
else
git commit-tree "$@";
fi' tag1..tag2

Btw, you can also use filter-branch to rename things at the code, e. g.:

git filter-branch -f --tree-filter 'for i in $(git grep -l 
MEDIA_ENT_T_AV_BRIDGE); do sed s,MEDIA_ENT_T_AV_BRIDGE,MEDIA_ENT_T_AV_DMA,g -i 
$i; done ' tag2..tag3

(tag1, tag2, tag3 can actually be a changeset hash, a branch and/or a tag)

> However I should change that and my mail program
> still accepting it?
> I will try.
> 
> BR,
> Jasmin



Thanks,
Mauro

Re: [PATCH 05/12] intel-ipu3: css: tables

Hi Yong,

On Tue, Jun 6, 2017 at 5:39 AM, Yong Zhi  wrote:
> Coeff, config parameters etc const definitions for
> IPU3 programming.
>
> Signed-off-by: Yong Zhi 
> ---
>  drivers/media/pci/intel/ipu3/ipu3-tables.c | 9621 
> 
>  drivers/media/pci/intel/ipu3/ipu3-tables.h |   82 +
>  2 files changed, 9703 insertions(+)
>  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-tables.c
>  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-tables.h

I wonder if this patch really reached the mailing list. It seems to
not be present on patchwork.linuxtv.org. Possibly due to size
restrictions.

Best regards,
Tomasz

Re: [PATCH 02/12] intel-ipu3: mmu: implement driver

Hi Sakari,

On Thu, Jun 8, 2017 at 6:59 AM, Sakari Ailus
 wrote:
> Hi Tomasz,
>
> On Tue, Jun 06, 2017 at 07:13:19PM +0900, Tomasz Figa wrote:
>> Hi Yong, Tuukka,
>>
>> +CC IOMMU ML and Joerg. (Technically you should resend this patch
>> including them.)
>
> Thanks!
>
>>
>> On Tue, Jun 6, 2017 at 5:39 AM, Yong Zhi  wrote:
> ...
>> > diff --git a/drivers/media/pci/intel/ipu3/Kconfig 
>> > b/drivers/media/pci/intel/ipu3/Kconfig
>> > index 2a895d6..ab2edcb 100644
>> > --- a/drivers/media/pci/intel/ipu3/Kconfig
>> > +++ b/drivers/media/pci/intel/ipu3/Kconfig
>> > @@ -15,3 +15,14 @@ config VIDEO_IPU3_CIO2
>> > Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
>> > connected camera.
>> > The module will be called ipu3-cio2.
>> > +
>> > +config INTEL_IPU3_MMU
>> > +   tristate "Intel ipu3-mmu driver"
>> > +   select IOMMU_API
>> > +   select IOMMU_IOVA
>> > +   ---help---
>> > + For IPU3, this option enables its MMU driver to translate its 
>> > internal
>> > + virtual address to 39 bits wide physical address for 64GBytes 
>> > space access.
>> > +
>> > + Say Y here if you have Skylake/Kaby Lake SoC with IPU3.
>> > + Say N if un-sure.
>>
>> Is the MMU optional? I.e. can you still use the IPU3 without the MMU
>> driver? If no, then it doesn't make sense to flood the user with
>> meaningless choice and the driver could simply be selected by other
>> IPU3 drivers.
>
> There are other IPUs that contain the same hardware, so they would
> presumably use the same driver.

My question was slightly different. I was wondering if one can use the
IPU3 without the MMU, i.e. if the implication "if IPU3 then IPU3_MMU"
exists.

>
>>
>> And the other way around, is the IPU3 MMU driver useful for anything
>> else than IPU3? If no (but yes for the above), then it should depend
>> on some other IPU3 drivers being enabled, as otherwise it would just
>> confuse the user.
>
> Very likely not.
>
> For now I think it'd be fine to have the driver separate from the rest of
> the IPU3 but without a separate Kconfig option.

Yeah, I'm not questioning the need for this to be a separate driver.
:) I just want to avoid adding Kconfig option, in case there is no
practical choice (i.e. IPU3 requires IPU3_MMU and IPU3_MMU is useful
without IPU3).

>
>>
>> > diff --git a/drivers/media/pci/intel/ipu3/Makefile 
>> > b/drivers/media/pci/intel/ipu3/Makefile
>> > index 20186e3..2b669df 100644
>> > --- a/drivers/media/pci/intel/ipu3/Makefile
>> > +++ b/drivers/media/pci/intel/ipu3/Makefile
>> > @@ -1 +1,2 @@
>> >  obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
>> > +obj-$(CONFIG_INTEL_IPU3_MMU) += ipu3-mmu.o
>> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-mmu.c 
>> > b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
>> > new file mode 100644
>> > index 000..a9fb116
>> > --- /dev/null
>> > +++ b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
>
> ...
>
>> > +/**
>> > + * ipu3_mmu_alloc_page_table - get page to fill entries with dummy 
>> > defaults
>> > + * @d: mapping domain to be worked on
>> > + * @l1: True for L1 page table, false for L2 page table.
>> > + *
>> > + * Index of L1 page table points to L2 tbl
>> > + *
>> > + * Return: Pointer to allocated page table
>> > + * or NULL on failure.
>> > + */
>> > +static uint32_t *ipu3_mmu_alloc_page_table(struct ipu3_mmu_domain *d, 
>> > bool l1)
>> > +{
>> > +   uint32_t *pt = (uint32_t *)__get_free_page(GFP_KERNEL);
>>
>> Style: I believe u32 is preferred in the kernel.
>
> There are some 3 users of uint32_t alone in the kernel. I'd say it
> should be fine. (I'm not trying saying it'd be more common than u32
> though.)

Okay, checked the CodingStyle again and they are generally okay,
except userspace headers, where __u* ones should be used.

>
>> > +   DMA_BIT_MASK(IPU3_MMU_ADDRESS_BITS);
>> > +   mmu_dom->domain.geometry.force_aperture = true;
>> > +
>> > +   ptr = (void *)__get_free_page(GFP_KERNEL);
>> > +   if (!ptr)
>> > +   goto fail_get_page;
>> > +   mmu_dom->dummy_page = virt_to_phys(ptr) >> IPU3_MMU_PAGE_SHIFT;
>>
>> Is virt_to_phys() correct here? I'm not an expert on x86 systems, but
>> since this is a PCI device, there might be some other memory mapping
>> involved.
>
> In theory yes --- if the IPU3 were behind an IOMMU managed by the Linux
> kernel.

Doesn't the VT-d IOMMU actually allow such configuration? (Disclaimer:
I don't know too much about x86 and am reasoning based on few high
level hardware overviews.)

> That kind of configuration wouldn't make much sense

It would make sense from security perspective (the main system IOMMU
is likely more trusted that one of some peripheral device).

It would also make sense from stability perspective. Given that the
IPU3 PCI device seems to be non-coherent, the system IOMMU (which I
believe is coherent) would catch any kinds of device memory reads or
writes due to some

Re: [PATCH 0/7] Add block read/write to en50221 CAM functions

2017-06-08 Thread Jasmin J.


Hello Mauro!

> It should be, instead:
>From: Ralph Metzler 
I thought it is enough to write him in the Signed-off-by as first.
This From line is automatically generated by Git with Format Patch
and then used by send. However I should change that and my mail program
still accepting it?
I will try.

BR,
   Jasmin

Re: [PATCH 8/8] omapdrm: hdmi4: hook up the HDMI CEC support

Hi Tomi,

On 08/05/17 12:26, Tomi Valkeinen wrote:
> On 06/05/17 14:58, Hans Verkuil wrote:
> 
>> My assumption was that hdmi_display_disable() was called when the hotplug 
>> would go
>> away. But I discovered that that isn't the case, or at least not when X is 
>> running.
>> It seems that the actual HPD check is done in hdmic_detect() in
>> omapdrm/displays/connector-hdmi.c.
> 
> For some HW it's done there (in the case there's no IP handling the
> HPD), but in some cases it's done in tpd12s015 driver (e.g. pandaboard),
> and in some cases it also could be done in the hdmi driver (if the HPD
> is handled by the HDMI IP, but at the moment we don't have this case
> supported in the SW).
> 
>> But there I have no access to hdmi.core (needed for the 
>> hdmi4_cec_set_phys_addr() call).
>>
>> Any idea how to solve this? I am not all that familiar with drm, let alone 
>> omapdrm,
>> so if you can point me in the right direction, then that would be very 
>> helpful.
> 
> Hmm, indeed, looks the the output is kept enabled even if HPD drops and
> the connector status is changed to disconnected.
> 
> I don't have a very good solution... I think we have to add a function
> to omapdss_hdmi_ops, which the connector-hdmi and tpd12s015 drivers can
> call when they detect a HPD change. That call would go to the HDMI IP
> driver.
> 
> Peter is about to send hotplug-interrupt-handling series, I think the
> HPD function work should be done on top of that, as otherwise it'll just
> conflict horribly.

Has that been merged yet? And if so, what git repo/branch should I base
my next version of this patch series on? If not, do you know when it is
expected?

Regards,

Hans

[GIT FIXES FOR v4.12] cec: fix race condition between poll and CEC_RECEIVE

Hi Mauro,

This bug fix should go to 4.12 (and has a CC to stable to get it in for
4.10 and 4.11 as well).

It fixes a race condition that can cause an application to loop forever, which
is how we (Cisco) discovered it.

Regards,

Hans

The following changes since commit 47b586f66a9e78c91586b9c363603a52c75840d7:

  [media] pvrusb2: remove redundant check on cnt > 8 (2017-06-07 13:52:41 -0300)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git cec-race-fix

for you to fetch changes up to bfafcd0557a8d963b05d4974a077e6a8306b164a:

  cec: race fix: don't return -ENONET in cec_receive() (2017-06-08 09:21:04 
+0200)


Hans Verkuil (1):
  cec: race fix: don't return -ENONET in cec_receive()

 drivers/media/cec/cec-api.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

Re: [PATCH v2 2/4] [media] davinci: vpif_capture: get subdevs from DT when available