diff mbox

[RFC,12/12] mipi-csis: Enable all interrupts for fimc-is usage

Message ID 1362754765-2651-13-git-send-email-arun.kk@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Arun Kumar K March 8, 2013, 2:59 p.m. UTC
FIMC-IS firmware needs all the MIPI-CSIS interrupts to be enabled.
This patch enables all those MIPI interrupts.

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 drivers/media/platform/s5p-fimc/mipi-csis.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Hi Arun,

On 03/08/2013 03:59 PM, Arun Kumar K wrote:
> FIMC-IS firmware needs all the MIPI-CSIS interrupts to be enabled.
> This patch enables all those MIPI interrupts.
> 
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
>  drivers/media/platform/s5p-fimc/mipi-csis.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.c b/drivers/media/platform/s5p-fimc/mipi-csis.c
> index debda7c..11eef67 100644
> --- a/drivers/media/platform/s5p-fimc/mipi-csis.c
> +++ b/drivers/media/platform/s5p-fimc/mipi-csis.c
> @@ -64,7 +64,7 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
>  
>  /* Interrupt mask */
>  #define S5PCSIS_INTMSK			0x10
> -#define S5PCSIS_INTMSK_EN_ALL		0xf000103f
> +#define S5PCSIS_INTMSK_EN_ALL		0xfc00103f

Do you know what interrupts are assigned to the CSIS_INTMSK
bits 26, 27 ? In the documentation I have they are marked
as reserved. I have tested this patch on Exynos4x12, it seems
OK but you might want to merge it to the patch adding compatible
property for exynos5.

It would be good to know what these bits are for. And how
enabling the interrupts actually help without modifying the
interrupt handler ? Is it enough to just acknowledge those
interrupts ? Or how it works ?

--

Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arun Kumar K March 13, 2013, 4:09 a.m. UTC | #2
Hi Sylwester,

>>
>>  /* Interrupt mask */
>>  #define S5PCSIS_INTMSK                       0x10
>> -#define S5PCSIS_INTMSK_EN_ALL                0xf000103f
>> +#define S5PCSIS_INTMSK_EN_ALL                0xfc00103f
>
> Do you know what interrupts are assigned to the CSIS_INTMSK
> bits 26, 27 ? In the documentation I have they are marked
> as reserved. I have tested this patch on Exynos4x12, it seems
> OK but you might want to merge it to the patch adding compatible
> property for exynos5.

The bits 26 and 27 are for Frame start and Frame end interrupts.
Yes this change can be merged with the MIPI-CSIS support for Exynos5.
Shaik will pick it up and merge it along with his patch series in v2.

>
> It would be good to know what these bits are for. And how
> enabling the interrupts actually help without modifying the
> interrupt handler ? Is it enough to just acknowledge those
> interrupts ? Or how it works ?
>

These interrupts are used by the FIMC-IS firmware possibly to check if the
sensor is working. Without enabling these, I get the error from firmware
on Sensor Open command.

Regards
Arun
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hi Arun,

On 03/13/2013 05:09 AM, Arun Kumar K wrote:
> Hi Sylwester,
> 
>>>
>>>  /* Interrupt mask */
>>>  #define S5PCSIS_INTMSK                       0x10
>>> -#define S5PCSIS_INTMSK_EN_ALL                0xf000103f
>>> +#define S5PCSIS_INTMSK_EN_ALL                0xfc00103f
>>
>> Do you know what interrupts are assigned to the CSIS_INTMSK
>> bits 26, 27 ? In the documentation I have they are marked
>> as reserved. I have tested this patch on Exynos4x12, it seems
>> OK but you might want to merge it to the patch adding compatible
>> property for exynos5.
> 
> The bits 26 and 27 are for Frame start and Frame end interrupts.
> Yes this change can be merged with the MIPI-CSIS support for Exynos5.
> Shaik will pick it up and merge it along with his patch series in v2.

OK, thanks a lot for the clarification. I tested this patch on
Exynos4x12 and I could see twice as many interrupts from MIPI-CSIS as
there was captured frames from the sensor. Certainly we don't want to
see these interrupts when they are not needed. I have been thinking of
some interface that the MIPI-CSIS subdev would provide to the media
driver, so it can enable the interrupts when needed. I suppose a
private subdev ioctl might be good for that. But first I think there
is e.g. a subdev flag needed so a subdev driver can decide that it
doesn't want to have its non-standard ioctls called from user space.

I'll see if I can address those issues.

>> It would be good to know what these bits are for. And how
>> enabling the interrupts actually help without modifying the
>> interrupt handler ? Is it enough to just acknowledge those
>> interrupts ? Or how it works ?
>>
> 
> These interrupts are used by the FIMC-IS firmware possibly to check if the
> sensor is working. Without enabling these, I get the error from firmware
> on Sensor Open command.

Hm, interesting... Looks like the MIPI-CSIS interrupts get routed to the
FIMC-IS GIC. I was also wondering how the FIMC-IS receives Embedded Data
from the MIPI CSIS IP, which is sent by an image sensor. Presumably
FIMC-IS can memory map the Embedded Data buffer at the MIPI CSIS internal
memory, and then it reads from there. It's just a guess though.

Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.c b/drivers/media/platform/s5p-fimc/mipi-csis.c
index debda7c..11eef67 100644
--- a/drivers/media/platform/s5p-fimc/mipi-csis.c
+++ b/drivers/media/platform/s5p-fimc/mipi-csis.c
@@ -64,7 +64,7 @@  MODULE_PARM_DESC(debug, "Debug level (0-2)");
 
 /* Interrupt mask */
 #define S5PCSIS_INTMSK			0x10
-#define S5PCSIS_INTMSK_EN_ALL		0xf000103f
+#define S5PCSIS_INTMSK_EN_ALL		0xfc00103f
 #define S5PCSIS_INTMSK_EVEN_BEFORE	(1 << 31)
 #define S5PCSIS_INTMSK_EVEN_AFTER	(1 << 30)
 #define S5PCSIS_INTMSK_ODD_BEFORE	(1 << 29)