Hi Laurent,
On Thu, Jul 25, 2013 at 01:46:54PM +0200, Laurent Pinchart wrote:
> > On Wed, Jul 17, 2013 at 04:54:42PM +0200, Laurent Pinchart wrote:
> > ...
> >
> > > +static void vsp1_device_init(struct vsp1_device *vsp1)
> > > +{
> > > + unsigned int i;
> > > + u32 status;
> > > +
> > > + /* Reset any channel that might be running. */
> > > + status = vsp1_read(vsp1, VI6_STATUS);
> > > +
> > > + for (i = 0; i < VPS1_MAX_WPF; ++i) {
> > > + unsigned int timeout;
> > > +
> > > + if (!(status & VI6_STATUS_SYS_ACT(i)))
> > > + continue;
> > > +
> > > + vsp1_write(vsp1, VI6_SRESET, VI6_SRESET_SRTS(i));
> > > + for (timeout = 10; timeout > 0; --timeout) {
> > > + status = vsp1_read(vsp1, VI6_STATUS);
> > > + if (!(status & VI6_STATUS_SYS_ACT(i)))
> > > + break;
> > > +
> > > + usleep_range(1000, 2000);
> > > + }
> > > +
> > > + if (timeout)
> > > + dev_err(vsp1->dev, "failed to reset wpf.%u\n", i);
> >
> > Have you seen this happening in practice? Do you expect the device to
> > function if resetting it fails?
>
> I've seen this happening during development when I had messed up register
> values, but not otherwise. I don't expect the deviec to still function if
> resetting the WPF fails, but I need to make sure that the busy loop exits.
Shouldn't you also return an error in this case? The function currently
returns void.
...
> > > + /* Follow links downstream for each input and make sure the graph
> > > + * contains no loop and that all branches end at the output WPF.
> > > + */
> >
> > I wonder if checking for loops should be done already in pipeline validation
> > done by the framework. That's fine to do later on IMHO, too.
>
> It would have to be performed by the core, as the callbacks are local to
> links. That's feasible (but should be optional, as some devices might support
> circular graphs), feel free to submit a patch :-)
As a matter of fact I think I will. I'd like you to test it though since I
have no hardware with such media graph. :-)
But please don't expect to see that in time for your driver to get in. Let's
think about that later on.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
--
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
