From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 13A1C60869; Wed, 13 Mar 2024 19:12:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.167.242.64 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710357141; cv=none; b=AETp4T5u5v4jqHON+dRaETeSyOz5fSjmWjKIVIx4ByIh2jY0J/OIYmoF3xYo5/alfucxCXcZH0ukcWdUbFhddDprnIH99IcMmVNv0Tl15Sa8OIFIcFovfE9BIXjt0YJB0xSs3iDVts76cZxFMYBISCbX5+/tQ+r7leJ3Z0mkUuM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710357141; c=relaxed/simple; bh=hTfoqgr5I8nbMsecUcmRsIEjs7m0kEe+SvwfjmNPTL4=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=o+c8kbjQDakpKI5AIFOUJYVIpg+KblPhfnRkS/zqacUmQxk3mPk6xXj3662cQJvZlsLAjs4aSriNLtCcB85VKM4u6+csgrjxRcF0BCOwR6aSSDS+zX01BI3FlZsu6Mbpu6SKDeg+1OlIZCBTeUQfsXvZXprOSoPtvthHXb5kFOs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com; spf=pass smtp.mailfrom=ideasonboard.com; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b=lMKDdus4; arc=none smtp.client-ip=213.167.242.64 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="lMKDdus4" Received: from [192.168.1.104] (unknown [103.251.226.70]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CA0EA720; Wed, 13 Mar 2024 20:11:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710357115; bh=hTfoqgr5I8nbMsecUcmRsIEjs7m0kEe+SvwfjmNPTL4=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=lMKDdus4yYsWv68nkNJjncIXF6yzNIPvugGvQPNeGzqqO5tAzVg5O5OWfhyLix7oi zVsnSroLgDnLIcXdN8M0bgW6IflBC/pjOoRQSA4RTsRgmY3fA9VR1aDnP9yUAEc7WS 5EfNDHWrPAjoaf1/OJ98I33W6RMjpFk5GUE/tm9A= Message-ID: <99069f32-4bad-4cb9-aa03-4c06a596a248@ideasonboard.com> Date: Thu, 14 Mar 2024 00:42:08 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mipi-csis: Emit V4L2_EVENT_FRAME_SYNC events To: Stefan Klug , libcamera-devel@lists.libcamera.org, linux-media@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Cc: Martin Kepplinger , Purism Kernel Team , Fabio Estevam , Sascha Hauer , Rui Miguel Silva , Pengutronix Kernel Team , Mauro Carvalho Chehab , Shawn Guo , NXP Linux Team References: <20240313153058.189684-1-stefan.klug@ideasonboard.com> Content-Language: en-US From: Umang Jain In-Reply-To: <20240313153058.189684-1-stefan.klug@ideasonboard.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Stefan, Thank you for the patch. On 13/03/24 9:00 pm, Stefan Klug wrote: > The Samsung CSIS Mipi receiver provides a start-of-frame interrupt and > a framecount register. As the CSI receiver is the hardware unit > that lies closest to the sensor, the frame counter is the best we can > get on these devices. > In case of the ISI available on the i.MX8 M Plus it is also the only > native start-of-frame signal available. > > This patch exposes the sof interrupt and the framecount as > V4L2_EVENT_FRAME_SYNC event on the subdevice. > > It was tested on a Debix-Som-A with a 6.8-rc4 kernel. > > Signed-off-by: Stefan Klug > --- > drivers/media/platform/nxp/imx-mipi-csis.c | 34 +++++++++++++++++++++- > 1 file changed, 33 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c > index db8ff5f5c4d3..caeb1622f741 100644 > --- a/drivers/media/platform/nxp/imx-mipi-csis.c > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c > @@ -30,6 +30,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -742,6 +743,18 @@ static void mipi_csis_stop_stream(struct mipi_csis_device *csis) > mipi_csis_system_enable(csis, false); > } > > +static void mipi_csis_queue_event_sof(struct mipi_csis_device *csis) > +{ > + struct v4l2_event event = { > + .type = V4L2_EVENT_FRAME_SYNC, > + }; > + > + u32 frame = mipi_csis_read(csis, MIPI_CSIS_FRAME_COUNTER_CH(0)); nit: variables declaration go at the start of function and then need to be assigned. rest is pointed already on the thread. Otherwise LGTM, Reviewed-by: Umang Jain > + > + event.u.frame_sync.frame_sequence = frame; > + v4l2_event_queue(csis->sd.devnode, &event); > +} > + > static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id) > { > struct mipi_csis_device *csis = dev_id; > @@ -765,6 +778,10 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id) > event->counter++; > } > } > + > + if (status & MIPI_CSIS_INT_SRC_FRAME_START) > + mipi_csis_queue_event_sof(csis); > + > spin_unlock_irqrestore(&csis->slock, flags); > > mipi_csis_write(csis, MIPI_CSIS_INT_SRC, status); > @@ -1154,8 +1171,23 @@ static int mipi_csis_log_status(struct v4l2_subdev *sd) > return 0; > } > > +static int mipi_csis_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh, > + struct v4l2_event_subscription *sub) > +{ > + if (sub->type != V4L2_EVENT_FRAME_SYNC) > + return -EINVAL; > + > + /* V4L2_EVENT_FRAME_SYNC doesn't require an id, so zero should be set */ > + if (sub->id != 0) > + return -EINVAL; > + > + return v4l2_event_subscribe(fh, sub, 0, NULL); > +} > + > static const struct v4l2_subdev_core_ops mipi_csis_core_ops = { > .log_status = mipi_csis_log_status, > + .subscribe_event = mipi_csis_subscribe_event, > + .unsubscribe_event = v4l2_event_subdev_unsubscribe, > }; > > static const struct v4l2_subdev_video_ops mipi_csis_video_ops = { > @@ -1358,7 +1390,7 @@ static int mipi_csis_subdev_init(struct mipi_csis_device *csis) > snprintf(sd->name, sizeof(sd->name), "csis-%s", > dev_name(csis->dev)); > > - sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS; > sd->ctrl_handler = NULL; > > sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;