Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>,
	linux-media@vger.kernel.org,
	Jacopo Mondi <jacopo+renesas@jmondi.org>,
	niklas.soderlund+renesas@ragnatech.se,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>
Subject: Re: [PATCH v5 20/24] v4l: mc: Add an S_ROUTING helper function for power state changes
Date: Tue, 20 Apr 2021 19:41:47 +0300
Message-ID: <20210420164147.GV3@paasikivi.fi.intel.com> (raw)
In-Reply-To: <YHyArMhUhcamSf++@pendragon.ideasonboard.com>

Hi Laurent,

On Sun, Apr 18, 2021 at 09:55:40PM +0300, Laurent Pinchart wrote:
> Hi Tomi and Sakari,
> 
> Thank you for the patch.
> 
> On Thu, Apr 15, 2021 at 04:04:46PM +0300, Tomi Valkeinen wrote:
> > From: Sakari Ailus <sakari.ailus@linux.intel.com>
> > 
> > With the addition of the has_route() media entity operation, all pads of an
> > entity are no longer interconnected. The S_ROUTING IOCTL for sub-devices can
> > be used to enable and disable routes for an entity. The consequence is that
> > the routing information has to be taken into account in use count
> > calculation: disabling a route has a very similar effect on use counts as
> > has disabling a link.
> > 
> > Add a helper function for drivers implementing VIDIOC_SUBDEV_S_ROUTING
> > IOCTL to take the change into account.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  drivers/media/v4l2-core/v4l2-mc.c | 34 +++++++++++++++++++++++++++++++
> >  include/media/v4l2-mc.h           | 22 ++++++++++++++++++++
> >  2 files changed, 56 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c
> > index 35d18ed89fa5..71acb389aa7b 100644
> > --- a/drivers/media/v4l2-core/v4l2-mc.c
> > +++ b/drivers/media/v4l2-core/v4l2-mc.c
> > @@ -588,3 +588,37 @@ int v4l2_pipeline_link_notify(struct media_link *link, u32 flags,
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_pipeline_link_notify);
> > +
> > +int v4l2_subdev_routing_pm_use(struct media_entity *entity,
> > +			       struct v4l2_subdev_route *route)
> > +{
> > +	struct media_graph *graph =
> > +		&entity->graph_obj.mdev->pm_count_walk;
> > +	struct media_pad *source = &entity->pads[route->source_pad];
> > +	struct media_pad *sink = &entity->pads[route->sink_pad];
> > +	int source_use;
> > +	int sink_use;
> > +	int ret;
> > +
> > +	source_use = pipeline_pm_use_count(source, graph);
> > +	sink_use = pipeline_pm_use_count(sink, graph);
> > +
> > +	if (!(route->flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE)) {
> > +		/* Route disabled. */
> > +		pipeline_pm_power(source, -sink_use, graph);
> > +		pipeline_pm_power(sink, -source_use, graph);
> > +		return 0;
> > +	}
> > +
> > +	/* Route enabled. */
> > +	ret = pipeline_pm_power(source, sink_use, graph);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = pipeline_pm_power(sink, source_use, graph);
> > +	if (ret < 0)
> > +		pipeline_pm_power(source, -sink_use, graph);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_subdev_routing_pm_use);
> 
> .s_power() is getting deprecated for sensors. Sakari, should we move
> away from pipeline-based power management and drop this patch, or is it
> too early ?

Good question.

I've previously requested that instead of adding s_power support for bridge
and ISP drivers, sensor drivers used with them should be converted to
runtime PM instead. So proceeding along those lines, this code isn't
necessary. I'd actually drop it if there's no sound use case (I don't think
so).

> 
> > diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
> > index c181685923d5..ab8f4dc143aa 100644
> > --- a/include/media/v4l2-mc.h
> > +++ b/include/media/v4l2-mc.h
> > @@ -18,6 +18,7 @@
> >  /* We don't need to include pci.h or usb.h here */
> >  struct pci_dev;
> >  struct usb_device;
> > +struct v4l2_subdev_route;
> >  
> >  #ifdef CONFIG_MEDIA_CONTROLLER
> >  /**
> > @@ -184,6 +185,22 @@ void v4l2_pipeline_pm_put(struct media_entity *entity);
> >  int v4l2_pipeline_link_notify(struct media_link *link, u32 flags,
> >  			      unsigned int notification);
> >  
> > +/**
> > + * v4l2_subdev_routing_pm_use - Handle power state changes due to S_ROUTING
> > + * @entity: The entity
> > + * @route: The new state of the route
> > + *
> > + * Propagate the use count across a route in a pipeline whenever the
> > + * route is enabled or disabled. The function is called before
> > + * changing the route state when enabling a route, and after changing
> > + * the route state when disabling a route.
> > + *
> > + * Return 0 on success or a negative error code on failure. Powering entities
> > + * off is assumed to never fail. This function will not fail for disconnection
> > + * events.
> 
> "disconnection events" make me think about hotplug. How about "will not
> fail when disabling a route" ?
> 
> > + */
> > +int v4l2_subdev_routing_pm_use(struct media_entity *entity,
> > +			       struct v4l2_subdev_route *route);
> >  #else /* CONFIG_MEDIA_CONTROLLER */
> >  
> >  static inline int v4l2_mc_create_media_graph(struct media_device *mdev)
> > @@ -219,5 +236,10 @@ static inline int v4l2_pipeline_link_notify(struct media_link *link, u32 flags,
> >  	return 0;
> >  }
> >  
> > +static inline int v4l2_subdev_routing_pm_use(struct media_entity *entity,
> > +					     struct v4l2_subdev_route *route)
> > +{
> > +	return 0;
> > +}
> 
> Missing blank line.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> (assuming we want to keep the patch)
> 
> >  #endif /* CONFIG_MEDIA_CONTROLLER */
> >  #endif /* _V4L2_MC_H */
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Sakari Ailus

  reply index

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-15 13:04 [PATCH v5 00/24] v4l: subdev internal routing Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 01/24] media: entity: Use pad as a starting point for graph walk Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 02/24] media: entity: Use pads instead of entities in the media graph walk stack Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 03/24] media: entity: Walk the graph based on pads Tomi Valkeinen
2021-04-18 17:47   ` Laurent Pinchart
2021-04-20 11:30     ` Sakari Ailus
2021-04-15 13:04 ` [PATCH v5 04/24] v4l: mc: Start walk from a specific pad in use count calculation Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 05/24] media: entity: Add iterator helper for entity pads Tomi Valkeinen
2021-04-18 17:52   ` Laurent Pinchart
2021-04-22 12:04     ` Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 06/24] media: entity: Move the pipeline from entity to pads Tomi Valkeinen
2021-04-18 18:00   ` Laurent Pinchart
2021-04-20 11:38     ` Sakari Ailus
2021-04-15 13:04 ` [PATCH v5 07/24] media: entity: Use pad as the starting point for a pipeline Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 08/24] media: entity: Add has_route entity operation Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 09/24] media: entity: Add media_entity_has_route() function Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 10/24] media: entity: Use routing information during graph traversal Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 11/24] media: entity: Skip link validation for pads to which there is no route to Tomi Valkeinen
2021-04-18 18:06   ` Laurent Pinchart
2021-04-20 11:41     ` Sakari Ailus
2021-04-23 12:37       ` Tomi Valkeinen
2021-04-29 12:06         ` Sakari Ailus
2021-04-29 14:10           ` Laurent Pinchart
2021-04-15 13:04 ` [PATCH v5 12/24] media: entity: Add an iterator helper for connected pads Tomi Valkeinen
2021-04-18 18:20   ` Laurent Pinchart
2021-04-20 11:48     ` Sakari Ailus
2021-04-29  1:33       ` Laurent Pinchart
2021-04-29 11:56         ` Sakari Ailus
2021-04-29 12:04           ` Tomi Valkeinen
2021-04-29 12:07             ` Sakari Ailus
2021-04-29 12:14               ` Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 13/24] media: entity: Add only connected pads to the pipeline Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 14/24] media: entity: Add debug information in graph walk route check Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 15/24] v4l: Add bus type to frame descriptors Tomi Valkeinen
2021-04-18 19:23   ` Laurent Pinchart
2021-04-20 11:50     ` Sakari Ailus
2021-04-22 12:30       ` Tomi Valkeinen
2021-04-29 11:58         ` Sakari Ailus
2021-04-29 14:09           ` Laurent Pinchart
2021-04-15 13:04 ` [PATCH v5 16/24] v4l: Add CSI-2 bus configuration " Tomi Valkeinen
2021-04-18 19:24   ` Laurent Pinchart
2021-04-20 16:32     ` Sakari Ailus
2021-04-15 13:04 ` [PATCH v5 17/24] v4l: Add stream to frame descriptor Tomi Valkeinen
2021-04-18 19:27   ` Laurent Pinchart
2021-04-22 12:47     ` Tomi Valkeinen
2021-04-22 16:18       ` Laurent Pinchart
2021-04-15 13:04 ` [PATCH v5 18/24] v4l: subdev: Add [GS]_ROUTING subdev ioctls and operations Tomi Valkeinen
2021-04-18 18:32   ` Laurent Pinchart
2021-04-22 11:16     ` Tomi Valkeinen
2021-04-22 16:20       ` Laurent Pinchart
2021-04-22 16:58         ` Tomi Valkeinen
2021-04-20 16:35   ` Sakari Ailus
2021-04-15 13:04 ` [PATCH v5 19/24] media: Documentation: Add GS_ROUTING documentation Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 20/24] v4l: mc: Add an S_ROUTING helper function for power state changes Tomi Valkeinen
2021-04-18 18:55   ` Laurent Pinchart
2021-04-20 16:41     ` Sakari Ailus [this message]
2021-04-15 13:04 ` [PATCH v5 21/24] v4l: subdev: routing kernel helper functions Tomi Valkeinen
2021-04-18 19:18   ` Laurent Pinchart
2021-04-15 13:04 ` [PATCH v5 22/24] v4l: subdev: add v4l2_subdev_get_format_dir() Tomi Valkeinen
2021-04-18 19:04   ` Laurent Pinchart
2021-04-21 13:04     ` Tomi Valkeinen
2021-04-29  1:43       ` Laurent Pinchart
2021-05-04  6:49         ` Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 23/24] v4l: subdev: Take routing information into account in link validation Tomi Valkeinen
2021-04-15 13:04 ` [PATCH v5 24/24] v4l: subdev: increase V4L2_FRAME_DESC_ENTRY_MAX to 8 Tomi Valkeinen
2021-04-18 19:06   ` Laurent Pinchart
2021-04-16  8:38 ` [PATCH v5 00/24] v4l: subdev internal routing Niklas Söderlund
2021-04-16  8:47   ` Tomi Valkeinen
2021-04-16  8:56     ` Niklas Söderlund
2021-04-18 17:32 ` Laurent Pinchart
2021-04-21 12:57   ` Tomi Valkeinen
2021-04-29  1:27     ` Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210420164147.GV3@paasikivi.fi.intel.com \
    --to=sakari.ailus@linux.intel.com \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=jacopo+renesas@jmondi.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=tomi.valkeinen@ideasonboard.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git