Hi Paul, Hi Michael
On Wed, Oct 25, 2023 at 11:54:27AM +0200, Paul Kocialkowski wrote:
> Michael,
>
> On Wed 25 Oct 23, 11:17, Michael Riesch wrote:
> > Hi Paul,
> >
> > On 10/25/23 10:43, Paul Kocialkowski wrote:
> > > [...]
> > >>> Here is the detail of my research on the concerned chips. The + at the beginning
> > >>> of the line indicate support in Rockchip's 4.4 tree:
> > >>>
> > >>> - RK3566/RK3568 (2020): CIF pins + VICAP terminology
> > >>> + RK1808 (2019): CIF pins + VIP registers + VIP_MIPI registers
> > >>> + PX30 (2017): VIP pins + VIP registers
> > >>> + RK3328 (2017): CIF pins + VIP terminology
> > >>> - RK3326 (2017): CIF pins + VIP terminology
> > >>> - RK3399 (2016): CIF pins
> > >>> - RK3368 (2015): CIF pins
> > >>> - PX2 (2014-11): CIF pins + CIF registers
> > >>> + RK3126/RK3128 (2014-10): CIF pins + registers
> > >>> + RK3288 (2014-05): CIF pins + VIP terminology
> > >>> - RK3026 (2013): CIF pins + CIF registers
> > >>> - RK3168/RK3188/PX3 (2012): CIF pins + CIF registers
> > >>> - RK3066 (2012): CIF pins + CIF registers
> > >>>
> > >>> Note that there are a few variations over time (added/removed registers), but
> > >>> the offsets of crucial registers are always the same, so we can safely
> > >>> assume this is the same unit in different generations.
> > >>>
> > >>> Since the RK3066 is the first model starting the RK30 lineup I think we can
> > >>> safely use that for the "base" compatible to be used for e.g. the bindings
> > >>> document, instead of px30 which is just one of the many SoCs that use this unit.
> > >>
> > >> Once the name of the driver is defined and adjusted in v9, I can try to
> > >> give the series a shot on my RK3568 board. First attempts to do so
This sounds good!
> > >> basing on Maxime's v5 showed that with a few modifications the DVP
> > >> feature works fine. In a subsequent step, we could discuss the inclusion
> > >> of the MIPI CSI-2 things in order to keep the driver sufficiently general.
> > >
> > > Nice! I guess there will be a need to introduce a variant structure associated
> > > to each compatible to express the differences betweens these different
> > > generations.
> >
> > Indeed. If Mehdi and you suggest something, I'd be happy to review.
>
> Well the be honest the scope of work on our side is really centered on PX30
> and merging this first version.
>
> > Otherwise, I'll try to come up with something reasonable. IMHO it would
> > make sense (as a first step) to have the clocks and the resets in this
> > structure, as well as a sub-structure that describes the DVP. The latter
> > consists of registers mainly, but maybe supported input/output formats
> > and other things should go in there as well. Also, downstream code has a
> > significant number of
> > if (some condition including chip_id) A; else B;
> > things that we should probably get rid of with this variant structure.
>
> Indeed I think we want to try avoid that. Another common option is to define
> capability flags to represent differences between generations in a more
> practical and clean way than explicitly checking chip ids or so.
>
> > As next step, a sub-structure for MIPI CSI-2 could be defined. RK356X
> > will have one of those, RK3588 will feature even six of them. So we
> > should add a const array to the variant structure.
> >
> > > Note that we will also probably need to convert the driver over to a MC-centric
> > > approach, but this is of course outside of the scope of this series.
> >
> > That would absolutely make sense. What is missing, though? (I was
> > wondering that the driver calls media_device_(un)register but no
> > /dev/mediaX device pops up.)
>
> Switching from video node-centric to MC-centric is more of a semantic change.
> In the first case we expect that subdevs are configured by the video device
> driver and userspace is not expected to change anything in the media topology
> or to configure media entities explicitly.
>
> In the latter case it's the opposite : the driver should never try to push
> configuration to a subdev and should instead validate that the current
> configuration makes sense.
>
> Still, I believe should be a media device registered and visible to userspace.
> Mehdi could you take a look at this? Do you see a media device in `media-ctl -p`
> and /dev/mediaX?
Yes I do have a media device
media-ctl -p
Media device information
------------------------
driver rockchip-cif
model rk_cif
serial
bus info platform:ff490000.video-capture
hw revision 0x0
driver version 6.6.0
Device topology
- entity 1: rockchip_cif (1 pad, 1 link)
type Node subtype V4L flags 0
device node name /dev/video0
pad0: Sink
<- "tw9900 2-0044":0 [ENABLED]
- entity 5: tw9900 2-0044 (1 pad, 1 link)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev0
pad0: Source
[fmt:UYVY8_2X8/720x480 field:none colorspace:smpte170m xfer:709 ycbcr:601]
-> "rockchip_cif":0 [ENABLED]
>
> Cheers,
>
> Paul
>
> > Best regards,
> > Michael
> >
> > >
> > > Cheers,
> > >
> > > Paul
> > >
> > >> @Mehdi: If you could Cc: me when you send out v9 it'd be much appreciated.
Of course I will :)
--
Kind Regards
Mehdi Djait
