Hi Niklas,
On Sat, Jun 14, 2025 at 04:15:44PM +0200, Niklas Söderlund wrote:
> Depending on if the capture session deals with fields or whole frames
> interrupts can be generated at an end of field, or end of frame event.
> The interrupt mask is setup to generate an interrupt on one of the two
> events depending on what is needed when the VIN is started. The end of
> field bit is set in both cases so controlling the mask that generates an
> interrupt have been enough to control the two use-cases.
>
> Before extending the interrupt handler to deal with other types of
> interrupt events it is needs to extended to "capture complete" check for
> correct the use-case in operation. Without this the simplification in
> the handler can result in corrupted frames when the mask on what type of
> events can generate an interrupt generated can no longer be assumed to
> only be an "capture complete" event.
>
> Which bit is checked matches which bit is enabled at configuration time
> as which event can generate an interrupt for "capture complete". There
> is no functional change.
>
> While at it switch to use the BIT() macro to describe the bit positions
> for the interrupt functions.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> ---
> drivers/media/platform/renesas/rcar-vin/rcar-dma.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> index 585b8b3dcfd8..85e44a00e0fc 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> @@ -115,11 +115,12 @@
> #define VNFC_S_FRAME (1 << 0)
>
> /* Video n Interrupt Enable Register bits */
> -#define VNIE_FIE (1 << 4)
> -#define VNIE_EFE (1 << 1)
> +#define VNIE_FIE BIT(4)
> +#define VNIE_EFE BIT(1)
>
> /* Video n Interrupt Status Register bits */
> -#define VNINTS_FIS (1 << 4)
> +#define VNINTS_FIS BIT(4)
> +#define VNINTS_EFS BIT(1)
>
> /* Video n Data Mode Register bits */
> #define VNDMR_A8BIT(n) (((n) & 0xff) << 24)
> @@ -1034,7 +1035,7 @@ static void rvin_capture_stop(struct rvin_dev *vin)
> static irqreturn_t rvin_irq(int irq, void *data)
> {
> struct rvin_dev *vin = data;
> - u32 status, vnms;
> + u32 capture, status, vnms;
> int slot;
> unsigned int handled = 0;
> unsigned long flags;
> @@ -1049,7 +1050,10 @@ static irqreturn_t rvin_irq(int irq, void *data)
> handled = 1;
>
> /* Nothing to do if nothing was captured. */
> - if (!(status & VNINTS_FIS))
> + capture = vin->format.field == V4L2_FIELD_NONE ||
> + vin->format.field == V4L2_FIELD_ALTERNATE ?
> + VNINTS_FIS : VNINTS_EFS;
I would find
capture = vin->format.field == V4L2_FIELD_NONE ||
vin->format.field == V4L2_FIELD_ALTERNATE
? VNINTS_FIS : VNINTS_EFS;
easier to read, but it could be just me.
Do I read it correctly that you use the "Field Interrupt" with
V4L2_FIELD_NONE ? That seems a bit weird to me, but maybe I don't get
how the hardware operates.
> + if (!(status & capture))
> goto done;
>
> /* Nothing to do if not running. */
--
Regards,
Laurent Pinchart
