Em Fri, 19 Feb 2016 16:40:46 +0200
Sakari Ailus <sakari.ailus@xxxxxx> escreveu:
> Hi Mauro,
>
> On Fri, Feb 19, 2016 at 12:03:41PM -0200, Mauro Carvalho Chehab wrote:
> > Hi Sakari,
> >
> > Em Wed, 27 Jan 2016 16:47:55 +0200
> > Sakari Ailus <sakari.ailus@xxxxxx> escreveu:
> >
> > > Re-create the graph walk object as needed in order to have one large enough
> > > available for all entities in the graph.
> > >
> > > This enumeration is used for pipeline power management in the future.
> > >
> > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/media/media-device.c | 21 +++++++++++++++++++++
> > > include/media/media-device.h | 5 +++++
> > > 2 files changed, 26 insertions(+)
> > >
> > > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> > > index 4d1c13d..52d7809 100644
> > > --- a/drivers/media/media-device.c
> > > +++ b/drivers/media/media-device.c
> > > @@ -577,6 +577,26 @@ int __must_check media_device_register_entity(struct media_device *mdev,
> > >
> > > spin_unlock(&mdev->lock);
> > >
> > > + mutex_lock(&mdev->graph_mutex);
> > > + if (mdev->entity_internal_idx_max
> > > + >= mdev->pm_count_walk.ent_enum.idx_max) {
> > > + struct media_entity_graph new = { 0 };
> > > +
> > > + /*
> > > + * Initialise the new graph walk before cleaning up
> > > + * the old one in order not to spoil the graph walk
> > > + * object of the media device if graph walk init fails.
> > > + */
> > > + ret = media_entity_graph_walk_init(&new, mdev);
> > > + if (ret) {
> > > + mutex_unlock(&mdev->graph_mutex);
> > > + return ret;
> > > + }
> > > + media_entity_graph_walk_cleanup(&mdev->pm_count_walk);
> > > + mdev->pm_count_walk = new;
> > > + }
> > > + mutex_unlock(&mdev->graph_mutex);
> > > +
> >
> > I don't like the idea of creating a new graph init and destroying the
> > previous one every time. In principle, this needs to be done only
> > when trying to start the graph - or just before registering the
> > MC devnode, if the driver needs/wants to optimize it.
>
> It's not every time --- with the previous patch, that's every 32 or 64
> additional entity, depending on how many bits the unsigned long is.
Still it will be called a lot of times for DVB devices. Why doing that,
if such alloc can be done only after finishing registering all devices?
>
> >
> > As kbuildtest also didn't like this patch, I'm not applying it
> > for now.
>
> For missing KernelDoc documentation for a struct field.
>
> Other fields in the struct don't have KernelDoc documentation either, and I
> didn't feel it'd fit well for this patch. I can add a patch to change the
> field documentation to the set if you like.
Ok, it could be done on a separate patch. Feel free to submit it.
Thanks!
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
