lkml.org 
[lkml]   [2023]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v7 00/13] CSI2RX support on J721E
    Hi Martyn,

    Thanks for the tests.

    On Mar 23, 2023 at 19:36:18 +0000, Martyn Welch wrote:
    > On Tue, 2023-03-14 at 17:25 +0530, Vaishnav Achath wrote:
    > > Hi,
    > >
    > > This series adds support for CSI2 capture on J721E. It includes some
    > > fixes to the Cadence CSI2RX driver, and adds the TI CSI2RX wrapper
    > > driver.
    > >
    > > This is a V7 of the below V6 series,
    > > https://lore.kernel.org/all/20220121142904.4091481-1-p.yadav@ti.com/
    > >
    > > Since Pratyush moved out of TI, I will be working on upstreaming the
    > > TI CSI2RX wrapper support.
    > >
    > > Tested on TI's J721E EVM with LI OV5640 sensor module.
    > > https://gist.github.com/vaishnavachath/f030a257d5b6569817bc9deab1c4fa77
    > >
    > > Also, Tested on TI AM62-SK with Pcam5C OV5640 module.
    > > https://gist.github.com/vaishnavachath/ff2605faa92f1a6ab5670426da28ccee
    > >
    >
    > Hi Vaishnav,
    >
    > I assume I'm doing something wrong. I have a TI AM62-SK and the Pcam5C
    > OV5640 module. I've been trying to test this with gstreamer using the
    > following command:
    >
    > gst-launch-1.0 -v v4l2src device=/dev/video0 num-buffers=10 ! video/x-
    > raw, width=640, height=480, format=UYVY, framerate=30/1 ! jpegenc !
    > multifilesink location=test%d.jpg
    >
    > However I've not been able to get this working, failing with this
    > failure unless I patch in some changes I found in the TI BSP:
    >
    > [ 28.083635] cdns-mipi-dphy-rx 30110000.phy: DPHY wait for lane ready
    > timeout
    > [ 28.090905] cdns-csi2rx 30101000.csi-bridge: Failed to configure
    > external DPHY: -110

    That is expected as currently the DPHY driver does not release the SW
    reset which all SoCs since J721E SR1.0 expect. See the patch linked by
    Vaishnav below.

    >
    > The changes (and the device tree nodes I added, which might be
    > wrong...) can be found here:
    >
    > https://gitlab.collabora.com/martyn/linux/-/commits/am625-sk-ov5640
    >
    > Any ideas what I'm doing wrong?
    >
    > Martyn
    >
    > > For all newer TI platforms that TI J721E Silicon Revision 1.0, below
    > > update
    > > to DPHY RX driver is needed:
    > > https://lore.kernel.org/all/20230314073137.2153-1-vaishnav.a@ti.com/

    ^

    With that patch and this series on top of linux-next, I was able to get
    Pcam5C capturing with SK-AM62. Although I did face issues with the
    sensor framerate and had to revert a few recent sensor commits, not sure
    why exactly yet. But here is the working branch with all the changes:
    https://github.com/jailuthra/linux/commits/b4/csi_single

    Let us know if you face any other issues during your tests.

    > >
    > > Changes in v7:
    > > - For patch 10/13 ("Add CSI2RX upport for J721E"):
    > > - Fix incorrect value written in SHIM_PSI_CFG0_DST_TAG
    > > - Drop support for 2X8 formats.
    > > - Update maintainer to Vaishnav as Pratyush moved out of TI.
    > > - Address Sakari's review comments:
    > > - Update MAX_HEIGHT_LINES, MAX_WIDTH_BYTES to prevent overflow.
    > > - Assign dma_slave_config during declaration, drop memset().
    > > - dma_release_channel() on slave_config failure.
    > > - provide entity ops for the vdev entity with link_validate().
    > > - mutex_destroy() on ti_csi2rx_probe failure path.
    > > - Drop busy check in remove().
    > > - mutex_destroy() in ti_csi2rx_remove().
    > > - Address Laurent's review comments:
    > > - Update entries in Makefile in alphabetical order.
    > > - include headers in alphabetical order.
    > > - Drop redundant CSI DT defines and use from media/mipi-csi2.h.
    > > - Rename csi_df to csi_dt.
    > > - Drop v4l2_colorspace from ti_csi2rx_fmt and set default in
    > >   ti_csi2rx_v4l2_init()
    > > - Adjust field and not return EINVAL in ti_csi2rx_try_fmt_vid_cap().
    > > - inline ti_csi2rx_video_register().
    > > - start DMA before starting source subdev.
    > > - move buffer cleanup to separate function
    > > ti_csi2rx_cleanup_buffers()
    > >   to be used in ti_csi2rx_stop_streaming() and
    > > ti_csi2rx_start_streaming()
    > >   failure path.
    > > - Drop VB2_USERPTR, VB2_READ and V4L2_CAP_READWRITE.
    > > - For patch 4/13 ("media: cadence: csi2rx: Add external DPHY
    > > support"):
    > > - Fix multiplier and divider in v4l2_get_link_freq() which caused
    > >   failures during streaming.
    > >
    > > Changes in v6:
    > > - Move the lock around the dereference for framefmt in
    > >   csi2rx_{get,set}_fmt() instead of when we get the pointer.
    > > - Do not return an error when an unsupported format is set. Instead
    > >   adjust the code to the first format in the list.
    > > - Drop variable bpp and use fmt->bpp directly.
    > > - Drop variable got_pm. Call phy_pm_runtime_put() unconditionally
    > > since
    > >   it will just return an error if runtime PM is not enabled.
    > > - Drop transcoding from the commit message.
    > > - Make csi2rx_media_ops const.
    > >
    > > Changes in v5:
    > > - Cleanup notifier in csi2rx_parse_dt() after the call to
    > >   v4l2_async_nf_add_fwnode_remote().
    > > - Use YUV 1X16 formats instead of 2X8.
    > > - Only error out when phy_pm_runtime_get_sync() returns a negative
    > >   value. A positive value can be returned if the phy was already
    > >   resumed.
    > > - Do not query the source subdev for format. Use the newly added
    > >   internal format instead.
    > > - Make i unsigned.
    > > - Change %d to %u
    > > - Add dependency on PHY_CADENCE_DPHY_RX instead of PHY_CADENCE_DPHY
    > >   since the Rx mode DPHY now has a separate driver.
    > > - Drop ti_csi2rx_validate_pipeline(). Pipeline validation should be
    > > done
    > >   at media_pipeline_start().
    > > - Do not assign flags.
    > > - Fix error handling in ti_csi2rx_start_streaming(). Free up vb2
    > > buffers
    > >   when media_pipeline_start() fails.
    > > - Move clock description in comments under the clocks property.
    > > - Make ports required.
    > > - Add link validation to cdns-csi2rx driver.
    > >
    > > Changes in v4:
    > > - Drop the call to set PHY submode. It is now being done via
    > > compatible
    > >   on the DPHY side.
    > > - Acquire the media device's graph_mutex before starting the graph
    > > walk.
    > > - Call media_graph_walk_init() and media_graph_walk_cleanup() when
    > >   starting and ending the graph walk respectively.
    > > - Reduce max frame height and width in enum_framesizes. Currently
    > > they
    > >   are set to UINT_MAX but they must be a multiple of step_width, so
    > > they
    > >   need to be rounded down. Also, these values are absurdly large
    > > which
    > >   causes some userspace applications like gstreamer to trip up. While
    > > it
    > >   is not generally right to change the kernel for an application bug,
    > > it
    > >   is not such a big deal here. This change is replacing one set of
    > >   absurdly large arbitrary values with another set of smaller but
    > > still
    > >   absurdly large arbitrary values. Both limits are unlikely to be hit
    > > in
    > >   practice.
    > > - Add power-domains property.
    > > - Drop maxItems from clock-names.
    > > - Drop the type for data-lanes.
    > > - Drop uniqueItems from data-lanes. Move it to video-interfaces.yaml
    > >   instead.
    > > - Drop OV5640 runtime pm patch. It seems to be a bit complicated and
    > > it
    > >   is not exactly necessary for this series. Any CSI-2 camera will
    > > work
    > >   just fine, OV5640 just happens to be the one I tested with. I don't
    > >   want it to block this series. I will submit it as a separate patch
    > >   later.
    > >
    > > Changes in v3:
    > > - Use v4l2_get_link_freq() to calculate pixel clock.
    > > - Move DMA related fields in struct ti_csi2rx_dma.
    > > - Protect DMA buffer queue with a spinlock to make sure the queue
    > > buffer
    > >   and DMA callback don't race on it.
    > > - Track the current DMA state. It might go idle because of a lack of
    > >   buffers. This state can be used to restart it if needed.
    > > - Do not include the current buffer in the pending queue. It is
    > > slightly
    > >   better modelling than leaving it at the head of the pending queue.
    > > - Use the buffer as the callback argument, and add a reference to csi
    > > in it.
    > > - If queueing a buffer to DMA fails, the buffer gets leaked and DMA
    > > gets
    > >   stalled with. Instead, report the error to vb2 and queue the next
    > >   buffer in the pending queue.
    > > - DMA gets stalled if we run out of buffers since the callback is the
    > >   only one that fires subsequent transfers and it is no longer being
    > >   called. Check for that when queueing buffers and restart DMA if
    > >   needed.
    > > - Do not put of node until we are done using the fwnode.
    > > - Set inital format to UYVY 640x480.
    > > - Add compatible: contains: const: cdns,csi2rx to allow SoC specific
    > >   compatible.
    > > - Add more constraints for data-lanes property.
    > >
    > > Changes in v2:
    > > - Use phy_pm_runtime_get_sync() and phy_pm_runtime_put() before
    > > making
    > >   calls to set PHY mode, etc. to make sure it is ready.
    > > - Use dmaengine_get_dma_device() instead of directly accessing
    > >   dma->device->dev.
    > > - Do not set dst_addr_width when configuring slave DMA.
    > > - Move to a separate subdir and rename to j721e-csi2rx.c
    > > - Convert compatible to ti,j721e-csi2rx.
    > > - Move to use Media Controller centric APIs.
    > > - Improve cleanup in probe when one of the steps fails.
    > > - Add colorspace to formats database.
    > > - Set hw_revision on media_device.
    > > - Move video device initialization to probe time instead of register
    > > time.
    > > - Rename to ti,j721e-csi2rx.yaml
    > > - Add an entry in MAINTAINERS.
    > > - Add a description for the binding.
    > > - Change compatible to ti,j721e-csi2rx to make it SoC specific.
    > > - Remove description from dmas, reg, power-domains.
    > > - Remove a limit of 2 from #address-cells and #size-cells.
    > > - Fix add ^ to csi-bridge subnode regex.
    > > - Make ranges mandatory.
    > > - Add unit address in example.
    > > - Add a reference to cdns,csi2rx in csi-bridge subnode.
    > > - Expand the example to include the csi-bridge subnode as well.
    > > - Re-order subject prefixes.
    > > - Convert OV5640 to use runtime PM and drop Cadence CSI2RX s_power
    > > patch.
    > > - Drop subdev call wrappers from cdns-csi2rx.
    > > - Move VPE and CAL to a separate subdir.
    > > - Rename ti-csi2rx.c to j721e-csi2rx.c
    > >
    > > Pratyush Yadav (13):
    > >   media: cadence: csi2rx: Unregister v4l2 async notifier
    > >   media: cadence: csi2rx: Cleanup media entity properly
    > >   media: cadence: csi2rx: Add get_fmt and set_fmt pad ops
    > >   media: cadence: csi2rx: Add external DPHY support
    > >   media: cadence: csi2rx: Soft reset the streams before starting
    > > capture
    > >   media: cadence: csi2rx: Set the STOP bit when stopping a stream
    > >   media: cadence: csi2rx: Fix stream data configuration
    > >   media: cadence: csi2rx: Populate subdev devnode
    > >   media: cadence: csi2rx: Add link validation
    > >   media: ti: Add CSI2RX support for J721E
    > >   media: dt-bindings: Make sure items in data-lanes are unique
    > >   media: dt-bindings: Add DT bindings for TI J721E CSI2RX driver
    > >   media: dt-bindings: Convert Cadence CSI2RX binding to YAML
    > >
    > >  .../devicetree/bindings/media/cdns,csi2rx.txt |  100 --
    > >  .../bindings/media/cdns,csi2rx.yaml           |  176 +++
    > >  .../bindings/media/ti,j721e-csi2rx.yaml       |  101 ++
    > >  .../bindings/media/video-interfaces.yaml      |    1 +
    > >  MAINTAINERS                                   |    7 +
    > >  drivers/media/platform/cadence/cdns-csi2rx.c  |  273 ++++-
    > >  drivers/media/platform/ti/Kconfig             |   12 +
    > >  drivers/media/platform/ti/Makefile            |    1 +
    > >  .../media/platform/ti/j721e-csi2rx/Makefile   |    2 +
    > >  .../platform/ti/j721e-csi2rx/j721e-csi2rx.c   | 1022
    > > +++++++++++++++++
    > >  10 files changed, 1580 insertions(+), 115 deletions(-)
    > >  delete mode 100644
    > > Documentation/devicetree/bindings/media/cdns,csi2rx.txt
    > >  create mode 100644
    > > Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
    > >  create mode 100644 Documentation/devicetree/bindings/media/ti,j721e-
    > > csi2rx.yaml
    > >  create mode 100644 drivers/media/platform/ti/j721e-csi2rx/Makefile
    > >  create mode 100644 drivers/media/platform/ti/j721e-csi2rx/j721e-
    > > csi2rx.c
    > >
    >

    Thanks,
    Jai
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2023-03-28 08:02    [W:2.119 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site