public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Philippe De Muyter <phdm@macq.eu>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Steve Longerbeam <steve_longerbeam@mentor.com>,
	Tim Harvey <tharvey@gateworks.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	linux-media <linux-media@vger.kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>
Subject: Re: i.mx6 camera interface (CSI) and mainline kernel
Date: Mon, 14 Mar 2016 15:55:25 +0100	[thread overview]
Message-ID: <20160314145525.GA21719@frolo.macqel> (raw)
In-Reply-To: <56E1227A.9020705@xs4all.nl>

Hi Steve and all,

I am busy porting to Steve's subdev version a driver that I had written
in intdev style, for the freescale imx6 linux version.  And I have a
problem :

My previous driver had the following ioctl function, which used
the V4L2_FRMIVAL_TYPE_CONTINUOUS type answer.

static int ioctl_enum_frameintervals(struct v4l2_int_device *s,
                                         struct v4l2_frmivalenum *fival)
{
        if (fival->index)
                return -EINVAL;
        fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
        fival->stepwise.min.numerator = 1;
        fival->stepwise.min.denominator = MAX_FPS;
        fival->stepwise.max.numerator = 1;
        fival->stepwise.max.denominator = MIN_FPS;
        fival->stepwise.step.numerator = 1;
        fival->stepwise.step.denominator = 1;
        return 0;
}

Now I see that Steve's related ioctl, using the 'enum_frame_interval'
pad function is :

static int vidioc_enum_frameintervals(struct file *file, void *priv,
                                      struct v4l2_frmivalenum *fival)
{
        struct v4l2_subdev_frame_interval_enum fie ...;
	...

        ret = v4l2_subdev_call(dev->ep->sd, pad, enum_frame_interval,
                               NULL, &fie);

        fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
        fival->discrete = fie.interval;
        return 0;
}

where type is arbitrarily set to V4L2_FRMIVAL_TYPE_DISCRETE, because of the
limitation of the struct v4l2_subdev_frame_interval_enum, which has no 'type',
field nor a 'stepwise' field.

How can I do to continue to provide de V4L2_FRMIVAL_TYPE_CONTINUOUS answer ?

TIA

Philippe

On Thu, Mar 10, 2016 at 08:30:02AM +0100, Hans Verkuil wrote:
> On 03/10/2016 01:12 AM, Steve Longerbeam wrote:
> > On 03/09/2016 02:44 PM, Tim Harvey wrote:
> >> On Tue, Mar 8, 2016 at 6:06 PM, Steve Longerbeam
> >> <steve_longerbeam@mentor.com> wrote:
> >>> On 03/07/2016 08:19 AM, Tim Harvey wrote:
> >> <snip>
> >>>
> >>> Hi Tim, good to hear it works for you on the Ventana boards.
> >>>
> >>> I've just pushed some more commits to the mx6-media-staging branch that
> >>> get the drivers/media/i2c/adv7180.c subdev working with the imx6 capture
> >>> backend. Images look perfect when switching to UYVY8_2X8 mbus code instead
> >>> of YUYV8_2X8. But I'm holding off on that change because this subdev is used
> >>> by Renesas targets and would likely corrupt captured images for those
> >>> targets. But I believe UYVY is the correct transmit order according to the
> >>> BT.656 standard.
> >>>
> >>> Another thing that should also be changed in drivers/media/i2c/adv7180.c
> >>> is the field type. It should be V4L2_FIELD_SEQ_TB for NTSC and V4L2_FIELD_SEQ_BT
> >>> for PAL.
> >>>
> >>> Steve
> >>>
> >>>
> >> Steve,
> >>
> >> with your latest patches, I'm crashing with an null-pointer-deref in
> >> adv7180_set_pad_format. What is your kernel config for
> >> CONFIG_MEDIA_CONTROLLER and CONFIG_VIDEO_V4L2_SUBDEV_API?
> > 
> > Right, I thought I fixed that, I was passing a NULL pad_cfg for
> > TRY_FORMAT, but that was fixed. Maybe you fetched a version
> > of mx6-media-staging while I was in the middle of debugging?
> > Try fetching again.
> > 
> > I tried with both CONFIG_MEDIA_CONTROLLER and
> > CONFIG_VIDEO_V4L2_SUBDEV_API enabled and both disabled, and
> > I don't get the null deref in adv7180_set_pad_format.
> > 
> > 
> >>
> >> Your tree contains about 16 or so patches on top of linux-media for
> >> imx6 capture. Are you close to the point where you will be posting a
> >> patch series? If so, please CC me for testing with the adv7180 sensor.
> > 
> > I guess I can try posting a series again, but there will likely be push-back from
> > Pengutronix. They have their own video capture driver for imx6. Last I heard (a while ago!)
> > their version did not implement scaling, colorspace conversion, or image rotation via
> > the IC. Instead their driver sends raw camera frames directly to memory, and image
> > conversion is carried out by separate mem2mem device. Our capture driver does
> > image conversion (scaling, CSC, and rotation) natively using the IC pre-processing channel.
> > We also have a mem2mem device that does conversion using IC post-processing,
> > which I have included in mx6-media-staging.
> > 
> > Also IIRC they did some pretty slick stuff with a video bus multiplexer subdev, which
> > can multiplex video from different sensors either using the internal mux in the SoC,
> > or can control an external mux via gpio. Our driver only supports the internal mux,
> > and does it with a platform data function.
> > 
> > But like I said, I don't what the latest status is of the Pengutronix video capture support.
> > 
> > Btw, I just pushed an update of mx6-media-staging that implements vidioc_[gs]_selection.
> 
> Steve & Pengutronix,
> 
> I would be happy to add drivers to staging, either Steve's, Pengutronix or both.
> 
> The i.mx6 is quite popular and the lack of video capture support in the kernel is
> a big stumbling block, especially given the pile of crap that freescale provides.
> 
> Having decent code in the kernel will help progress a lot I hope.
> 
> Regards,
> 
> 	Hans

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

  reply	other threads:[~2016-03-14 14:55 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-23 11:49 i.mx6 camera interface (CSI) and mainline kernel Philippe De Muyter
2016-02-23 14:12 ` Philippe De Muyter
2016-02-25 22:05   ` Laurent Pinchart
2016-03-03  2:02     ` Steve Longerbeam
2016-03-03  7:19       ` Hans Verkuil
2016-03-03  8:36         ` Philippe De Muyter
2016-03-03 17:45           ` Steve Longerbeam
2016-03-03 17:52             ` Philippe De Muyter
2016-03-07 16:19             ` Tim Harvey
2016-03-09  2:06               ` Steve Longerbeam
2016-03-09 22:44                 ` Tim Harvey
2016-03-10  0:12                   ` Steve Longerbeam
2016-03-10  7:30                     ` Hans Verkuil
2016-03-14 14:55                       ` Philippe De Muyter [this message]
2016-03-15 20:54                     ` Tim Harvey
2016-06-02 13:29                     ` Tim Harvey
2016-06-02 13:55                       ` Hans Verkuil
2016-06-02 16:50                         ` Steve Longerbeam
2016-06-06 16:48                           ` Steve Longerbeam
2016-06-10 15:58                             ` Jack Mitchell
2016-06-10 16:34                               ` Steve Longerbeam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160314145525.GA21719@frolo.macqel \
    --to=phdm@macq.eu \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=steve_longerbeam@mentor.com \
    --cc=tharvey@gateworks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox