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 05B3E3A9D9E for ; Mon, 13 Apr 2026 12:42:10 +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=1776084133; cv=none; b=dIXsIUSgWDYi0BMl5XtbKQHMiNoIyZL8ZeUsgPjAvakyWGPb3yUf84Yy8rxHkbuXtyh1nMP/iWx5M0H9vwBm3+8ENs1+SpxT+3e066KicU89+zE/4Hl3/hBFrl2xS05Hi05Jh34964d4kX0Y18LPidqG3llJ+kIP3eNmSp7xMTQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776084133; c=relaxed/simple; bh=hwg0EDRv3yHjzQFTe/pH0P6RzegOXDD35zLsu+q/GW4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=WMlO3b1W0x7hB5HEUtO6xbkmbnywuVNFQByNcksOsfOK1rMueCk8cnTLY+gfl+6OrVhr1BU1fdlewKPdf0PzzYFxjm9O9uiws6GWDpVjuFj1YHd3f0rmOuZpwGiT0wV0dMBQ+FIWmND4l6E2lGHT5+xhBaSt084JSKx4w15VANI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (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=FpAtJdpV; arc=none smtp.client-ip=213.167.242.64 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (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="FpAtJdpV" Received: from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it [93.46.82.201]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E0E8663D; Mon, 13 Apr 2026 14:40:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1776084035; bh=hwg0EDRv3yHjzQFTe/pH0P6RzegOXDD35zLsu+q/GW4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=FpAtJdpVmUo58tjbgXKA5uGAvH6NkBQ4aHzYkbIvDhDCPSez6pnm7GEsoyAJogKq7 hAaF1xPRVn+r/9j8VavE74eJsYaeZdEtGYag/vPsJzDoKPz0v/2nzzpuvEx+DgR8Ql hgBWnatk23wJEWfcRT5BT7U7l+ypDirCUb3nZkkQ= Date: Mon, 13 Apr 2026 14:42:03 +0200 From: Jacopo Mondi To: Sakari Ailus Cc: Jacopo Mondi , linux-media@vger.kernel.org, hans@jjverkuil.nl, laurent.pinchart@ideasonboard.com, Prabhakar , Kate Hsuan , Dave Stevenson , Tommaso Merciai , Benjamin Mugnier , Sylvain Petinot , Christophe JAILLET , Julien Massot , Naushir Patuck , "Yan, Dongcheng" , "Cao, Bingbu" , "Qiu, Tian Shu" , Stefan Klug , Mirela Rabulea , =?utf-8?B?QW5kcsOp?= Apitzsch , Heimir Thor Sverrisson , Kieran Bingham , Mehdi Djait , Ricardo Ribalda Delgado , Hans de Goede , Tomi Valkeinen , David Plowman , "Yu, Ong Hock" , "Ng, Khai Wen" , Jai Luthra , Rishikesh Donadkar Subject: Re: [PATCH v4 25/29] media: v4l2-subdev: Move subdev client capabilities into a new struct Message-ID: References: <20260408153939.969381-1-sakari.ailus@linux.intel.com> <20260408153939.969381-26-sakari.ailus@linux.intel.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Hi Sakari On Mon, Apr 13, 2026 at 11:11:33AM +0300, Sakari Ailus wrote: > Hi Jacopo, > > On Fri, Apr 10, 2026 at 03:31:32PM +0200, Jacopo Mondi wrote: > > Hi Sakari > > > > On Wed, Apr 08, 2026 at 06:39:34PM +0300, Sakari Ailus wrote: > > > Add struct v4l2_subdev_client_info to hold sub-device client capability > > > bits that used to be stored in the client_caps field of struct > > > v4l2_subdev_fh. The intent is to enable passing this struct to sub-device > > > pad operation callbacks for capability information. The main reason why > > > this is a new struct instead of a u64 field is that modifying the callback > > > arguments requires touching almost every sub-device driver and that is > > > desirable to avoid in the future, should more than the client capability bits > > > need to be known to the callbacks. > > > > > > Signed-off-by: Sakari Ailus > > > Reviewed-by: Mirela Rabulea > > > --- > > > drivers/media/v4l2-core/v4l2-subdev.c | 8 ++++---- > > > include/media/v4l2-subdev.h | 12 ++++++++++-- > > > 2 files changed, 14 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > > > index 40b28e070726..eaa408832c6b 100644 > > > --- a/drivers/media/v4l2-core/v4l2-subdev.c > > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > > > @@ -612,7 +612,7 @@ subdev_ioctl_get_state(struct v4l2_subdev *sd, struct v4l2_subdev_fh *subdev_fh, > > > case VIDIOC_SUBDEV_S_FRAME_INTERVAL: { > > > struct v4l2_subdev_frame_interval *fi = arg; > > > > > > - if (!(subdev_fh->client_caps & > > > + if (!(subdev_fh->ci.client_caps & > > > V4L2_SUBDEV_CLIENT_CAP_INTERVAL_USES_WHICH)) > > > fi->which = V4L2_SUBDEV_FORMAT_ACTIVE; > > > > > > @@ -652,7 +652,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg, > > > struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh); > > > bool ro_subdev = test_bit(V4L2_FL_SUBDEV_RO_DEVNODE, &vdev->flags); > > > bool streams_subdev = sd->flags & V4L2_SUBDEV_FL_STREAMS; > > > - bool client_supports_streams = subdev_fh->client_caps & > > > + bool client_supports_streams = subdev_fh->ci.client_caps & > > > V4L2_SUBDEV_CLIENT_CAP_STREAMS; > > > int rval; > > > > > > @@ -1119,7 +1119,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg, > > > case VIDIOC_SUBDEV_G_CLIENT_CAP: { > > > struct v4l2_subdev_client_capability *client_cap = arg; > > > > > > - client_cap->capabilities = subdev_fh->client_caps; > > > + client_cap->capabilities = subdev_fh->ci.client_caps; > > > > > > return 0; > > > } > > > @@ -1139,7 +1139,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg, > > > client_cap->capabilities &= (V4L2_SUBDEV_CLIENT_CAP_STREAMS | > > > V4L2_SUBDEV_CLIENT_CAP_INTERVAL_USES_WHICH); > > > > > > - subdev_fh->client_caps = client_cap->capabilities; > > > + subdev_fh->ci.client_caps = client_cap->capabilities; > > > > > > I'm sorry for being annoying, but the discussion on v2 ended with the > > question on why we can't propagate the client caps to the drivers > > using the subdev state. > > > > I understand we have the active state which is stored in the subdev > > and not per-file handle. But ioctls are called on a file handle and it > > seems trivial to me copy the caps from the subdev_fh to the active > > state. > > > > It would result in a much smaller set of changes. > > > > What am I missing ? > > I thought the discussion ended with the conclusion that we can't do that as > the sub-device active state isn't bound to a file handle. :-) > Can you clarify why this can't happen in your opinion ? Yes, the active state is not bound to any fh, but all ioctls operations are issued on an fh, so it's trivial to copy the caps flags in the active state before passing it to drivers. If this is not preferred because it feels like an hack I understand it, it actually is, but I would like to think in perspective here. We want contexts, I think that's clear. In the design I proposed the active state will be moved from the subdev to the context (when available). A state lives in a context which is bound to file handle. Getting the caps from a state is then a matter of providing a helpers in the form of a chain of container_of. The active_state which lives in the subdev will be used as a "default" state, to support userspace application which are not context-aware. Would it be so bad to populate a v4l2_subdev_fh * in the active state stored in the subdev to indicate on which file handle an ioctl has been called on and retrieve the caps from there, so that the same (name tbd) v4l2_subdev_get_state_caps(struct v4l2_subdev_state *state) could be used ? Adding the caps argument to driver's ioctl handlers to me pollutes the interface diminishing the value of the state-centric interface we are trying to implement. > -- > Kind regards, > > Sakari Ailus