[bug] omap3isp: missing support for ENUM_FMT

[Pavel Machek]

Hi!

Currently, ispvideo.c does not support enum_format. This causes
problems for example for libv4l2.

Now, I'm pretty sure patch below is not the right fix. But it fixes
libv4l2 problem for me.

Pointer to right solution welcome.

Regards,
									Pavel

diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
index 218e6d7..2ce0327 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -772,6 +772,44 @@ isp_video_try_format(struct file *file, void *fh, struct v4l2_format *format)
 }
 
 static int
+isp_video_enum_format(struct file *file, void *fh, struct v4l2_fmtdesc *format)
+{
+	struct isp_video *video = video_drvdata(file);
+	struct v4l2_subdev_format fmt;
+	struct v4l2_subdev *subdev;
+	u32 pad;
+	int ret;
+
+	printk("ispvideo: enum_fmt\n");
+
+	subdev = isp_video_remote_subdev(video, &pad);
+	if (subdev == NULL) {
+		printk("No subdev\n");
+		//return -EINVAL;
+	}
+
+	//isp_video_pix_to_mbus(&format->fmt.pix, &fmt.format);
+	if (format->index)
+		return -EINVAL;
+	format->type = video->type;
+	format->flags = 0;
+	strcpy(format->description, "subdev description");
+	format->pixelformat = V4L2_PIX_FMT_SGRBG10;
+
+	printk("Returning SRGBG10\n");
+#if 0	
+	fmt.pad = pad;
+	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
+	if (ret)
+		return ret == -ENOIOCTLCMD ? -ENOTTY : ret;
+
+	isp_video_mbus_to_pix(video, &fmt.format, &format->fmt.pix);
+#endif
+	return 0;
+}
+
+static int
 isp_video_get_selection(struct file *file, void *fh, struct v4l2_selection *sel)
 {
 	struct isp_video *video = video_drvdata(file);
@@ -1276,6 +1314,7 @@ static const struct v4l2_ioctl_ops isp_video_ioctl_ops = {
 	.vidioc_g_fmt_vid_cap		= isp_video_get_format,
 	.vidioc_s_fmt_vid_cap		= isp_video_set_format,
 	.vidioc_try_fmt_vid_cap		= isp_video_try_format,
+	.vidioc_enum_fmt_vid_cap        = isp_video_enum_format,
 	.vidioc_g_fmt_vid_out		= isp_video_get_format,
 	.vidioc_s_fmt_vid_out		= isp_video_set_format,
 	.vidioc_try_fmt_vid_out		= isp_video_try_format,

On Sat 2017-03-04 20:44:50, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Saturday 04 Mar 2017 17:39:46 Sakari Ailus wrote:
> > On Sat, Mar 04, 2017 at 03:03:18PM +0200, Sakari Ailus wrote:
> > > On Thu, Mar 02, 2017 at 01:38:48PM +0100, Pavel Machek wrote:
> > >> 
> > >>>> Ok, how about this one?
> > >>>> omap3isp: add rest of CSI1 support
> > >>>> 
> > >>>> CSI1 needs one more bit to be set up. Do just that.
> > >>>> 
> > >>>> It is not as straightforward as I'd like, see the comments in the
> > >>>> code for explanation.
> > >>
> > >> ...
> > >> 
> > >>>> +	if (isp->phy_type == ISP_PHY_TYPE_3430) {
> > >>>> +		struct media_pad *pad;
> > >>>> +		struct v4l2_subdev *sensor;
> > >>>> +		const struct isp_ccp2_cfg *buscfg;
> > >>>> +
> > >>>> +		pad = media_entity_remote_pad(&ccp2
> > >>>> ->pads[CCP2_PAD_SINK]);
> > >>>> +		sensor = media_entity_to_v4l2_subdev(pad->entity);
> > >>>> +		/* Struct isp_bus_cfg has union inside */
> > >>>> +		buscfg = &((struct isp_bus_cfg *)sensor->host_priv)
> > >>>> ->bus.ccp2;
> > >>>> +
> > >>>> +		csiphy_routing_cfg_3430(&isp->isp_csiphy2,
> > >>>> +					ISP_INTERFACE_CCP2B_PHY1,
> > >>> > +					enable, !!buscfg->phy_layer,
> > >>> > +					buscfg->strobe_clk_pol);
> > >>> 
> > >>> You should do this through omap3isp_csiphy_acquire(), and not call
> > >>> csiphy_routing_cfg_3430() directly from here.
> > >> 
> > >> Well, unfortunately omap3isp_csiphy_acquire() does have csi2
> > >> assumptions hard-coded :-(.
> > >> 
> > >> This will probably fail.
> > >> 
> > >> 	        rval = omap3isp_csi2_reset(phy->csi2);
> > >> 	        if (rval < 0)
> > >> 		                goto done;
> > > 
> > > Could you try to two patches I've applied on the ccp2 branch (I'll remove
> > > them if there are issues).
> > > 
> > > That's compile tested for now only.
> > 
> > One more thing. What's needed for configuring the PHY for CCP2?
> > 
> > For instance, is the CSI-2 PHY regulator still needed in
> > omap3isp_csiphy_acquire()? One way to do this might go to see the original
> > driver for N900; I don't have the TRM at hand right now.
> 
> The OMAP34xx TRM and data manual both mention separate VDDS power supplies for 
> the CSIb and CSI2 I/O complexes.
> 
> vdds_csi2		CSI2 Complex I/O
> vdds_csib		CSIb Complex I/O
> 
> On OMAP36xx, we instead have
> 
> vdda_csiphy1		Input power for camera PHY buffer
> vdda_csiphy2		Input power for camera PHY buffer
> 
> We need to enable the vds_csib regulator to operate the CSI1/CCP2 PHY, but 
> that regulator gets enabled in ispccp2.c as that module is powered by the 
> vdds_csib supply on OMAP34xx. However, it won't hurt to do so, and the code 
> could be simpler if we manage the regulators the same way on OMAP34xx and 
> OMAP36xx.
> 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Attachment: signature.asc
Description: Digital signature