[11/16] media: ti-vpe: cal: program number of lines properly

Message ID 20200313114121.32182-11-tomi.valkeinen@ti.com (mailing list archive)
State Changes Requested, archived
Delegated to: Hans Verkuil
Headers
Series [01/16] media: ti-vpe: cal: fix use of wrong macro |

Commit Message

Tomi Valkeinen March 13, 2020, 11:41 a.m. UTC
  CAL_CSI2_CTX register has LINES field, which, according to the
documentation, should be programmed to the number of lines transmitted
by the camera. If the number of lines is unknown, it can be set to 0.
The driver sets the field to 0 for some reason, even if we know the
number of lines.

This patch sets the number of lines properly, which will allow the HW to
discard extra lines (if the sensor would send such for some reason),
and, according to documentation: "This leads to regular video timings
and avoids potential artifacts".

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/media/platform/ti-vpe/cal.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
  

Comments

Laurent Pinchart March 16, 2020, 12:37 p.m. UTC | #1
On Fri, Mar 13, 2020 at 01:41:16PM +0200, Tomi Valkeinen wrote:
> CAL_CSI2_CTX register has LINES field, which, according to the
> documentation, should be programmed to the number of lines transmitted
> by the camera. If the number of lines is unknown, it can be set to 0.
> The driver sets the field to 0 for some reason, even if we know the
> number of lines.
> 
> This patch sets the number of lines properly, which will allow the HW to
> discard extra lines (if the sensor would send such for some reason),
> and, according to documentation: "This leads to regular video timings
> and avoids potential artifacts".

And possibly buffer overflows !

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/ti-vpe/cal.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index b5fd90a1ec09..832f37ebad0d 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -961,8 +961,7 @@ static void csi2_ctx_config(struct cal_ctx *ctx)
>  	set_field(&val, 0x1, CAL_CSI2_CTX_DT_MASK);
>  	/* Virtual Channel from the CSI2 sensor usually 0! */
>  	set_field(&val, ctx->virtual_channel, CAL_CSI2_CTX_VC_MASK);
> -	/* NUM_LINES_PER_FRAME => 0 means auto detect */
> -	set_field(&val, 0, CAL_CSI2_CTX_LINES_MASK);
> +	set_field(&val, ctx->v_fmt.fmt.pix.height, CAL_CSI2_CTX_LINES_MASK);
>  	set_field(&val, CAL_CSI2_CTX_ATT_PIX, CAL_CSI2_CTX_ATT_MASK);
>  	set_field(&val, CAL_CSI2_CTX_PACK_MODE_LINE,
>  		  CAL_CSI2_CTX_PACK_MODE_MASK);
  
Tomi Valkeinen March 16, 2020, 1:13 p.m. UTC | #2
On 16/03/2020 14:37, Laurent Pinchart wrote:
> On Fri, Mar 13, 2020 at 01:41:16PM +0200, Tomi Valkeinen wrote:
>> CAL_CSI2_CTX register has LINES field, which, according to the
>> documentation, should be programmed to the number of lines transmitted
>> by the camera. If the number of lines is unknown, it can be set to 0.
>> The driver sets the field to 0 for some reason, even if we know the
>> number of lines.
>>
>> This patch sets the number of lines properly, which will allow the HW to
>> discard extra lines (if the sensor would send such for some reason),
>> and, according to documentation: "This leads to regular video timings
>> and avoids potential artifacts".
> 
> And possibly buffer overflows !

There's a register in the DMA block which defines the max number of lines the DMA will transfer. So 
overflow should not be possible even without this patch.

  Tomi
  

Patch

diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index b5fd90a1ec09..832f37ebad0d 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -961,8 +961,7 @@  static void csi2_ctx_config(struct cal_ctx *ctx)
 	set_field(&val, 0x1, CAL_CSI2_CTX_DT_MASK);
 	/* Virtual Channel from the CSI2 sensor usually 0! */
 	set_field(&val, ctx->virtual_channel, CAL_CSI2_CTX_VC_MASK);
-	/* NUM_LINES_PER_FRAME => 0 means auto detect */
-	set_field(&val, 0, CAL_CSI2_CTX_LINES_MASK);
+	set_field(&val, ctx->v_fmt.fmt.pix.height, CAL_CSI2_CTX_LINES_MASK);
 	set_field(&val, CAL_CSI2_CTX_ATT_PIX, CAL_CSI2_CTX_ATT_MASK);
 	set_field(&val, CAL_CSI2_CTX_PACK_MODE_LINE,
 		  CAL_CSI2_CTX_PACK_MODE_MASK);