11 Jan, 2016

40 commits

  • Add media_entity_graph_walk_init() and media_entity_graph_walk_cleanup()
    functions in order to dynamically allocate memory for the graph. This is
    not done in media_entity_graph_walk_start() as there are situations where
    e.g. correct error handling, that itself may not fail, requires successful
    graph walk.

    Signed-off-by: Sakari Ailus
    Signed-off-by: Mauro Carvalho Chehab

    Sakari Ailus
     
  • The struct media_entity_graph was allocated in the stack, limiting the
    number of entities that could be reasonably allocated. Instead, move the
    struct to struct media_pipeline which is typically allocated using
    kmalloc() instead.

    The intent is to keep the enumeration around for later use for the
    duration of the streaming. As streaming is eventually stopped, an
    unfortunate memory allocation failure would prevent stopping the
    streaming. As no memory will need to be allocated, the problem is avoided
    altogether.

    Signed-off-by: Sakari Ailus
    Reviewed-by: Mauro Carvalho Chehab
    Signed-off-by: Mauro Carvalho Chehab

    Sakari Ailus
     
  • KernelDoc doesn't appear to handle anonymous structs defined inside
    another gracefully. As the struct is internal to the framework graph walk
    algorithm, detailed documentation isn't seen very important.

    Signed-off-by: Sakari Ailus
    Signed-off-by: Mauro Carvalho Chehab

    Sakari Ailus
     
  • It will be needed in struct media_pipeline shortly.

    Signed-off-by: Sakari Ailus
    Signed-off-by: Mauro Carvalho Chehab

    Sakari Ailus
     
  • This is useful in e.g. knowing whether certain operations have already
    been performed for an entity. The users include the framework itself (for
    graph walking) and a number of drivers.

    Signed-off-by: Sakari Ailus
    Signed-off-by: Mauro Carvalho Chehab

    Sakari Ailus
     
  • The internal index can be used internally by the framework in order to keep
    track of entities for a purpose or another. The internal index is constant
    while it's registered to a media device, but the same index may be re-used
    once the entity having that index is unregistered.

    Signed-off-by: Sakari Ailus
    Signed-off-by: Mauro Carvalho Chehab

    Sakari Ailus
     
  • We need to set "err = -ENOMEM" here.

    Fixes: 38b11f19667a ('[media] v4l2-core: create MC interfaces for devnodes')

    Signed-off-by: Dan Carpenter
    Signed-off-by: Mauro Carvalho Chehab

    Dan Carpenter
     
  • The copy_to/from_user() functions return the number of bytes *not*
    copied. They don't return error codes.

    Fixes: 4f6b3f363475 ('media] media-device: add support for MEDIA_IOC_G_TOPOLOGY ioctl')

    Signed-off-by: Dan Carpenter
    Signed-off-by: Mauro Carvalho Chehab

    Dan Carpenter
     
  • The G_TOPOLOGY ioctl is used to get a graph topology and since in the
    future a graph can be dynamically updated, there is a way to know the
    topology version so userspace can be aware that the graph has changed.

    The version 0 is reserved to indicate that the graph is static (i.e no
    graphs updates since the media device was registered).

    So, now that the media device initialization and registration has been
    split and the media device node is not exposed to user-space until all
    the entities have been registered and links created, it is safe to set
    a topology version 0 in media_device_register().

    Suggested-by: Laurent Pinchart
    Suggested-by: Mauro Carvalho Chehab
    Signed-off-by: Javier Martinez Canillas
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • Registering a V4L2 sub-device includes, among other things, registering
    the related media entity and calling the sub-device's registered op. Since
    patch "media: convert links from array to list", creating a link between
    two pads requires registering the entity first. If the registered() op
    involves link creation, the link list head will not be initialised before
    it is used.

    Resolve this by first registering the entity, then calling its
    registered() op.

    Signed-off-by: Sakari Ailus
    Signed-off-by: Mauro Carvalho Chehab

    Sakari Ailus
     
  • There are now two new warnings:

    drivers/media/usb/dvb-usb-v2/dvb_usb_core.c: In function 'dvb_usbv2_media_device_register':
    drivers/media/usb/dvb-usb-v2/dvb_usb_core.c:433:2: warning: ignoring return value of '__media_device_register', declared with attribute warn_unused_result [-Wunused-result]
    media_device_register(adap->dvb_adap.mdev);
    ^
    drivers/media/usb/dvb-usb/dvb-usb-dvb.c: In function 'dvb_usb_media_device_register':
    drivers/media/usb/dvb-usb/dvb-usb-dvb.c:128:2: warning: ignoring return value of '__media_device_register', declared with attribute warn_unused_result [-Wunused-result]
    media_device_register(adap->dvb_adap.mdev);
    ^

    Those are because the drivers are not properly checking if the
    media device init and register were succeeded.

    Fix it.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • The media device node is registered and so made visible to user-space
    before entities are registered and links created which means that the
    media graph obtained by user-space could be only partially enumerated
    if that happens too early before all the graph has been created.

    To avoid this race condition, split the media init and registration
    in separate functions and only register the media device node when
    all the pending subdevices have been registered, either explicitly
    by the driver or asynchronously using v4l2_async_register_subdev().

    The media_device_register() had a check for drivers not filling dev
    and model fields but all drivers in mainline set them and not doing
    it will be a driver bug so change the function return to void and
    add a BUG_ON() for dev being NULL instead.

    Also, add a media_device_cleanup() function that will destroy the
    graph_mutex that is initialized in media_device_init().

    [mchehab@osg.samsung.com: Fix compilation if !CONFIG_MEDIA_CONTROLLER
    and remove two warnings added by this changeset]
    Suggested-by: Sakari Ailus
    Signed-off-by: Javier Martinez Canillas
    Acked-by: Sakari Ailus
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • If media_device_unregister() is called by two different
    drivers, a race condition may happen, as the check if the
    device is not registered is not protected.

    Move the spin_lock() to happen earlier in the function, in order
    to prevent such race condition.

    Reported-by: Shuah Khan
    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • media entity register and unregister functions are called by media
    device register/unregister. Move them to occur earlier, as we'll need
    an unlocked version of media_device_entity_unregister() and we don't
    want to add a function prototype without needing it.

    No functional changes.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • Most media functions that unregister, check if the corresponding register
    function succeed before. So these functions can safely be called even if a
    registration was never made or the component as already been unregistered.

    Add the same check to media_device_unregister() function for consistency.

    This will also allow to split the media_device_register() function in an
    initialization and registration functions without the need to change the
    generic cleanup functions and error code paths for all the media drivers.

    Suggested-by: Sakari Ailus
    Signed-off-by: Javier Martinez Canillas
    Acked-by: Sakari Ailus
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • As pointed by Dan, the commit f8fd4c61b5ae ("[media] media-entity:
    protect object creation/removal using spin lock")' leads to the
    following static checker warning:

    drivers/media/media-entity.c:781 media_remove_intf_link()
    error: dereferencing freed memory 'link'

    drivers/media/media-entity.c
    777 void media_remove_intf_link(struct media_link *link)
    778 {
    779 spin_lock(&link->graph_obj.mdev->lock);
    780 __media_remove_intf_link(link);
    781 spin_unlock(&link->graph_obj.mdev->lock);

    In practice, I didn't see any troubles even with KASAN enabled. I guess
    gcc optimizer internally cached the mdev reference, instead of getting
    it twice. Yet, it is a very bad idea to rely on such optimization. So,
    let's fix the code.

    Reported-by: Dan Carpenter
    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • Changeset f8fd4c61b5ae ("[media] media-entity: protect object
    creation/removal using spin lock") changed the object creation/removal
    protection to spin lock, as this is what's used on media-device,
    keeping the mutex reserved for graph traversal routines. However, it
    also changed the link setup, by mistake.

    This could cause troubles, as the link setup can affect the graph
    traversal, and this is likely the reason for a mutex there.

    So, revert media_entity_setup_link() to use mutex.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • There is one struct and two functions that were not documented.
    Add the corresponding kernel-doc documentation for them.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • As we're using the headers file only for documentation, move the
    two kernel-doc macros to the header, and fix it to avoid
    warnings.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • Add kernel-doc documentation for media_device_get_devres and
    media_device_find_devres.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • Add a basic documentation for most ancillary functions.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • If a different entity->pipe in a pipeline was encountered, a warning was
    issued but the execution continued as if nothing had happened. Return an
    error instead right there.

    Signed-off-by: Sakari Ailus
    Signed-off-by: Mauro Carvalho Chehab

    Sakari Ailus
     
  • Add description for this new media controller ioctl.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • Document the media controller interfaces at the media
    uAPI docbook.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • There were some changes on the media types that were not reflected
    on the types tables. Update them to reflect the upstream changes.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • As MEDIA_IOC_G_TOPOLOGY shares the data types already declared
    for entities, pads and links, we should move those to a separate
    part of the document, and use cross-references where needed.

    So, move the following tables to a separate section at the
    DocBook:
    media-entity-type
    media-entity-flag
    media-pad-flag
    media-link-flag

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • The Media Controller next generation patches added a new graph
    element type: interfaces. It also allows links between interfaces
    and entities. Update the docbook to reflect that.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • The Media Controller next gen patchset added several new fields
    to be used with it. Document them.

    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • The V4L2 core won't be adding connectors to the tuners and other
    entities that need them. Let it be clear.

    Suggested-by: Laurent Pinchart
    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • Instead of flagging an interface link as MEDIA_LNK_FL_INTERFACE_LINK
    only when returning to userspace, do it at link creation time.

    That would allow using such flag internally, and cleans up a
    little bit the code for G_TOPOLOGY ioctl.

    [mchehab@osg.samsung.com: folded with a fixup from Dan Carpenter,
    replacing & by &&]
    Suggested-by: Hans Verkuil
    Signed-off-by: Mauro Carvalho Chehab

    Mauro Carvalho Chehab
     
  • Commit 86ee417578a2 ("[media] media: convert links from array to list")
    had many changes that were automated using coccinelle but the semantic
    patch was not smart enough to rely on operators precedence and avoid
    using unnecessary enclosing parenthesis.

    This patch removes them since are not needed.

    Suggested-by: Laurent Pinchart
    Signed-off-by: Javier Martinez Canillas
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • The uvc_mc_register_entities() function iterated over the entities three
    times to initialize the entities, register the subdev for the ones whose
    type was UVC_TT_STREAMING and to create the entities links.

    But this can be simplied by merging the init and registration logic in a
    single loop.

    Suggested-by: Laurent Pinchart
    Signed-off-by: Javier Martinez Canillas
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • The function uvc_mc_create_pads_links() creates entities links but the
    "pads" prefix is redundant since the driver doesn't handle any other
    kind of link, so it can be removed.

    Suggested-by: Laurent Pinchart
    Signed-off-by: Javier Martinez Canillas
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • The for loop in the vsp1_create_entities() function that create the links,
    checks the entity type and call the proper link creation function but then
    it uses continue to force the next iteration of the loop to take place and
    skipping code in between that creates links for different entities types.

    It is more readable and easier to understand if the if else constructs is
    used instead of the continue statement.

    Suggested-by: Laurent Pinchart
    Signed-off-by: Javier Martinez Canillas
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • The functions that create entities links are called *_create_pads_links()
    but the "pads" prefix is redundant since the driver doesn't handle any
    other kind of link so it can be removed.

    Suggested-by: Laurent Pinchart
    Signed-off-by: Javier Martinez Canillas
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • The functions that create ISS internal and external entities links are
    called *_create_pads_links() but the "pads" prefix is redundant since
    the driver doesn't handle any other kind of link so it can be removed.

    Suggested-by: Laurent Pinchart
    Signed-off-by: Javier Martinez Canillas
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • The isp_subdev_notifier_complete() complete callback defines a struct
    v4l2_device *v4l2_dev to avoid needing two level of indirections to
    access the V4L2 subdevs but the var is not always used when possible
    as when calling v4l2_device_register_subdev_nodes().

    So change that to consistently use the defined v4l2_dev pointer var.

    Suggested-by: Sakari Ailus
    Signed-off-by: Javier Martinez Canillas
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • Commit bc36b30fe06b ("[media] omap3isp: separate links creation from
    entities init") moved the link creation logic from the entities init
    functions and so removed the error_link labels from the error paths.

    But after that, some functions have a single error label so it makes
    more sense to rename the label to just "error" in thi case.

    Suggested-by: Laurent Pinchart
    Signed-off-by: Javier Martinez Canillas
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • The function that creates the links between ISP internal and external
    entities is called isp_create_pads_links() but the "pads" prefix is
    redundant since the driver doesn't handle any other kind of link so
    it can just be removed.

    While being there, fix the function's kernel-doc since is not using
    a proper format.

    Suggested-by: Laurent Pinchart
    Signed-off-by: Javier Martinez Canillas
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas
     
  • The entities to video nodes links were created on separate functions for
    each ISP module but since the only thing that these functions do is to
    call media_create_pad_link(), there's no need for that indirection level
    and all link creation logic can be just inlined in the caller function.

    Also, since the only possible failure for the link creation is a memory
    allocation, there is no need for error messages since the core already
    reports a very verbose message in that case.

    Suggested-by: Laurent Pinchart
    Signed-off-by: Javier Martinez Canillas
    Signed-off-by: Mauro Carvalho Chehab

    Javier Martinez Canillas