Request your Partnership for a legal business Deal, reply if interested
-- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Feb 1 05:00:23 CET 2017 media-tree git hash:e7b3a2b22176d01db0c3b31d6389ccf542ba1967 media_build git hash: 3c6ce4ff75f19adf45869e34b376c5b9dee4d50a v4l-utils git hash: 9df320dd3d1a498fcd6cdeef7d783da609b526e0 gcc version:i686-linux-gcc (GCC) 6.2.0 sparse version: v0.5.0-3553-g78b2ea6 smatch version: v0.5.0-3553-g78b2ea6 host hardware: x86_64 host os:4.8.0-164 linux-git-arm-at91: ERRORS linux-git-arm-davinci: ERRORS linux-git-arm-multi: ERRORS linux-git-arm-pxa: OK linux-git-blackfin-bf561: ERRORS linux-git-i686: OK linux-git-m32r: OK linux-git-mips: ERRORS linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: ERRORS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-3.10.1-i686: ERRORS linux-3.11.1-i686: ERRORS linux-3.12.67-i686: ERRORS linux-3.13.11-i686: ERRORS linux-3.14.9-i686: ERRORS linux-3.15.2-i686: ERRORS linux-3.16.7-i686: ERRORS linux-3.17.8-i686: ERRORS linux-3.18.7-i686: ERRORS linux-3.19-i686: ERRORS linux-4.0.9-i686: ERRORS linux-4.1.33-i686: ERRORS linux-4.2.8-i686: ERRORS linux-4.3.6-i686: ERRORS linux-4.4.22-i686: ERRORS linux-4.5.7-i686: ERRORS linux-4.6.7-i686: ERRORS linux-4.7.5-i686: ERRORS linux-4.8-i686: ERRORS linux-4.9-i686: ERRORS linux-4.10-rc3-i686: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12.67-x86_64: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.9-x86_64: ERRORS linux-3.15.2-x86_64: ERRORS linux-3.16.7-x86_64: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.7-x86_64: ERRORS linux-3.19-x86_64: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.33-x86_64: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-x86_64: ERRORS linux-4.4.22-x86_64: ERRORS linux-4.5.7-x86_64: ERRORS linux-4.6.7-x86_64: ERRORS linux-4.7.5-x86_64: ERRORS linux-4.8-x86_64: ERRORS linux-4.9-x86_64: ERRORS linux-4.10-rc3-x86_64: ERRORS apps: WARNINGS spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/24] i.MX Media Driver
On 01/31/2017 04:23 PM, Russell King - ARM Linux wrote: On Tue, Jan 31, 2017 at 03:43:22PM -0800, Steve Longerbeam wrote: On 01/31/2017 03:00 AM, Russell King - ARM Linux wrote: Just like smiapp, the camera sensor block (which is the very far end of the pipeline) is marked with MEDIA_ENT_F_CAM_SENSOR. However, in front of that is the binner, which just like smiapp gets a separate entity. It's this entity which is connected to the mipi-csi2 subdev. wow, ok got it. So the sensor pipeline and binner, and the OF graph connecting them, are described in the device tree I presume. No - because the binner and sensor are on the same die, it's even one single device, there's no real separation of the two devices. The reason there's no real separation is because the binning is done as part of the process of reading the array - sometimes before the analog voltage is converted to its digital pixel value representation. The separation comes because of the requirements of the v4l2 media subdevs, which requires scalers to have a sink pad and a source pad. (Please see the v4l2 documentation, I think I've already quoted this: .. _MEDIA-ENT-F-PROC-VIDEO-SCALER: - ``MEDIA_ENT_F_PROC_VIDEO_SCALER`` - Video scaler. An entity capable of video scaling must have at least one sink pad and one source pad, and scale the video frame(s) received on its sink pad(s) to a different resolution output on its source pad(s). The range of supported scaling ratios is entity-specific and can differ between the horizontal and vertical directions (in particular scaling can be supported in one direction only). Binning and skipping are considered as scaling. (Oh yes, I see it was the mail to which you were replying to...) So, in order to configure the scaling (which can be none, /2 and /4) we have to expose two subdevs - one which is the scaler, and has a source pad connected to the upstream (in this case CSI2 receiver) and a sink pad immutably connected to the camera sensor. Since the split is entirely down to the V4L2 implementation requirements, it's not something that should be reflected in DT - we don't put implementation details in DT. It's just the same (as I've already said) as the SMIAPP camera driver, the reason I'm pointing you towards that is because this is an already mainlined camera driver which nicely illustrates how my driver is structured from the v4l2 subdev API point of view. The OF graph AFAIK, has no information about which ports are sinks and which are sources, so of_parse_subdev() tries to determine that based on the compatible string of the device node. So ATM of_parse_subdev() assumes there is nothing but the imx6-mipi-csi2, video-multiplexer, and camera sensors upstream from the CSI ports in the OF graph. I realize that's not a robust solution, and is the reason for the "no sensor attached" below. Is there any way to determine from the OF graph the data-direction of a port (whether it is a sink or a source)? If so it will make of_parse_subdev() much more robust. I'm not sure why you need to know the data direction. First, thank you for the explanation, it clears up a lot. But of_parse_subdev() is used to parse the OF graph starting from the CSI ports, to discover all the nodes to add to subdev async registration. It also forms the media link info to be used later to create the media graph, after all discovered subdevs have come online (registered themselves). This happens at driver load time, it doesn't have anything to do with pad negotiation. I think the issue here is how you're going about dealing with the subdevs. Here's the subdev setup: +-camera+ | pixel array -> binner |---> csi2 --> ipu1csi0 mux --> csi0 --> smfc --> idmac +---+ How the subdevs are supposed to work is that you start from the first subdev in sequence (in this case the pixel array) and negotiate with the driver through the TRY formats on its source pad, as well as negotiating with the sink pad of the directly neighbouring subdev. The neighbouring subdev propagates the configuration in a driver specific manner from its source pad to the sink pad, giving a default configuration at its source. Let me try to re-phrase. You mean the subdev's set_fmt(), when configured its source pad(s), should call set_fmt() at the connected sink subdev to automatically propagate the format to the sink's pad? This process repeats throughout the chain all the way up to the bridge device. Now, where things go wrong is that you want to know what each type of these subdevs are, and the reason you want that is so you can do this (for example - I know similar stuff happens with the "sensor" stuff further up the chain as well): +-camera+ | pixel array -> binner |---> csi2 --> ipu1csi0 mux --> csi0 --> smfc --> idmac +---+|
Re: [PATCH v3 17/24] media: imx: Add CSI subdev driver
On 01/31/2017 08:48 AM, Philipp Zabel wrote: On Tue, 2017-01-31 at 16:51 +0100, Philipp Zabel wrote: On Fri, 2017-01-06 at 18:11 -0800, Steve Longerbeam wrote: [...] +static int csi_set_fmt(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_format *sdformat) +{ + struct csi_priv *priv = v4l2_get_subdevdata(sd); + struct v4l2_mbus_framefmt *infmt, *outfmt; + struct v4l2_rect crop; + int ret; + + if (sdformat->pad >= CSI_NUM_PADS) + return -EINVAL; + + if (priv->stream_on) + return -EBUSY; + + infmt = >format_mbus[priv->input_pad]; + outfmt = >format_mbus[priv->output_pad]; + + if (sdformat->pad == priv->output_pad) { + sdformat->format.code = infmt->code; + sdformat->format.field = infmt->field; + crop.left = priv->crop.left; + crop.top = priv->crop.top; + crop.width = sdformat->format.width; + crop.height = sdformat->format.height; + ret = csi_try_crop(priv, ); This is the wrong way around, see also below. Here the the output sdformat->format.width/height should be set to the priv->crop.width/height (or priv->crop.width/height / 2, to enable downscaling). The crop rectangle should not be changed by an output pad set_fmt. [...] The crop rectangle instead should be reset to the full input frame when the input pad format is set. How about this: Thanks for the patch, I'll try it tomorrow. Steve --8<-- diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index 5e80a0871b139..8220e4204a125 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -484,6 +484,8 @@ static int csi_setup(struct csi_priv *priv) if_fmt.field = outfmt->field; ipu_csi_set_window(priv->csi, >crop); + ipu_csi_set_downsize(priv->csi, priv->crop.width == 2 * outfmt->width, + priv->crop.height == 2 * outfmt->height); ipu_csi_init_interface(priv->csi, _mbus_cfg, _fmt); @@ -830,15 +832,14 @@ static int csi_set_fmt(struct v4l2_subdev *sd, switch (sdformat->pad) { case CSI_SRC_PAD_DIRECT: case CSI_SRC_PAD_IDMAC: - crop.left = priv->crop.left; - crop.top = priv->crop.top; - crop.width = sdformat->format.width; - crop.height = sdformat->format.height; - ret = csi_try_crop(priv, , sensor); - if (ret) - return ret; - sdformat->format.width = crop.width; - sdformat->format.height = crop.height; + if (sdformat->format.width < priv->crop.width * 3 / 4) + sdformat->format.width = priv->crop.width / 2; + else + sdformat->format.width = priv->crop.width; + if (sdformat->format.height < priv->crop.height * 3 / 4) + sdformat->format.height = priv->crop.height / 2; + else + sdformat->format.height = priv->crop.height; if (sdformat->pad == CSI_SRC_PAD_IDMAC) { cc = imx_media_find_format(0, sdformat->format.code, @@ -887,6 +888,14 @@ static int csi_set_fmt(struct v4l2_subdev *sd, } break; case CSI_SINK_PAD: + crop.left = 0; + crop.top = 0; + crop.width = sdformat->format.width; + crop.height = sdformat->format.height; + ret = csi_try_crop(priv, , sensor); + if (ret) + return ret; + cc = imx_media_find_format(0, sdformat->format.code, true, false); if (!cc) { @@ -904,9 +913,8 @@ static int csi_set_fmt(struct v4l2_subdev *sd, } else { priv->format_mbus[sdformat->pad] = sdformat->format; priv->cc[sdformat->pad] = cc; - /* Update the crop window if this is an output pad */ - if (sdformat->pad == CSI_SRC_PAD_DIRECT || - sdformat->pad == CSI_SRC_PAD_IDMAC) + /* Reset the crop window if this is the input pad */ + if (sdformat->pad == CSI_SINK_PAD) priv->crop = crop; } -->8-- regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: rtl2832_sdr and /dev/swradio0
Le mardi 31 janvier 2017 à 17:50 +, Russel Winder a écrit : > Hi, > > Is anyone actively working on the rtl2832_sdr driver? > > I am particularly interested in anyone who has code for turning the > byte stream from /dev/swradio0 into an ETI stream. Or failing that > getting enough data about the API for using /dev/swradio0 so as to > write a byte sequence to ETI driver based on the dab2eti program in > DABtool (which uses the librtlsdr mechanism instead of the > /dev/swradio0 one). This device works like any V4L2 driver, with the differences explained here: https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/dev-sdr.html For the rest I'm no expert. You can probably port dab2eti to use this interface instead of librtlsdr and keep the rest. You may be able to skip an fft if your driver supports it, otherwise you'll get RU12, that you'll probably have to convert to floats before passing to the rest of the processing code. regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v3 00/24] i.MX Media Driver
On 01/31/2017 05:54 AM, Philipp Zabel wrote: Hi Steve, I have just tested the imx-media-staging-md-wip branch on a Nitrogen6X with a tc358743 (BD_HDMI_MIPI HDMI to MIPI CSI-2 receiver board). Some observations: # Link pipeline media-ctl -l "'tc358743 1-000f':0->'imx6-mipi-csi2':0[1]" media-ctl -l "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]" media-ctl -l "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]" media-ctl -l "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]" # Provide an EDID to the HDMI source v4l2-ctl -d /dev/v4l-subdev2 --set-edid=file=edid-1080p.hex # At this point the HDMI source is enabled and sends a 1080p60 signal # Configure detected DV timings media-ctl --set-dv "'tc358743 1-000f':0" # Set pad formats media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]" media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]" media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]" media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080]" v4l2-ctl -d /dev/video4 -V # This still is configured to 640x480, which is inconsistent with # the 'ipu1_csi0':2 pad format. The pad set_fmt above should # have set this, too. Because you've only configured the source pads, and not the sink pads. The ipu_csi source format is dependent on the sink format - output crop window is limited by max input sensor frame, and since sink pad is still at 640x480, output is reduced to that. Maybe I'm missing something, is it expected behavior that a source format should be automatically propagated to the sink? v4l2-ctl --list-formats -d /dev/video4 # This lists all the RGB formats, which it shouldn't. There is # no CSC in this pipeline, so we should be limited to YUV formats # only. right, need to fix that. Probably by poking the attached source subdev (csi or prpenc/vf) for its supported formats. # Set capture format v4l2-ctl -d /dev/video4 -v width=1920,height=1080,pixelformat=UYVY v4l2-ctl -d /dev/video4 -V # Now the capture format is correctly configured to 1920x1080. v4l2-ctl -d 4 --list-frameintervals=width=1920,height=1080,pixelformat=UYVY # This lists nothing. We should at least provide the 'ipu1_csi0':2 pad # frame interval. In the future this should list fractions achievable # via frame skipping. yes, need to implement g_frame_interval. v4l2-compliance -d /dev/video4 # This fails two tests: # fail: v4l2-test-input-output.cpp(383): std == 0 # fail: v4l2-test-input-output.cpp(449): invalid attributes for input 0 # test VIDIOC_G/S/ENUMINPUT: FAIL # and # fail: v4l2-test-controls.cpp(782): subscribe event for control 'User Controls' failed # test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL # (Slowly) stream JPEG images to a display host: gst-launch-1.0 -v v4l2src device=/dev/video4 ! jpegenc ! rtpjpegpay ! udpsink I've done this a few times, and sometimes I only get a "ipu1_csi0: EOF timeout" message when starting streaming. It's hard to say what is going on there, it would be great if I could get my hands on a Nitrogen6X with the tc35874 to help you debug. Steve -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver
Hi Steve, On Tue, Jan 31, 2017 at 05:02:40PM -0800, Steve Longerbeam wrote: > But this also puts a requirement on MIPI sensors that s_power(ON) > should only place the D_PHY in LP-11, and _not_ start the clock lane. > But perhaps that is correct behavior anyway. If the CSI2 DPHY is held in reset state, it shouldn't matter what the sensor does. In the case of IMX219, it needs a full setup of the device, including enabling it to stream (so it starts the clock lane etc) in order to get it into LP-11 state. Merely disabling the XCLR signal leaves the lanes grounded. I do seem to remember reading in one of the MIPI specs that this is rather expected behaviour, though I can't point at a paragraph this late in the night. So, the only way to satisfy these requirements is this order: - assert PHY reset signals (so blocking any activity on the CSI lanes) - initialise the sensor (including allowing it to start streaming and then stopping the stream - at that point, the lanes will be in LP-11.) - deassert the resets as per the iMX6 documentation and follow the remaining procedure. I'll look at your other points tomorrow. Thanks. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver
On 01/31/2017 01:49 AM, Philipp Zabel wrote: On Tue, 2017-01-31 at 00:01 +, Russell King - ARM Linux wrote: [...] The iMX6 manuals call for a very specific seven sequence of initialisation for CSI2, which begins with: 1. reset the D-PHY. 2. place MIPI sensor in LP-11 state 3. perform D-PHY initialisation 4. configure CSI2 lanes and de-assert resets and shutdown signals Since you reset the CSI2 at power up and then release it, how do you guarantee that the published sequence is followed? Hi Russell, In "40.3.1 Startup Sequence", it states that step 1 is to "De-assert CSI2 presetn signal (global reset)". I can't find any description of this signal in the manual, that statement is the only mention of it. I don't know if this the D-PHY reset signal, it sounds more like CSI2_RESETN (CSI-2 host controller reset). In any case, I re-reviewed the published sequence in the manual, and it does look like there are a couple problems. The pipeline power up sequence in imx-media driver is as follows: s_power(ON) op is called first on the imx6-mipi-csi2, in which CSI2 and D-PHY resets are asserted and then de-asserted via the CSI2_RESETN and CSI2_DPHY_RSTZ registers, the D-PHY is initialized, and lanes set. At this point the MIPI sensor may be powered down (in fact, in OV5640 case, the PWDN pin is asserted). So there could be a problem here, I don't think the D-PHY is considered in the LP-11 stop state when the D-PHY master is powered off :). A fix might simply be to reverse power on, sensor first so that it can be placed in LP-11, then imx6-mipi-csi2. The following steps are carried out by s_stream() calls. Sensor s_stream(ON) is called first which starts a clock on the clock lane. Then imx6-mipi-csi2 s_stream(ON) in which the PHY_STATE register is polled to confirm the D-PHY stop state, then looks for active clock on lock lane. There could be a problem there too. Again should be fixed simply by calling stream-on on the imx6-mipi-csi2 first, then sensor. So I will try the following sequence: 1. sensor power on (put D-PHY in LP-11 stop state). 2. csi-2 power on (deassert CSI2 and D-PHY resets, D-PHY init, verify LP-11). 3. sensor stream on (starts clock on clock lane). 4. csi-2 stream on (confirm clock on clock lane). That comes closest to meeting the sequence requirements. But this also puts a requirement on MIPI sensors that s_power(ON) should only place the D_PHY in LP-11, and _not_ start the clock lane. But perhaps that is correct behavior anyway. Steve With Philipp's driver, this is easy, because there is a prepare_stream callback which gives the sensor an opportunity to get everything correctly configured according to the negotiated parameters, and place the sensor in LP-11 state. Some sensors do not power up in LP-11 state, but need to be programmed fully before being asked to momentarily stream. Only at that point is the sensor guaranteed to be in the required LP-11 state. Do you expect that 1. and 2. could depend on the negotiated parameters in any way on some hardware? I had removed the prepare_stream callback from my driver in v2 because for my use case at least the above sequence could be realized by 1. in imx-mipi-csi2 s_power(1) 2. in MIPI sensor s_power(1) 3./4. in imx-mipi-csi2 s_stream(1) 4. in MIPI sensor s_stream(1) as long as the sensor is correctly put back into LP-11 in s_stream(0). regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 17/24] media: imx: Add CSI subdev driver
On Tue, Jan 31, 2017 at 04:31:55PM -0800, Steve Longerbeam wrote: > > > On 01/31/2017 03:20 AM, Russell King - ARM Linux wrote: > >On Fri, Jan 06, 2017 at 06:11:35PM -0800, Steve Longerbeam wrote: > >>+static int csi_link_validate(struct v4l2_subdev *sd, > >>+struct media_link *link, > >>+struct v4l2_subdev_format *source_fmt, > >>+struct v4l2_subdev_format *sink_fmt) > >>+{ > >>+ struct csi_priv *priv = v4l2_get_subdevdata(sd); > >>+ bool is_csi2; > >>+ int ret; > >>+ > >>+ ret = v4l2_subdev_link_validate_default(sd, link, source_fmt, sink_fmt); > >>+ if (ret) > >>+ return ret; > >>+ > >>+ priv->sensor = __imx_media_find_sensor(priv->md, >sd.entity); > >>+ if (IS_ERR(priv->sensor)) { > >>+ v4l2_err(>sd, "no sensor attached\n"); > >>+ ret = PTR_ERR(priv->sensor); > >>+ priv->sensor = NULL; > >>+ return ret; > >>+ } > >>+ > >>+ ret = v4l2_subdev_call(priv->sensor->sd, video, g_mbus_config, > >>+ >sensor_mbus_cfg); > >>+ if (ret) > >>+ return ret; > >>+ > >>+ is_csi2 = (priv->sensor_mbus_cfg.type == V4L2_MBUS_CSI2); > >>+ > >>+ if (is_csi2) { > >>+ int vc_num = 0; > >>+ /* > >>+* NOTE! It seems the virtual channels from the mipi csi-2 > >>+* receiver are used only for routing by the video mux's, > >>+* or for hard-wired routing to the CSI's. Once the stream > >>+* enters the CSI's however, they are treated internally > >>+* in the IPU as virtual channel 0. > >>+*/ > >>+#if 0 > >>+ vc_num = imx_media_find_mipi_csi2_channel(priv->md, > >>+ >sd.entity); > >>+ if (vc_num < 0) > >>+ return vc_num; > >>+#endif > >>+ ipu_csi_set_mipi_datatype(priv->csi, vc_num, > >>+ >format_mbus[priv->input_pad]); > >This seems (at least to me) to go against the spirit of the subdev > >negotiation. Here, you seem to bypass the negotiation performed > >between the CSI and the attached device, wanting to grab the > >format from the CSI2 sensor directly. That bypasses negotiation > >performed at the CSI2 subdev and video muxes. > > This isn't so much grabbing a pad format, it is determining > which source pad at the imx6-mipi-csi2 receiver subdev is > reached from this CSI (which determines the virtual channel > the CSI is receiving). > > If there was a way to determine the vc# via a status register > in the CSI, that would be perfect, but there isn't. In fact, the CSIs > seem to be ignoring the meta-data bus sent by the CSI2IPU gasket > which contains this info, or that bus is not being routed to the CSIs. > As the note says, the CSIs only accept vc0 at the CSI_MIPI_DI register. > Any other value programmed there results in no data from the CSI. > > And even the presence of CSI_MIPI_DI register makes no sense to me, > the CSIs are given the vc# and MIPI datatype from the CSI2IPU meta-data > bus, so I don't understand why it needs to be told. The CSI_MIPI_DI register selects the data stream we want from the CSI2 camera. CSI2 cameras can produce many different data streams - for example, a CSI2 camera can, for the same image, produce a YUV encoded frame and a jpeg-encoded frame. These are sent over the CSI2 serial bus using two different data types. The CSI2IPU converts the serial data streams into a parallel data stream, feeding that into the CSI layer. The CSI layer, in conjunction with the CSI_MIPI_DI register, selects one of these streams to pass on to the SMFC and other blocks. >From what I've read, the CSI can also be programmed to pass other streams on as well, but I've never tried that. In my particular case, the IMX219 camera produces a complete frame using the RAW8 or RAW10 MIPI data type, and also produces two lines of register data per frame, encoded using another data type. I think it should be possible to program the CSI to pass this other data on as "generic data" through a different FIFO and have it written to memory, which makes it possible to know the camera settings for each frame. (This isn't something I'm interested in though, I'm just using it as an example of why, possibly, there's that register in the CSI block.) -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 17/24] media: imx: Add CSI subdev driver
On 01/31/2017 03:20 AM, Russell King - ARM Linux wrote: On Fri, Jan 06, 2017 at 06:11:35PM -0800, Steve Longerbeam wrote: +static int csi_link_validate(struct v4l2_subdev *sd, +struct media_link *link, +struct v4l2_subdev_format *source_fmt, +struct v4l2_subdev_format *sink_fmt) +{ + struct csi_priv *priv = v4l2_get_subdevdata(sd); + bool is_csi2; + int ret; + + ret = v4l2_subdev_link_validate_default(sd, link, source_fmt, sink_fmt); + if (ret) + return ret; + + priv->sensor = __imx_media_find_sensor(priv->md, >sd.entity); + if (IS_ERR(priv->sensor)) { + v4l2_err(>sd, "no sensor attached\n"); + ret = PTR_ERR(priv->sensor); + priv->sensor = NULL; + return ret; + } + + ret = v4l2_subdev_call(priv->sensor->sd, video, g_mbus_config, + >sensor_mbus_cfg); + if (ret) + return ret; + + is_csi2 = (priv->sensor_mbus_cfg.type == V4L2_MBUS_CSI2); + + if (is_csi2) { + int vc_num = 0; + /* +* NOTE! It seems the virtual channels from the mipi csi-2 +* receiver are used only for routing by the video mux's, +* or for hard-wired routing to the CSI's. Once the stream +* enters the CSI's however, they are treated internally +* in the IPU as virtual channel 0. +*/ +#if 0 + vc_num = imx_media_find_mipi_csi2_channel(priv->md, + >sd.entity); + if (vc_num < 0) + return vc_num; +#endif + ipu_csi_set_mipi_datatype(priv->csi, vc_num, + >format_mbus[priv->input_pad]); This seems (at least to me) to go against the spirit of the subdev negotiation. Here, you seem to bypass the negotiation performed between the CSI and the attached device, wanting to grab the format from the CSI2 sensor directly. That bypasses negotiation performed at the CSI2 subdev and video muxes. This isn't so much grabbing a pad format, it is determining which source pad at the imx6-mipi-csi2 receiver subdev is reached from this CSI (which determines the virtual channel the CSI is receiving). If there was a way to determine the vc# via a status register in the CSI, that would be perfect, but there isn't. In fact, the CSIs seem to be ignoring the meta-data bus sent by the CSI2IPU gasket which contains this info, or that bus is not being routed to the CSIs. As the note says, the CSIs only accept vc0 at the CSI_MIPI_DI register. Any other value programmed there results in no data from the CSI. And even the presence of CSI_MIPI_DI register makes no sense to me, the CSIs are given the vc# and MIPI datatype from the CSI2IPU meta-data bus, so I don't understand why it needs to be told. Anyway I can just yank this disabled code, it's only there for documentation purposes. The same goes for the frame rate monitoring code - imho, the frame rate should be something that results from negotiation with the neighbouring element, not by poking at some entity that is several entities away. I will fix this once the g_frame_interval op is implemented in every subdev. I believe you mentioned this already, but g_frame_interval will need to be chained until it reaches the originating sensor. Steve -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/24] i.MX Media Driver
On Tue, Jan 31, 2017 at 03:43:22PM -0800, Steve Longerbeam wrote: > > > On 01/31/2017 03:00 AM, Russell King - ARM Linux wrote: > >Just like smiapp, the camera sensor block (which is the very far end > >of the pipeline) is marked with MEDIA_ENT_F_CAM_SENSOR. However, in > >front of that is the binner, which just like smiapp gets a separate > >entity. It's this entity which is connected to the mipi-csi2 subdev. > > wow, ok got it. > > So the sensor pipeline and binner, and the OF graph connecting > them, are described in the device tree I presume. No - because the binner and sensor are on the same die, it's even one single device, there's no real separation of the two devices. The reason there's no real separation is because the binning is done as part of the process of reading the array - sometimes before the analog voltage is converted to its digital pixel value representation. The separation comes because of the requirements of the v4l2 media subdevs, which requires scalers to have a sink pad and a source pad. (Please see the v4l2 documentation, I think I've already quoted this: .. _MEDIA-ENT-F-PROC-VIDEO-SCALER: - ``MEDIA_ENT_F_PROC_VIDEO_SCALER`` - Video scaler. An entity capable of video scaling must have at least one sink pad and one source pad, and scale the video frame(s) received on its sink pad(s) to a different resolution output on its source pad(s). The range of supported scaling ratios is entity-specific and can differ between the horizontal and vertical directions (in particular scaling can be supported in one direction only). Binning and skipping are considered as scaling. (Oh yes, I see it was the mail to which you were replying to...) So, in order to configure the scaling (which can be none, /2 and /4) we have to expose two subdevs - one which is the scaler, and has a source pad connected to the upstream (in this case CSI2 receiver) and a sink pad immutably connected to the camera sensor. Since the split is entirely down to the V4L2 implementation requirements, it's not something that should be reflected in DT - we don't put implementation details in DT. It's just the same (as I've already said) as the SMIAPP camera driver, the reason I'm pointing you towards that is because this is an already mainlined camera driver which nicely illustrates how my driver is structured from the v4l2 subdev API point of view. > The OF graph AFAIK, has no information about which ports are sinks > and which are sources, so of_parse_subdev() tries to determine that > based on the compatible string of the device node. So ATM > of_parse_subdev() assumes there is nothing but the imx6-mipi-csi2, > video-multiplexer, and camera sensors upstream from the CSI ports > in the OF graph. > > I realize that's not a robust solution, and is the reason for the > "no sensor attached" below. > > Is there any way to determine from the OF graph the data-direction > of a port (whether it is a sink or a source)? If so it will make > of_parse_subdev() much more robust. I'm not sure why you need to know the data direction. I think the issue here is how you're going about dealing with the subdevs. Here's the subdev setup: +-camera+ | pixel array -> binner |---> csi2 --> ipu1csi0 mux --> csi0 --> smfc --> idmac +---+ How the subdevs are supposed to work is that you start from the first subdev in sequence (in this case the pixel array) and negotiate with the driver through the TRY formats on its source pad, as well as negotiating with the sink pad of the directly neighbouring subdev. The neighbouring subdev propagates the configuration in a driver specific manner from its source pad to the sink pad, giving a default configuration at its source. This process repeats throughout the chain all the way up to the bridge device. Now, where things go wrong is that you want to know what each type of these subdevs are, and the reason you want that is so you can do this (for example - I know similar stuff happens with the "sensor" stuff further up the chain as well): +-camera+ | pixel array -> binner |---> csi2 --> ipu1csi0 mux --> csi0 --> smfc --> idmac +---+| ^--I-want-your-bus-format-and-frame-rate---' which goes against the negotiation mechanism of v4l2 subdevs. This is broken - it bypass the subdev negotiation that has been performed on the intervening subdevs between the "sensor" and the csi0 subdevs, so if there were a subdev in that chain that (eg) reduced the frame rate by discarding the odd frames, you'd be working with the wrong frame interval for your frame interval monitor at csi. Note that the "MEDIA_ENT_F_PROC_VIDEO_SCALER" subdev type is documented as not only supports scaling as in changing the size of the image, but also in terms of skipping frames, which means a reduction in frame rate.
Re: [PATCH v3 00/24] i.MX Media Driver
On 01/31/2017 03:00 AM, Russell King - ARM Linux wrote: On Mon, Jan 30, 2017 at 05:22:01PM -0800, Steve Longerbeam wrote: I'm also having trouble finding a datasheet for it, but from what I've read, it has a MIPI CSI-2 interface. It should work fine as long as it presents a single source pad, registers asynchronously, and sets its entity function to MEDIA_ENT_F_CAM_SENSOR. Yes, it is MIPI CSI-2, and yes it has a single source pad, registers asynchronously, but that's about as far as it goes. The structure is a camera sensor followed by some processing. So just like the smiapp code, I've ended up with multiple subdevs describing each stage of the sensors pipeline. Just like smiapp, the camera sensor block (which is the very far end of the pipeline) is marked with MEDIA_ENT_F_CAM_SENSOR. However, in front of that is the binner, which just like smiapp gets a separate entity. It's this entity which is connected to the mipi-csi2 subdev. wow, ok got it. So the sensor pipeline and binner, and the OF graph connecting them, are described in the device tree I presume. The OF graph AFAIK, has no information about which ports are sinks and which are sources, so of_parse_subdev() tries to determine that based on the compatible string of the device node. So ATM of_parse_subdev() assumes there is nothing but the imx6-mipi-csi2, video-multiplexer, and camera sensors upstream from the CSI ports in the OF graph. I realize that's not a robust solution, and is the reason for the "no sensor attached" below. Is there any way to determine from the OF graph the data-direction of a port (whether it is a sink or a source)? If so it will make of_parse_subdev() much more robust. Steve Unlike smiapp, which does not set an entity function, I set my binner entity as MEDIA_ENT_F_PROC_VIDEO_SCALER on the basis that that is what V4L2 documentation recommend: - .. row 27 .. _MEDIA-ENT-F-PROC-VIDEO-SCALER: - ``MEDIA_ENT_F_PROC_VIDEO_SCALER`` - Video scaler. An entity capable of video scaling must have at least one sink pad and one source pad, and scale the video frame(s) received on its sink pad(s) to a different resolution output on its source pad(s). The range of supported scaling ratios is entity-specific and can differ between the horizontal and vertical directions (in particular scaling can be supported in one direction only). Binning and skipping are considered as scaling. This causes attempts to configure the ipu1_csi0 interface to fail: media-ctl -v -d /dev/media1 --set-v4l2 '"ipu1_csi0":1[fmt:SGBRG8/512x512@1/30]' Opening media device /dev/media1 Enumerating entities Found 29 entities Enumerating pads and links Setting up format SGBRG8 512x512 on pad ipu1_csi0/1 Unable to set format: No such device (-19) Unable to setup formats: No such device (19) and in the kernel log: ipu1_csi0: no sensor attached And yes, I already know that my next problem is going to be that the bayer formats are not supported in your driver (just like Philipp's driver) but adding them should not be difficult... but only once this issue is resolved. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver
On Tue, Jan 31, 2017 at 02:36:53PM -0800, Steve Longerbeam wrote: > On 01/31/2017 02:04 PM, Russell King - ARM Linux wrote: > >I don't want master though, I want v4.10-rc1, and if I ask for that > >it tells me it knows nothing about v4.10-rc1, despite the fact that's > >a tag in the mainline kernel repository which was merged into the > >linux-media tree that this tree is based upon. > > Hi Russell, yes g...@github.com:slongerbeam/mediatree.git is a fork > of the linux-media tree, and the imx-media-staging-md-wip branch > is up-to-date with master, currently at 4.10-rc1. "up to date" is different from "contains other stuff other than is in 4.10-rc1". What I see in your tree is that your code is based off a merge commit between something called "patchwork" (which I assume is a branch in the media tree containing stuff commited from patch work) and v4.10-rc1. Now, you don't get a commit when merging unless there's changes that aren't in the commit you're merging - if "patchwork" was up to date with v4.10-rc1, then git would have done a "fast forward" to v4.10-rc1. Therefore, while it may be "up to date" with v4.10-rc1 in so far that it's had v4.10-rc1 merged into it, that's not what I've been saying. There are other changes below that merge commit which aren't in v4.10-rc1. It's those other changes that I'm talking about, and it's those other changes I do not want without knowing what they are. It may be that those other changes have since been merged into v4.10-rc6 - but github's web interface can't show me that. In fact, github's web interface is pretty damned useless as far as this stuff goes. So, what I'll get if I clone or pull your imx-media-staging-md-wip branch is, yes, a copy of all your changes, but _also_ all the changes that are in the media tree that _aren't_ in mainline at the point that v4.10-rc1 was merged. > You don't need to use the web interface, just git clone the repo. You're assuming I want to work off the top of your commits. I don't. I've got other dependencies. Then there's yet another problem - lets say that I get a copy of your patches that haven't been on the mailing list, and I then want to make a comment about it. I can't reply to a patch that hasn't been on the mailing list. So, the long established mechanism by which the Linux community does patch review breaks down. So no, sorry, I'm not fetching your tree, and I will persist with your v3 patch set for the time being. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver
On 01/31/2017 03:30 PM, Russell King - ARM Linux wrote: On Tue, Jan 31, 2017 at 02:36:53PM -0800, Steve Longerbeam wrote: On 01/31/2017 02:04 PM, Russell King - ARM Linux wrote: I don't want master though, I want v4.10-rc1, and if I ask for that it tells me it knows nothing about v4.10-rc1, despite the fact that's a tag in the mainline kernel repository which was merged into the linux-media tree that this tree is based upon. Hi Russell, yes g...@github.com:slongerbeam/mediatree.git is a fork of the linux-media tree, and the imx-media-staging-md-wip branch is up-to-date with master, currently at 4.10-rc1. "up to date" is different from "contains other stuff other than is in 4.10-rc1". What I see in your tree is that your code is based off a merge commit between something called "patchwork" (which I assume is a branch in the media tree containing stuff commited from patch work) and v4.10-rc1. Now, you don't get a commit when merging unless there's changes that aren't in the commit you're merging - if "patchwork" was up to date with v4.10-rc1, then git would have done a "fast forward" to v4.10-rc1. Therefore, while it may be "up to date" with v4.10-rc1 in so far that it's had v4.10-rc1 merged into it, that's not what I've been saying. There are other changes below that merge commit which aren't in v4.10-rc1. It's those other changes that I'm talking about, and it's those other changes I do not want without knowing what they are. It may be that those other changes have since been merged into v4.10-rc6 - but github's web interface can't show me that. In fact, github's web interface is pretty damned useless as far as this stuff goes. So, what I'll get if I clone or pull your imx-media-staging-md-wip branch is, yes, a copy of all your changes, but _also_ all the changes that are in the media tree that _aren't_ in mainline at the point that v4.10-rc1 was merged. You don't need to use the web interface, just git clone the repo. You're assuming I want to work off the top of your commits. I don't. I've got other dependencies. Well, I was suggesting cloning it just to have a look at the new code, but I understand you don't want to attempt to bring in your SMIA/bayer format changes and run from this branch, due to the other changes in the mediatree. I suppose I should post the next version then. Trouble is, I see issues in the current driver that prevents working with your SMIA pipeline. But I guess that will have to be worked out in another version. Steve Then there's yet another problem - lets say that I get a copy of your patches that haven't been on the mailing list, and I then want to make a comment about it. I can't reply to a patch that hasn't been on the mailing list. So, the long established mechanism by which the Linux community does patch review breaks down. So no, sorry, I'm not fetching your tree, and I will persist with your v3 patch set for the time being. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver
On 01/31/2017 02:04 PM, Russell King - ARM Linux wrote: On Tue, Jan 31, 2017 at 09:55:29PM +, Ian Arkver wrote: On 31/01/17 20:33, Russell King - ARM Linux wrote: On Tue, Jan 31, 2017 at 10:21:26AM -0800, Steve Longerbeam wrote: On 01/31/2017 05:42 AM, Russell King - ARM Linux wrote: On Fri, Jan 20, 2017 at 03:38:28PM +0100, Hans Verkuil wrote: Should be set to something like 'platform:imx-media-camif'. v4l2-compliance should complain about this. ... and more. Right, in version 3 that you are working with, no v4l2-compliance fixes were in yet. A lot of the compliance errors are fixed, please look in latest branch imx-media-staging-md-wip at g...@github.com:slongerbeam/mediatree.git. Sorry, I'm not prepared to pull random trees from github as there's no easy way to see what's in the branch. I've always disliked github because its web interface makes it soo difficult to navigate around git trees hosted there. You can see a commit, you can see a diff of the commit. You can get a list of branches. But there seems to be no way to get a list of commits similar to "git log" or even a one-line summary of each commit on a branch. If there is, it's completely non-obvious (which I think is much of the problem with github, it's web interface is horrendous.) Or you can clone/pull the tree without knowing what you're fetching (eg, what the tree is based upon.) Or you can waste time clicking repeatedly on the "parent" commit link on each patch working your way back through the history... Well, it looks like it's bsaed on 4.10-rc1 with who-knows-what work >from the linux-media tree (I didn't try and go back any further.) As I don't want to take a whole pile of other changes into my tree, I'm certainly not going to pull from your github tree. Sorry. https://github.com/slongerbeam/mediatree/compare/master...imx-media-staging-md-wip It's under the "Compare" button from the main view. It would be nice though if the first commit's parent was some clearly tagged start point. I don't want master though, I want v4.10-rc1, and if I ask for that it tells me it knows nothing about v4.10-rc1, despite the fact that's a tag in the mainline kernel repository which was merged into the linux-media tree that this tree is based upon. Hi Russell, yes g...@github.com:slongerbeam/mediatree.git is a fork of the linux-media tree, and the imx-media-staging-md-wip branch is up-to-date with master, currently at 4.10-rc1. You don't need to use the web interface, just git clone the repo. Steve -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver
On 31/01/17 22:04, Russell King - ARM Linux wrote: On Tue, Jan 31, 2017 at 09:55:29PM +, Ian Arkver wrote: On 31/01/17 20:33, Russell King - ARM Linux wrote: On Tue, Jan 31, 2017 at 10:21:26AM -0800, Steve Longerbeam wrote: On 01/31/2017 05:42 AM, Russell King - ARM Linux wrote: On Fri, Jan 20, 2017 at 03:38:28PM +0100, Hans Verkuil wrote: Should be set to something like 'platform:imx-media-camif'. v4l2-compliance should complain about this. ... and more. Right, in version 3 that you are working with, no v4l2-compliance fixes were in yet. A lot of the compliance errors are fixed, please look in latest branch imx-media-staging-md-wip at g...@github.com:slongerbeam/mediatree.git. Sorry, I'm not prepared to pull random trees from github as there's no easy way to see what's in the branch. I've always disliked github because its web interface makes it soo difficult to navigate around git trees hosted there. You can see a commit, you can see a diff of the commit. You can get a list of branches. But there seems to be no way to get a list of commits similar to "git log" or even a one-line summary of each commit on a branch. If there is, it's completely non-obvious (which I think is much of the problem with github, it's web interface is horrendous.) Or you can clone/pull the tree without knowing what you're fetching (eg, what the tree is based upon.) Or you can waste time clicking repeatedly on the "parent" commit link on each patch working your way back through the history... Well, it looks like it's bsaed on 4.10-rc1 with who-knows-what work >from the linux-media tree (I didn't try and go back any further.) As I don't want to take a whole pile of other changes into my tree, I'm certainly not going to pull from your github tree. Sorry. https://github.com/slongerbeam/mediatree/compare/master...imx-media-staging-md-wip It's under the "Compare" button from the main view. It would be nice though if the first commit's parent was some clearly tagged start point. I don't want master though, I want v4.10-rc1, and if I ask for that it tells me it knows nothing about v4.10-rc1, despite the fact that's a tag in the mainline kernel repository which was merged into the linux-media tree that this tree is based upon. Yeah, that's what I meant about the first parent's commit not being a clearly tagged branch point. At least you get the series on one page. Maybe it's time for a rebase or a v4 series Steve? Personally, I use a bare repo with multiple remotes and fetch branches from various trees. Then gitk --all --since(etc) is pretty good at giving the overview picture. You don't need to pull the commits over into any of your working branches if you don't want to. Regards, Ian -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver
On Tue, Jan 31, 2017 at 09:55:29PM +, Ian Arkver wrote: > On 31/01/17 20:33, Russell King - ARM Linux wrote: > >On Tue, Jan 31, 2017 at 10:21:26AM -0800, Steve Longerbeam wrote: > >>On 01/31/2017 05:42 AM, Russell King - ARM Linux wrote: > >>>On Fri, Jan 20, 2017 at 03:38:28PM +0100, Hans Verkuil wrote: > Should be set to something like 'platform:imx-media-camif'. > v4l2-compliance > should complain about this. > >>>... and more. > >> > >>Right, in version 3 that you are working with, no v4l2-compliance fixes were > >>in yet. A lot of the compliance errors are fixed, please look in latest > >>branch > >>imx-media-staging-md-wip at g...@github.com:slongerbeam/mediatree.git. > > > >Sorry, I'm not prepared to pull random trees from github as there's > >no easy way to see what's in the branch. > > > >I've always disliked github because its web interface makes it soo > >difficult to navigate around git trees hosted there. You can see > >a commit, you can see a diff of the commit. You can get a list of > >branches. But there seems to be no way to get a list of commits > >similar to "git log" or even a one-line summary of each commit on > >a branch. If there is, it's completely non-obvious (which I think is > >much of the problem with github, it's web interface is horrendous.) > > > >Or you can clone/pull the tree without knowing what you're fetching > >(eg, what the tree is based upon.) > > > >Or you can waste time clicking repeatedly on the "parent" commit link > >on each patch working your way back through the history... > > > >Well, it looks like it's bsaed on 4.10-rc1 with who-knows-what work > >from the linux-media tree (I didn't try and go back any further.) > >As I don't want to take a whole pile of other changes into my tree, > >I'm certainly not going to pull from your github tree. Sorry. > > > > https://github.com/slongerbeam/mediatree/compare/master...imx-media-staging-md-wip > > It's under the "Compare" button from the main view. It would be nice though > if the first commit's parent was some clearly tagged start point. I don't want master though, I want v4.10-rc1, and if I ask for that it tells me it knows nothing about v4.10-rc1, despite the fact that's a tag in the mainline kernel repository which was merged into the linux-media tree that this tree is based upon. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver
On 31/01/17 20:33, Russell King - ARM Linux wrote: On Tue, Jan 31, 2017 at 10:21:26AM -0800, Steve Longerbeam wrote: On 01/31/2017 05:42 AM, Russell King - ARM Linux wrote: On Fri, Jan 20, 2017 at 03:38:28PM +0100, Hans Verkuil wrote: Should be set to something like 'platform:imx-media-camif'. v4l2-compliance should complain about this. ... and more. Right, in version 3 that you are working with, no v4l2-compliance fixes were in yet. A lot of the compliance errors are fixed, please look in latest branch imx-media-staging-md-wip at g...@github.com:slongerbeam/mediatree.git. Sorry, I'm not prepared to pull random trees from github as there's no easy way to see what's in the branch. I've always disliked github because its web interface makes it soo difficult to navigate around git trees hosted there. You can see a commit, you can see a diff of the commit. You can get a list of branches. But there seems to be no way to get a list of commits similar to "git log" or even a one-line summary of each commit on a branch. If there is, it's completely non-obvious (which I think is much of the problem with github, it's web interface is horrendous.) Or you can clone/pull the tree without knowing what you're fetching (eg, what the tree is based upon.) Or you can waste time clicking repeatedly on the "parent" commit link on each patch working your way back through the history... Well, it looks like it's bsaed on 4.10-rc1 with who-knows-what work from the linux-media tree (I didn't try and go back any further.) As I don't want to take a whole pile of other changes into my tree, I'm certainly not going to pull from your github tree. Sorry. https://github.com/slongerbeam/mediatree/compare/master...imx-media-staging-md-wip It's under the "Compare" button from the main view. It would be nice though if the first commit's parent was some clearly tagged start point. Regards, Ian -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] media: fix semicolon.cocci warnings
On Tue 2017-01-31 22:20:27, kbuild test robot wrote: > drivers/media/i2c/et8ek8/et8ek8_driver.c:1112:3-4: Unneeded semicolon > > > Remove unneeded semicolon. > > Generated by: scripts/coccinelle/misc/semicolon.cocci > Acked-by: Pavel Machek-- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
[PATCH] Documentation: devicetree: meson-ir: "linux,rc-map-name" is supported
The driver already parses the "linux,rc-map-name" property. Add this information to the documentation so .dts maintainers don't have to look it up in the source-code. Signed-off-by: Martin Blumenstingl--- Documentation/devicetree/bindings/media/meson-ir.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/media/meson-ir.txt b/Documentation/devicetree/bindings/media/meson-ir.txt index e7e3f3c4fc8f..efd9d29a8f10 100644 --- a/Documentation/devicetree/bindings/media/meson-ir.txt +++ b/Documentation/devicetree/bindings/media/meson-ir.txt @@ -8,6 +8,9 @@ Required properties: - reg : physical base address and length of the device registers - interrupts : a single specifier for the interrupt from the device +Optional properties: + - linux,rc-map-name: see rc.txt file in the same directory. + Example: ir-receiver@c8100480 { -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation: devicetree: add the RC map name of the geekbox remote
Signed-off-by: Martin Blumenstingl--- The geekbox keymap was added while the documentation patch was not applied yet (and I wasn't aware of this pending patch). This ensures that the documentation is in sync with the actual keymaps. Documentation/devicetree/bindings/media/rc.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/media/rc.txt b/Documentation/devicetree/bindings/media/rc.txt index 0d16d14ccd2a..d3e7a012bfda 100644 --- a/Documentation/devicetree/bindings/media/rc.txt +++ b/Documentation/devicetree/bindings/media/rc.txt @@ -46,6 +46,7 @@ The following properties are common to the infrared remote controllers: * "rc-flyvideo" * "rc-fusionhdtv-mce" * "rc-gadmei-rm008z" + * "rc-geekbox" * "rc-genius-tvgo-a11mce" * "rc-gotview7135" * "rc-hauppauge" -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver
On Tue, Jan 31, 2017 at 10:21:26AM -0800, Steve Longerbeam wrote: > On 01/31/2017 05:42 AM, Russell King - ARM Linux wrote: > >On Fri, Jan 20, 2017 at 03:38:28PM +0100, Hans Verkuil wrote: > >>Should be set to something like 'platform:imx-media-camif'. v4l2-compliance > >>should complain about this. > >... and more. > > Right, in version 3 that you are working with, no v4l2-compliance fixes were > in yet. A lot of the compliance errors are fixed, please look in latest > branch > imx-media-staging-md-wip at g...@github.com:slongerbeam/mediatree.git. Sorry, I'm not prepared to pull random trees from github as there's no easy way to see what's in the branch. I've always disliked github because its web interface makes it soo difficult to navigate around git trees hosted there. You can see a commit, you can see a diff of the commit. You can get a list of branches. But there seems to be no way to get a list of commits similar to "git log" or even a one-line summary of each commit on a branch. If there is, it's completely non-obvious (which I think is much of the problem with github, it's web interface is horrendous.) Or you can clone/pull the tree without knowing what you're fetching (eg, what the tree is based upon.) Or you can waste time clicking repeatedly on the "parent" commit link on each patch working your way back through the history... Well, it looks like it's bsaed on 4.10-rc1 with who-knows-what work from the linux-media tree (I didn't try and go back any further.) As I don't want to take a whole pile of other changes into my tree, I'm certainly not going to pull from your github tree. Sorry. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.
On Tue, 2017-01-31 at 10:30 -0800, Eric Anholt wrote: > Joe Percheswrites: > > > On Mon, 2017-01-30 at 12:05 -0800, Eric Anholt wrote: > > > Joe Perches writes: > > > > > > > On Fri, 2017-01-27 at 13:55 -0800, Eric Anholt wrote: > > > > > Generated with checkpatch.pl --fix-inplace and git add -p out of the > > > > > results. > > > > > > > > Maybe another. > > > > > > > > > diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c > > > > > b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c > > > > > > > > [] > > > > > @@ -239,7 +239,7 @@ static int bulk_receive(struct > > > > > vchiq_mmal_instance *instance, > > > > > pr_err("buffer list empty trying to submit bulk > > > > > receive\n"); > > > > > > > > > > /* todo: this is a serious error, we should never have > > > > > - * commited a buffer_to_host operation to the mmal > > > > > + * committed a buffer_to_host operation to the mmal > > > > >* port without the buffer to back it up (underflow > > > > >* handling) and there is no obvious way to deal with > > > > >* this - how is the mmal servie going to react when > > > > > > > > Perhaps s/servie/service/ ? > > > > > > I was trying to restrict this patch to just the fixes from checkpatch. > > > > That's the wrong thing to do if you're fixing > > spelling defects. checkpatch is just one mechanism > > to identify some, and definitely not all, typos and > > spelling defects. > > > > If you fixing, fix. Don't just rely on the brainless > > tools, use your decidedly non-mechanical brain. > > "if you touch anything, you must fix everything." If that's how things > work, I would just retract the patch. I didn't say that,and I don't mean that. If you notice a similar defect when you are fixing any arbitrary defect, please try to fix all of similar defects. As is, a patch that fixes just servie would cause a patch conflict with your patch. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.
Joe Percheswrites: > On Mon, 2017-01-30 at 12:05 -0800, Eric Anholt wrote: >> Joe Perches writes: >> >> > On Fri, 2017-01-27 at 13:55 -0800, Eric Anholt wrote: >> > > Generated with checkpatch.pl --fix-inplace and git add -p out of the >> > > results. >> > >> > Maybe another. >> > >> > > diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c >> > > b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c >> > >> > [] >> > > @@ -239,7 +239,7 @@ static int bulk_receive(struct vchiq_mmal_instance >> > > *instance, >> > > pr_err("buffer list empty trying to submit bulk >> > > receive\n"); >> > > >> > > /* todo: this is a serious error, we should never have >> > > - * commited a buffer_to_host operation to the mmal >> > > + * committed a buffer_to_host operation to the mmal >> > > * port without the buffer to back it up (underflow >> > > * handling) and there is no obvious way to deal with >> > > * this - how is the mmal servie going to react when >> > >> > Perhaps s/servie/service/ ? >> >> I was trying to restrict this patch to just the fixes from checkpatch. > > That's the wrong thing to do if you're fixing > spelling defects. checkpatch is just one mechanism > to identify some, and definitely not all, typos and > spelling defects. > > If you fixing, fix. Don't just rely on the brainless > tools, use your decidedly non-mechanical brain. "if you touch anything, you must fix everything." If that's how things work, I would just retract the patch. signature.asc Description: PGP signature
Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver
On 01/31/2017 05:42 AM, Russell King - ARM Linux wrote: On Fri, Jan 20, 2017 at 03:38:28PM +0100, Hans Verkuil wrote: Should be set to something like 'platform:imx-media-camif'. v4l2-compliance should complain about this. ... and more. Right, in version 3 that you are working with, no v4l2-compliance fixes were in yet. A lot of the compliance errors are fixed, please look in latest branch imx-media-staging-md-wip at g...@github.com:slongerbeam/mediatree.git. Total: 39, Succeeded: 33, Failed: 6, Warnings: 0 Not all of these may be a result of Steve's code - this is running against my gradually modified version to support bayer formats (which seems to be the cause of the v4l2-test-formats.cpp failure... for some reason the driver isn't enumerating all the formats.) And that reason is the way that the formats are enumerated: static int camif_enum_fmt_vid_cap(struct file *file, void *fh, struct v4l2_fmtdesc *f) { const struct imx_media_pixfmt *cc; u32 code; int ret; ret = imx_media_enum_format(, f->index, true, true); if (ret) return ret; cc = imx_media_find_format(0, code, true, true); if (!cc) return -EINVAL; When imx_media_enum_format() hits this entry in the table: }, { .fourcc = V4L2_PIX_FMT_BGR24, .cs = IPUV3_COLORSPACE_RGB, .bpp= 24, }, { becaues there's no .codes defined: int imx_media_enum_format(u32 *code, u32 index, bool allow_rgb, bool allow_planar) { ... *code = fmt->codes[0]; return 0; } So, we end up calling imx_media_find_format(0, 0, true, true), which fails, returning NULL. That causes camif_enum_fmt_vid_cap() to return -EINVAL. So everything past this entry is unable to be enumerated. I think this is a really round-about way of enumerating the pixel formats when there are soo many entries in the table which have no media bus code - there's absolutely no way that any of those entries can ever be enumerated in this fashion, so they might as well not be in the table... That's my present solution to this problem, to #if 0 out all the entries without any .codes field. I think the real answer is that this needs a _separate_ function to enumerate the pixel formats for camif_enum_fmt_vid_cap(). However, there may be other issues lurking that I've not yet found (still trying to get this code to work...) I believe this has been fixed in imx-media-staging-md-wip as well, see imx-media-capture.c:capture_enum_fmt_vid_cap() Camif subdev is gone, replaced with a set of exported functions that allow attaching a capture device (and v4l2 interface) to a calling subdev's output pad. See imx-media-capture.c. The subdev's capture device interface is the only subdev that can request a planar format from imx_media_enum_format(). All the others now (the non-device node pads), request only RGB or packed YUV, or the IPU internal formats for IPU internal connections, and these are the first entries in the table. The planar formats all are at the end, which can only be enumerated by the capture device interfaces. Steve -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
rtl2832_sdr and /dev/swradio0
Hi, Is anyone actively working on the rtl2832_sdr driver? I am particularly interested in anyone who has code for turning the byte stream from /dev/swradio0 into an ETI stream. Or failing that getting enough data about the API for using /dev/swradio0 so as to write a byte sequence to ETI driver based on the dab2eti program in DABtool (which uses the librtlsdr mechanism instead of the /dev/swradio0 one). -- Russel. = Dr Russel Winder t: +44 20 7585 2200 voip: sip:russel.win...@ekiga.net 41 Buckmaster Roadm: +44 7770 465 077 xmpp: rus...@winder.org.uk London SW11 1EN, UK w: www.russel.org.uk skype: russel_winder signature.asc Description: This is a digitally signed message part
Re: [PATCH v3 17/24] media: imx: Add CSI subdev driver
On Tue, 2017-01-31 at 16:51 +0100, Philipp Zabel wrote: > On Fri, 2017-01-06 at 18:11 -0800, Steve Longerbeam wrote: > [...] > > +static int csi_set_fmt(struct v4l2_subdev *sd, > > + struct v4l2_subdev_pad_config *cfg, > > + struct v4l2_subdev_format *sdformat) > > +{ > > + struct csi_priv *priv = v4l2_get_subdevdata(sd); > > + struct v4l2_mbus_framefmt *infmt, *outfmt; > > + struct v4l2_rect crop; > > + int ret; > > + > > + if (sdformat->pad >= CSI_NUM_PADS) > > + return -EINVAL; > > + > > + if (priv->stream_on) > > + return -EBUSY; > > + > > + infmt = >format_mbus[priv->input_pad]; > > + outfmt = >format_mbus[priv->output_pad]; > > + > > + if (sdformat->pad == priv->output_pad) { > > + sdformat->format.code = infmt->code; > > + sdformat->format.field = infmt->field; > > + crop.left = priv->crop.left; > > + crop.top = priv->crop.top; > > + crop.width = sdformat->format.width; > > + crop.height = sdformat->format.height; > > + ret = csi_try_crop(priv, ); > > This is the wrong way around, see also below. > > Here the the output sdformat->format.width/height should be set to the > priv->crop.width/height (or priv->crop.width/height / 2, to enable > downscaling). The crop rectangle should not be changed by an output pad > set_fmt. [...] > The crop rectangle instead should be reset to the full input frame when > the input pad format is set. How about this: --8<-- diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index 5e80a0871b139..8220e4204a125 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -484,6 +484,8 @@ static int csi_setup(struct csi_priv *priv) if_fmt.field = outfmt->field; ipu_csi_set_window(priv->csi, >crop); + ipu_csi_set_downsize(priv->csi, priv->crop.width == 2 * outfmt->width, + priv->crop.height == 2 * outfmt->height); ipu_csi_init_interface(priv->csi, _mbus_cfg, _fmt); @@ -830,15 +832,14 @@ static int csi_set_fmt(struct v4l2_subdev *sd, switch (sdformat->pad) { case CSI_SRC_PAD_DIRECT: case CSI_SRC_PAD_IDMAC: - crop.left = priv->crop.left; - crop.top = priv->crop.top; - crop.width = sdformat->format.width; - crop.height = sdformat->format.height; - ret = csi_try_crop(priv, , sensor); - if (ret) - return ret; - sdformat->format.width = crop.width; - sdformat->format.height = crop.height; + if (sdformat->format.width < priv->crop.width * 3 / 4) + sdformat->format.width = priv->crop.width / 2; + else + sdformat->format.width = priv->crop.width; + if (sdformat->format.height < priv->crop.height * 3 / 4) + sdformat->format.height = priv->crop.height / 2; + else + sdformat->format.height = priv->crop.height; if (sdformat->pad == CSI_SRC_PAD_IDMAC) { cc = imx_media_find_format(0, sdformat->format.code, @@ -887,6 +888,14 @@ static int csi_set_fmt(struct v4l2_subdev *sd, } break; case CSI_SINK_PAD: + crop.left = 0; + crop.top = 0; + crop.width = sdformat->format.width; + crop.height = sdformat->format.height; + ret = csi_try_crop(priv, , sensor); + if (ret) + return ret; + cc = imx_media_find_format(0, sdformat->format.code, true, false); if (!cc) { @@ -904,9 +913,8 @@ static int csi_set_fmt(struct v4l2_subdev *sd, } else { priv->format_mbus[sdformat->pad] = sdformat->format; priv->cc[sdformat->pad] = cc; - /* Update the crop window if this is an output pad */ - if (sdformat->pad == CSI_SRC_PAD_DIRECT || - sdformat->pad == CSI_SRC_PAD_IDMAC) + /* Reset the crop window if this is the input pad */ + if (sdformat->pad == CSI_SINK_PAD) priv->crop = crop; } -->8-- regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 08/10] [media] st-delta: EOS (End Of Stream) support
EOS (End Of Stream) support allows user to get all the potential decoded frames remaining in decoder pipeline after having reached the end of video bitstream. To do so, user calls VIDIOC_DECODER_CMD(V4L2_DEC_CMD_STOP) which will drain the decoder and get the drained frames that are then returned to user. User is informed of EOS completion in two ways: - dequeue of an empty frame flagged to V4L2_BUF_FLAG_LAST - reception of a V4L2_EVENT_EOS event. If, unfortunately, no buffer is available on CAPTURE queue to return the empty frame, EOS is delayed till user queue one CAPTURE buffer. Signed-off-by: Hugues Fruchet--- drivers/media/platform/sti/delta/delta-v4l2.c | 146 +- drivers/media/platform/sti/delta/delta.h | 23 2 files changed, 168 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c b/drivers/media/platform/sti/delta/delta-v4l2.c index 8af5014..9add119 100644 --- a/drivers/media/platform/sti/delta/delta-v4l2.c +++ b/drivers/media/platform/sti/delta/delta-v4l2.c @@ -107,7 +107,8 @@ static void delta_frame_done(struct delta_ctx *ctx, struct delta_frame *frame, vbuf->sequence = ctx->frame_num++; v4l2_m2m_buf_done(vbuf, err ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE); - ctx->output_frames++; + if (frame->info.size) /* ignore EOS */ + ctx->output_frames++; } static void requeue_free_frames(struct delta_ctx *ctx) @@ -765,6 +766,135 @@ static int delta_g_selection(struct file *file, void *fh, return 0; } +static void delta_complete_eos(struct delta_ctx *ctx, + struct delta_frame *frame) +{ + struct delta_dev *delta = ctx->dev; + const struct v4l2_event ev = {.type = V4L2_EVENT_EOS}; + + /* +* Send EOS to user: +* - by returning an empty frame flagged to V4L2_BUF_FLAG_LAST +* - and then send EOS event +*/ + + /* empty frame */ + frame->info.size = 0; + + /* set the last buffer flag */ + frame->flags |= V4L2_BUF_FLAG_LAST; + + /* release frame to user */ + delta_frame_done(ctx, frame, 0); + + /* send EOS event */ + v4l2_event_queue_fh(>fh, ); + + dev_dbg(delta->dev, "%s EOS completed\n", ctx->name); +} + +static int delta_try_decoder_cmd(struct file *file, void *fh, +struct v4l2_decoder_cmd *cmd) +{ + if (cmd->cmd != V4L2_DEC_CMD_STOP) + return -EINVAL; + + if (cmd->flags & V4L2_DEC_CMD_STOP_TO_BLACK) + return -EINVAL; + + if (!(cmd->flags & V4L2_DEC_CMD_STOP_IMMEDIATELY) && + (cmd->stop.pts != 0)) + return -EINVAL; + + return 0; +} + +static int delta_decoder_stop_cmd(struct delta_ctx *ctx, void *fh) +{ + const struct delta_dec *dec = ctx->dec; + struct delta_dev *delta = ctx->dev; + struct delta_frame *frame = NULL; + int ret = 0; + + dev_dbg(delta->dev, "%s EOS received\n", ctx->name); + + if (ctx->state != DELTA_STATE_READY) + return 0; + + /* drain the decoder */ + call_dec_op(dec, drain, ctx); + + /* release to user drained frames */ + while (1) { + frame = NULL; + ret = call_dec_op(dec, get_frame, ctx, ); + if (ret == -ENODATA) { + /* no more decoded frames */ + break; + } + if (frame) { + dev_dbg(delta->dev, "%s drain frame[%d]\n", + ctx->name, frame->index); + + /* pop timestamp and mark frame with it */ + delta_pop_dts(ctx, >dts); + + /* release decoded frame to user */ + delta_frame_done(ctx, frame, 0); + } + } + + /* try to complete EOS */ + ret = delta_get_free_frame(ctx, ); + if (ret) + goto delay_eos; + + /* new frame available, EOS can now be completed */ + delta_complete_eos(ctx, frame); + + ctx->state = DELTA_STATE_EOS; + + return 0; + +delay_eos: + /* +* EOS completion from driver is delayed because +* we don't have a free empty frame available. +* EOS completion is so delayed till next frame_queue() call +* to be sure to have a free empty frame available. +*/ + ctx->state = DELTA_STATE_WF_EOS; + dev_dbg(delta->dev, "%s EOS delayed\n", ctx->name); + + return 0; +} + +static int delta_decoder_cmd(struct file *file, void *fh, +struct v4l2_decoder_cmd *cmd) +{ + struct delta_ctx *ctx = to_ctx(fh); + int ret = 0; + + ret = delta_try_decoder_cmd(file, fh, cmd); + if (ret) + return ret; + + return delta_decoder_stop_cmd(ctx, fh); +} + +static int
[PATCH v5 09/10] [media] st-delta: add mjpeg support
Adds support of DELTA MJPEG video decoder back-end, implemented by calling JPEG_DECODER_HW0 firmware using RPMSG IPC communication layer. Signed-off-by: Hugues Fruchet--- drivers/media/platform/Kconfig | 6 + drivers/media/platform/sti/delta/Makefile | 4 + drivers/media/platform/sti/delta/delta-cfg.h | 3 + drivers/media/platform/sti/delta/delta-mjpeg-dec.c | 455 + drivers/media/platform/sti/delta/delta-mjpeg-fw.h | 225 ++ drivers/media/platform/sti/delta/delta-mjpeg-hdr.c | 149 +++ drivers/media/platform/sti/delta/delta-mjpeg.h | 35 ++ drivers/media/platform/sti/delta/delta-v4l2.c | 3 + 8 files changed, 880 insertions(+) create mode 100644 drivers/media/platform/sti/delta/delta-mjpeg-dec.c create mode 100644 drivers/media/platform/sti/delta/delta-mjpeg-fw.h create mode 100644 drivers/media/platform/sti/delta/delta-mjpeg-hdr.c create mode 100644 drivers/media/platform/sti/delta/delta-mjpeg.h diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 93023c3..9e71a7b 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -317,6 +317,12 @@ config VIDEO_STI_DELTA if VIDEO_STI_DELTA +config VIDEO_STI_DELTA_MJPEG + bool "STMicroelectronics DELTA MJPEG support" + default y + help + Enables DELTA MJPEG hardware support. + endif # VIDEO_STI_DELTA config VIDEO_SH_VEU diff --git a/drivers/media/platform/sti/delta/Makefile b/drivers/media/platform/sti/delta/Makefile index e47e0bd..663be70 100644 --- a/drivers/media/platform/sti/delta/Makefile +++ b/drivers/media/platform/sti/delta/Makefile @@ -1,2 +1,6 @@ obj-$(CONFIG_VIDEO_STI_DELTA) := st-delta.o st-delta-y := delta-v4l2.o delta-mem.o delta-ipc.o + +# MJPEG support +st-delta-$(CONFIG_VIDEO_STI_DELTA_MJPEG) += delta-mjpeg-hdr.o +st-delta-$(CONFIG_VIDEO_STI_DELTA_MJPEG) += delta-mjpeg-dec.o diff --git a/drivers/media/platform/sti/delta/delta-cfg.h b/drivers/media/platform/sti/delta/delta-cfg.h index f6674f6..c6388f5 100644 --- a/drivers/media/platform/sti/delta/delta-cfg.h +++ b/drivers/media/platform/sti/delta/delta-cfg.h @@ -57,5 +57,8 @@ #define DELTA_HW_AUTOSUSPEND_DELAY_MS 5 #define DELTA_MAX_DECODERS 10 +#ifdef CONFIG_VIDEO_STI_DELTA_MJPEG +extern const struct delta_dec mjpegdec; +#endif #endif /* DELTA_CFG_H */ diff --git a/drivers/media/platform/sti/delta/delta-mjpeg-dec.c b/drivers/media/platform/sti/delta/delta-mjpeg-dec.c new file mode 100644 index 000..e79bdc6 --- /dev/null +++ b/drivers/media/platform/sti/delta/delta-mjpeg-dec.c @@ -0,0 +1,455 @@ +/* + * Copyright (C) STMicroelectronics SA 2013 + * Author: Hugues Fruchet for STMicroelectronics. + * License terms: GNU General Public License (GPL), version 2 + */ + +#include + +#include "delta.h" +#include "delta-ipc.h" +#include "delta-mjpeg.h" +#include "delta-mjpeg-fw.h" + +#define DELTA_MJPEG_MAX_RESO DELTA_MAX_RESO + +struct delta_mjpeg_ctx { + /* jpeg header */ + struct mjpeg_header header_struct; + struct mjpeg_header *header; + + /* ipc */ + void *ipc_hdl; + struct delta_buf *ipc_buf; + + /* decoded output frame */ + struct delta_frame *out_frame; + + unsigned char str[3000]; +}; + +#define to_ctx(ctx) ((struct delta_mjpeg_ctx *)(ctx)->priv) + +static char *ipc_open_param_str(struct jpeg_video_decode_init_params_t *p, + char *str, unsigned int len) +{ + char *b = str; + + if (!p) + return ""; + + b += snprintf(b, len, + "jpeg_video_decode_init_params_t\n" + "circular_buffer_begin_addr_p 0x%x\n" + "circular_buffer_end_addr_p 0x%x\n", + p->circular_buffer_begin_addr_p, + p->circular_buffer_end_addr_p); + + return str; +} + +static char *ipc_decode_param_str(struct jpeg_decode_params_t *p, + char *str, unsigned int len) +{ + char *b = str; + + if (!p) + return ""; + + b += snprintf(b, len, + "jpeg_decode_params_t\n" + "picture_start_addr_p 0x%x\n" + "picture_end_addr_p0x%x\n" + "decoding_mode%d\n" + "display_buffer_addr.display_decimated_luma_p 0x%x\n" + "display_buffer_addr.display_decimated_chroma_p 0x%x\n" + "main_aux_enable %d\n" + "additional_flags 0x%x\n" + "field_flag %x\n" + "is_jpeg_image%x\n", + p->picture_start_addr_p, + p->picture_end_addr_p, +
[PATCH v5 05/10] [media] st-delta: STiH4xx multi-format video decoder v4l2 driver
This V4L2 driver enables DELTA multi-format video decoder of STMicroelectronics STiH4xx SoC series. Signed-off-by: Hugues Fruchet--- drivers/media/platform/Kconfig| 20 + drivers/media/platform/Makefile |2 + drivers/media/platform/sti/delta/Makefile |2 + drivers/media/platform/sti/delta/delta-cfg.h | 61 + drivers/media/platform/sti/delta/delta-v4l2.c | 1816 + drivers/media/platform/sti/delta/delta.h | 514 +++ 6 files changed, 2415 insertions(+) create mode 100644 drivers/media/platform/sti/delta/Makefile create mode 100644 drivers/media/platform/sti/delta/delta-cfg.h create mode 100644 drivers/media/platform/sti/delta/delta-v4l2.c create mode 100644 drivers/media/platform/sti/delta/delta.h diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index d944421..451a33b 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -298,6 +298,26 @@ config VIDEO_STI_HVA To compile this driver as a module, choose M here: the module will be called st-hva. +config VIDEO_STI_DELTA + tristate "STMicroelectronics STiH4xx DELTA multi-format video decoder V4L2 driver" + depends on VIDEO_DEV && VIDEO_V4L2 + depends on ARCH_STI || COMPILE_TEST + depends on HAS_DMA + select VIDEOBUF2_DMA_CONTIG + select V4L2_MEM2MEM_DEV + help + This V4L2 driver enables DELTA multi-format video decoder + of STMicroelectronics STiH4xx SoC series allowing hardware + decoding of various compressed video bitstream format in + raw uncompressed format. + + To compile this driver as a module, choose M here: + the module will be called st-delta. + +if VIDEO_STI_DELTA + +endif # VIDEO_STI_DELTA + config VIDEO_SH_VEU tristate "SuperH VEU mem2mem video processing driver" depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index 5b3cb27..349ddf6 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -39,6 +39,8 @@ obj-$(CONFIG_VIDEO_STI_BDISP) += sti/bdisp/ obj-$(CONFIG_VIDEO_STI_HVA)+= sti/hva/ obj-$(CONFIG_DVB_C8SECTPFE)+= sti/c8sectpfe/ +obj-$(CONFIG_VIDEO_STI_DELTA) += sti/delta/ + obj-$(CONFIG_BLACKFIN) += blackfin/ obj-$(CONFIG_ARCH_DAVINCI) += davinci/ diff --git a/drivers/media/platform/sti/delta/Makefile b/drivers/media/platform/sti/delta/Makefile new file mode 100644 index 000..07ba7ad --- /dev/null +++ b/drivers/media/platform/sti/delta/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_VIDEO_STI_DELTA) := st-delta.o +st-delta-y := delta-v4l2.o diff --git a/drivers/media/platform/sti/delta/delta-cfg.h b/drivers/media/platform/sti/delta/delta-cfg.h new file mode 100644 index 000..f6674f6 --- /dev/null +++ b/drivers/media/platform/sti/delta/delta-cfg.h @@ -0,0 +1,61 @@ +/* + * Copyright (C) STMicroelectronics SA 2015 + * Author: Hugues Fruchet for STMicroelectronics. + * License terms: GNU General Public License (GPL), version 2 + */ + +#ifndef DELTA_CFG_H +#define DELTA_CFG_H + +#define DELTA_FW_VERSION "21.1-3" + +#define DELTA_MIN_WIDTH 32 +#define DELTA_MAX_WIDTH 4096 +#define DELTA_MIN_HEIGHT 32 +#define DELTA_MAX_HEIGHT 2400 + +/* DELTA requires a 32x32 pixels alignment for frames */ +#define DELTA_WIDTH_ALIGNMENT32 +#define DELTA_HEIGHT_ALIGNMENT 32 + +#define DELTA_DEFAULT_WIDTH DELTA_MIN_WIDTH +#define DELTA_DEFAULT_HEIGHT DELTA_MIN_HEIGHT +#define DELTA_DEFAULT_FRAMEFORMAT V4L2_PIX_FMT_NV12 +#define DELTA_DEFAULT_STREAMFORMAT V4L2_PIX_FMT_MJPEG + +#define DELTA_MAX_RESO (DELTA_MAX_WIDTH * DELTA_MAX_HEIGHT) + +/* guard value for number of access units */ +#define DELTA_MAX_AUS 10 + +/* IP perf dependent, can be tuned */ +#define DELTA_PEAK_FRAME_SMOOTHING 2 + +/* + * guard output frame count: + * - at least 1 frame needed for display + * - at worst 21 + * ( max h264 dpb (16) + + * decoding peak smoothing (2) + + * user display pipeline (3) ) + */ +#define DELTA_MIN_FRAME_USER1 +#define DELTA_MAX_DPB 16 +#define DELTA_MAX_FRAME_USER3 /* platform/use-case dependent */ +#define DELTA_MAX_FRAMES (DELTA_MAX_DPB + DELTA_PEAK_FRAME_SMOOTHING +\ + DELTA_MAX_FRAME_USER) + +#if DELTA_MAX_FRAMES > VIDEO_MAX_FRAME +#undef DELTA_MAX_FRAMES +#define DELTA_MAX_FRAMES (VIDEO_MAX_FRAME) +#endif + +/* extra space to be allocated to store codec specific data per frame */ +#define DELTA_MAX_FRAME_PRIV_SIZE 100 + +/* PM runtime auto power-off after 5ms of inactivity */ +#define DELTA_HW_AUTOSUSPEND_DELAY_MS 5 + +#define DELTA_MAX_DECODERS 10 + +#endif /* DELTA_CFG_H */ diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c
[PATCH v5 01/10] Documentation: DT: add bindings for ST DELTA
This patch adds DT binding documentation for STMicroelectronics DELTA V4L2 video decoder. Signed-off-by: Hugues Fruchet--- Documentation/devicetree/bindings/media/st,st-delta.txt | 17 + 1 file changed, 17 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/st,st-delta.txt diff --git a/Documentation/devicetree/bindings/media/st,st-delta.txt b/Documentation/devicetree/bindings/media/st,st-delta.txt new file mode 100644 index 000..a538ab3 --- /dev/null +++ b/Documentation/devicetree/bindings/media/st,st-delta.txt @@ -0,0 +1,17 @@ +* STMicroelectronics DELTA multi-format video decoder + +Required properties: +- compatible: should be "st,st-delta". +- clocks: from common clock binding: handle hardware IP needed clocks, the + number of clocks may depend on the SoC type. + See ../clock/clock-bindings.txt for details. +- clock-names: names of the clocks listed in clocks property in the same order. + +Example: + delta0 { + compatible = "st,st-delta"; + clock-names = "delta", "delta-st231", "delta-flash-promip"; + clocks = <_s_c0_flexgen CLK_VID_DMU>, +<_s_c0_flexgen CLK_ST231_DMU>, +<_s_c0_flexgen CLK_FLASH_PROMIP>; + }; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 00/10] Add support for DELTA video decoder of STMicroelectronics STiH4xx SoC series
This patchset introduces a basic support for DELTA multi-format video decoder of STMicroelectronics STiH4xx SoC series. DELTA hardware IP is controlled by a remote firmware loaded in a ST231 coprocessor. Communication with firmware is done within an IPC layer using rpmsg kernel framework and a shared memory for messages handling. This driver is compatible with firmware version 21.1-3. While a single firmware is loaded in ST231 coprocessor, it is composed of several firmwares, one per video format family. This DELTA V4L2 driver is designed around files: - delta-v4l2.c : handles V4L2 APIs using M2M framework and calls decoder ops - delta-* : implements decoder calling its associated video firmware (for ex. MJPEG) using IPC layer - delta-ipc.c: IPC layer which handles communication with firmware using rpmsg This first basic support implements only MJPEG hardware acceleration but the driver structure is in place to support all the features of the DELTA video decoder hardware IP. This driver depends on: - ST remoteproc/rpmsg: - https://lkml.org/lkml/2017/1/31/389: remoteproc: st: add virtio_rpmsg support - https://lkml.org/lkml/2017/1/31/415: virtio_rpmsg: make rpmsg channel configurable - ST DELTA firmware: its license is under review. When available, pull request will be done on linux-firmware. === = history = === version 5 - update after v4 review: - fixed warning in case of no decoder selected in config - fixed multiple lines comments - dev_info changed to dev_dbg for summary trace (from HVA driver review) version 4 - update after v3 review: - "select" RPMSG instead "depends on" - v4l2-compliance S_SELECTION is no more failed till sync on latest v4l2-compliance codebase - sparse warnings fixes version 3 - update after v2 review: - fixed m2m_buf_done missing on start_streaming error case - fixed q->dev missing in queue_init() - removed unsupported s_selection - refactored string namings in delta-debug.c - fixed space before comment - all commits have commit messages - reword memory allocator helper commit version 2 - update after v1 review: - simplified tracing - G_/S_SELECTION reworked to fit COMPOSE(CAPTURE) - fixed m2m_buf_done missing on start_streaming error case - fixed q->dev missing in queue_init() - switch to kernel-4.9 rpmsg API - DELTA support added in multi_v7_defconfig - minor typo fixes & code cleanup version 1: - Initial submission === = v4l2-compliance = === Below is the v4l2-compliance report for the version 4 of the DELTA video decoder driver. v4l2-compliance has been build from SHA1: 003f31e59f353b4aecc82e8fb1c7555964da7efa (v4l2-compliance: allow S_SELECTION to return ENOTTY) root@sti-4:~# v4l2-compliance -d /dev/video0 v4l2-compliance SHA : 003f31e59f353b4aecc82e8fb1c7555964da7efa Driver Info: Driver name : st-delta Card type : st-delta-21.1-3 Bus info : platform:soc:delta0 Driver version: 4.9.0 Capabilities : 0x84208000 Video Memory-to-Memory Streaming Extended Pix Format Device Capabilities Device Caps : 0x04208000 Video Memory-to-Memory Streaming Extended Pix Format Compliance test for device /dev/video0 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported) test VIDIOC_QUERYCTRL: OK (Not Supported) test
[PATCH v5 07/10] [media] st-delta: rpmsg ipc support
IPC (Inter Process Communication) support for communication with DELTA coprocessor firmware using rpmsg kernel framework. Based on 4 services open/set_stream/decode/close and their associated rpmsg messages. The messages structures are duplicated on both host and firmware side and are packed (use only of 32 bits size fields in messages structures to ensure packing). Each service is synchronous; service returns only when firmware acknowledges the associated command message. Due to significant parameters size exchanged from host to copro, parameters are not inserted in rpmsg messages. Instead, parameters are stored in physical memory shared between host and coprocessor. Memory is non-cacheable, so no special operation is required to ensure memory coherency on host and on coprocessor side. Multi-instance support and re-entrance are ensured using host_hdl and copro_hdl in message header exchanged between both host and coprocessor. This avoids to manage tables on both sides to get back the running context of each instance. Signed-off-by: Hugues Fruchet--- drivers/media/platform/Kconfig| 1 + drivers/media/platform/sti/delta/Makefile | 2 +- drivers/media/platform/sti/delta/delta-ipc.c | 594 ++ drivers/media/platform/sti/delta/delta-ipc.h | 76 drivers/media/platform/sti/delta/delta-v4l2.c | 11 + drivers/media/platform/sti/delta/delta.h | 21 + 6 files changed, 704 insertions(+), 1 deletion(-) create mode 100644 drivers/media/platform/sti/delta/delta-ipc.c create mode 100644 drivers/media/platform/sti/delta/delta-ipc.h diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 451a33b..93023c3 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -305,6 +305,7 @@ config VIDEO_STI_DELTA depends on HAS_DMA select VIDEOBUF2_DMA_CONTIG select V4L2_MEM2MEM_DEV + select RPMSG help This V4L2 driver enables DELTA multi-format video decoder of STMicroelectronics STiH4xx SoC series allowing hardware diff --git a/drivers/media/platform/sti/delta/Makefile b/drivers/media/platform/sti/delta/Makefile index cbfb1b5..e47e0bd 100644 --- a/drivers/media/platform/sti/delta/Makefile +++ b/drivers/media/platform/sti/delta/Makefile @@ -1,2 +1,2 @@ obj-$(CONFIG_VIDEO_STI_DELTA) := st-delta.o -st-delta-y := delta-v4l2.o delta-mem.o +st-delta-y := delta-v4l2.o delta-mem.o delta-ipc.o diff --git a/drivers/media/platform/sti/delta/delta-ipc.c b/drivers/media/platform/sti/delta/delta-ipc.c new file mode 100644 index 000..41e4a4c --- /dev/null +++ b/drivers/media/platform/sti/delta/delta-ipc.c @@ -0,0 +1,594 @@ +/* + * Copyright (C) STMicroelectronics SA 2015 + * Author: Hugues Fruchet for STMicroelectronics. + * License terms: GNU General Public License (GPL), version 2 + */ + +#include + +#include "delta.h" +#include "delta-ipc.h" +#include "delta-mem.h" + +#define IPC_TIMEOUT 100 +#define IPC_SANITY_TAG 0xDEADBEEF + +enum delta_ipc_fw_command { + DELTA_IPC_OPEN, + DELTA_IPC_SET_STREAM, + DELTA_IPC_DECODE, + DELTA_IPC_CLOSE +}; + +#define to_rpmsg_driver(__drv) container_of(__drv, struct rpmsg_driver, drv) +#define to_delta(__d) container_of(__d, struct delta_dev, rpmsg_driver) + +#define to_ctx(hdl) ((struct delta_ipc_ctx *)hdl) +#define to_pctx(ctx) container_of(ctx, struct delta_ctx, ipc_ctx) + +struct delta_ipc_header_msg { + u32 tag; + void *host_hdl; + u32 copro_hdl; + u32 command; +}; + +#define to_host_hdl(ctx) ((void *)ctx) + +#define msg_to_ctx(msg) ((struct delta_ipc_ctx *)(msg)->header.host_hdl) +#define msg_to_copro_hdl(msg) ((msg)->header.copro_hdl) + +static inline dma_addr_t to_paddr(struct delta_ipc_ctx *ctx, void *vaddr) +{ + return (ctx->ipc_buf->paddr + (vaddr - ctx->ipc_buf->vaddr)); +} + +static inline bool is_valid_data(struct delta_ipc_ctx *ctx, +void *data, u32 size) +{ + return ((data >= ctx->ipc_buf->vaddr) && + ((data + size) <= (ctx->ipc_buf->vaddr + ctx->ipc_buf->size))); +} + +/* + * IPC shared memory (@ipc_buf_size, @ipc_buf_paddr) is sent to copro + * at each instance opening. This memory is allocated by IPC client + * and given through delta_ipc_open(). All messages parameters + * (open, set_stream, decode) will have their phy address within + * this IPC shared memory, avoiding de-facto recopies inside delta-ipc. + * All the below messages structures are used on both host and firmware + * side and are packed (use only of 32 bits size fields in messages + * structures to ensure packing): + * - struct delta_ipc_open_msg + * - struct delta_ipc_set_stream_msg + * - struct delta_ipc_decode_msg + * - struct delta_ipc_close_msg + * - struct delta_ipc_cb_msg + */ +struct delta_ipc_open_msg { + struct delta_ipc_header_msg header; + u32 ipc_buf_size;
[PATCH v5 10/10] [media] st-delta: debug: trace stream/frame information & summary
Adds some trace points showing input compressed stream or output decoded frame information. Adds an unconditional trace point when streaming starts showing the compressed stream and the decoded frame information. Adds an unconditional trace point at instance closure summarizing into a single line the decoding process (stream information, decoded and output frames number, potential errors observed). Signed-off-by: Hugues Fruchet--- drivers/media/platform/sti/delta/Makefile | 2 +- drivers/media/platform/sti/delta/delta-debug.c | 72 ++ drivers/media/platform/sti/delta/delta-debug.h | 18 +++ drivers/media/platform/sti/delta/delta-v4l2.c | 30 +-- 4 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 drivers/media/platform/sti/delta/delta-debug.c create mode 100644 drivers/media/platform/sti/delta/delta-debug.h diff --git a/drivers/media/platform/sti/delta/Makefile b/drivers/media/platform/sti/delta/Makefile index 663be70..f95580e 100644 --- a/drivers/media/platform/sti/delta/Makefile +++ b/drivers/media/platform/sti/delta/Makefile @@ -1,5 +1,5 @@ obj-$(CONFIG_VIDEO_STI_DELTA) := st-delta.o -st-delta-y := delta-v4l2.o delta-mem.o delta-ipc.o +st-delta-y := delta-v4l2.o delta-mem.o delta-ipc.o delta-debug.o # MJPEG support st-delta-$(CONFIG_VIDEO_STI_DELTA_MJPEG) += delta-mjpeg-hdr.o diff --git a/drivers/media/platform/sti/delta/delta-debug.c b/drivers/media/platform/sti/delta/delta-debug.c new file mode 100644 index 000..a7ebf2c --- /dev/null +++ b/drivers/media/platform/sti/delta/delta-debug.c @@ -0,0 +1,72 @@ +/* + * Copyright (C) STMicroelectronics SA 2015 + * Authors: Hugues Fruchet + * Fabrice Lecoultre + * for STMicroelectronics. + * License terms: GNU General Public License (GPL), version 2 + */ + +#include "delta.h" +#include "delta-debug.h" + +char *delta_streaminfo_str(struct delta_streaminfo *s, char *str, + unsigned int len) +{ + if (!s) + return NULL; + + snprintf(str, len, +"%4.4s %dx%d %s %s dpb=%d %s %s %s%dx%d@(%d,%d) %s%d/%d", +(char *)>streamformat, s->width, s->height, +s->profile, s->level, s->dpb, +(s->field == V4L2_FIELD_NONE) ? "progressive" : "interlaced", +s->other, +s->flags & DELTA_STREAMINFO_FLAG_CROP ? "crop=" : "", +s->crop.width, s->crop.height, +s->crop.left, s->crop.top, +s->flags & DELTA_STREAMINFO_FLAG_PIXELASPECT ? "par=" : "", +s->pixelaspect.numerator, +s->pixelaspect.denominator); + + return str; +} + +char *delta_frameinfo_str(struct delta_frameinfo *f, char *str, + unsigned int len) +{ + if (!f) + return NULL; + + snprintf(str, len, +"%4.4s %dx%d aligned %dx%d %s %s%dx%d@(%d,%d) %s%d/%d", +(char *)>pixelformat, f->width, f->height, +f->aligned_width, f->aligned_height, +(f->field == V4L2_FIELD_NONE) ? "progressive" : "interlaced", +f->flags & DELTA_STREAMINFO_FLAG_CROP ? "crop=" : "", +f->crop.width, f->crop.height, +f->crop.left, f->crop.top, +f->flags & DELTA_STREAMINFO_FLAG_PIXELASPECT ? "par=" : "", +f->pixelaspect.numerator, +f->pixelaspect.denominator); + + return str; +} + +void delta_trace_summary(struct delta_ctx *ctx) +{ + struct delta_dev *delta = ctx->dev; + struct delta_streaminfo *s = >streaminfo; + unsigned char str[100] = ""; + + if (!(ctx->flags & DELTA_FLAG_STREAMINFO)) + return; + + dev_dbg(delta->dev, "%s %s, %d frames decoded, %d frames output, %d frames dropped, %d stream errors, %d decode errors", + ctx->name, + delta_streaminfo_str(s, str, sizeof(str)), + ctx->decoded_frames, + ctx->output_frames, + ctx->dropped_frames, + ctx->stream_errors, + ctx->decode_errors); +} diff --git a/drivers/media/platform/sti/delta/delta-debug.h b/drivers/media/platform/sti/delta/delta-debug.h new file mode 100644 index 000..955c158 --- /dev/null +++ b/drivers/media/platform/sti/delta/delta-debug.h @@ -0,0 +1,18 @@ +/* + * Copyright (C) STMicroelectronics SA 2015 + * Authors: Hugues Fruchet + * Fabrice Lecoultre + * for STMicroelectronics. + * License terms: GNU General Public License (GPL), version 2 + */ + +#ifndef DELTA_DEBUG_H +#define DELTA_DEBUG_H + +char *delta_streaminfo_str(struct delta_streaminfo *s, char *str, + unsigned int len); +char *delta_frameinfo_str(struct delta_frameinfo *f, char *str, +
[PATCH v5 06/10] [media] st-delta: add memory allocator helper functions
Helper functions used by decoder back-ends to allocate physically contiguous memory required by hardware video decoder. Signed-off-by: Hugues Fruchet--- drivers/media/platform/sti/delta/Makefile| 2 +- drivers/media/platform/sti/delta/delta-mem.c | 51 drivers/media/platform/sti/delta/delta-mem.h | 14 drivers/media/platform/sti/delta/delta.h | 8 + 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 drivers/media/platform/sti/delta/delta-mem.c create mode 100644 drivers/media/platform/sti/delta/delta-mem.h diff --git a/drivers/media/platform/sti/delta/Makefile b/drivers/media/platform/sti/delta/Makefile index 07ba7ad..cbfb1b5 100644 --- a/drivers/media/platform/sti/delta/Makefile +++ b/drivers/media/platform/sti/delta/Makefile @@ -1,2 +1,2 @@ obj-$(CONFIG_VIDEO_STI_DELTA) := st-delta.o -st-delta-y := delta-v4l2.o +st-delta-y := delta-v4l2.o delta-mem.o diff --git a/drivers/media/platform/sti/delta/delta-mem.c b/drivers/media/platform/sti/delta/delta-mem.c new file mode 100644 index 000..d7b53d3 --- /dev/null +++ b/drivers/media/platform/sti/delta/delta-mem.c @@ -0,0 +1,51 @@ +/* + * Copyright (C) STMicroelectronics SA 2015 + * Author: Hugues Fruchet for STMicroelectronics. + * License terms: GNU General Public License (GPL), version 2 + */ + +#include "delta.h" +#include "delta-mem.h" + +int hw_alloc(struct delta_ctx *ctx, u32 size, const char *name, +struct delta_buf *buf) +{ + struct delta_dev *delta = ctx->dev; + dma_addr_t dma_addr; + void *addr; + unsigned long attrs = DMA_ATTR_WRITE_COMBINE; + + addr = dma_alloc_attrs(delta->dev, size, _addr, + GFP_KERNEL | __GFP_NOWARN, attrs); + if (!addr) { + dev_err(delta->dev, + "%s hw_alloc:dma_alloc_coherent failed for %s (size=%d)\n", + ctx->name, name, size); + ctx->sys_errors++; + return -ENOMEM; + } + + buf->size = size; + buf->paddr = dma_addr; + buf->vaddr = addr; + buf->name = name; + buf->attrs = attrs; + + dev_dbg(delta->dev, + "%s allocate %d bytes of HW memory @(virt=0x%p, phy=0x%pad): %s\n", + ctx->name, size, buf->vaddr, >paddr, buf->name); + + return 0; +} + +void hw_free(struct delta_ctx *ctx, struct delta_buf *buf) +{ + struct delta_dev *delta = ctx->dev; + + dev_dbg(delta->dev, + "%s free %d bytes of HW memory @(virt=0x%p, phy=0x%pad): %s\n", + ctx->name, buf->size, buf->vaddr, >paddr, buf->name); + + dma_free_attrs(delta->dev, buf->size, + buf->vaddr, buf->paddr, buf->attrs); +} diff --git a/drivers/media/platform/sti/delta/delta-mem.h b/drivers/media/platform/sti/delta/delta-mem.h new file mode 100644 index 000..f8ca109 --- /dev/null +++ b/drivers/media/platform/sti/delta/delta-mem.h @@ -0,0 +1,14 @@ +/* + * Copyright (C) STMicroelectronics SA 2015 + * Author: Hugues Fruchet for STMicroelectronics. + * License terms: GNU General Public License (GPL), version 2 + */ + +#ifndef DELTA_MEM_H +#define DELTA_MEM_H + +int hw_alloc(struct delta_ctx *ctx, u32 size, const char *name, +struct delta_buf *buf); +void hw_free(struct delta_ctx *ctx, struct delta_buf *buf); + +#endif /* DELTA_MEM_H */ diff --git a/drivers/media/platform/sti/delta/delta.h b/drivers/media/platform/sti/delta/delta.h index 74a4240..9e26525 100644 --- a/drivers/media/platform/sti/delta/delta.h +++ b/drivers/media/platform/sti/delta/delta.h @@ -191,6 +191,14 @@ struct delta_dts { u64 val; }; +struct delta_buf { + u32 size; + void *vaddr; + dma_addr_t paddr; + const char *name; + unsigned long attrs; +}; + struct delta_ctx; /* -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 03/10] ARM: multi_v7_defconfig: enable STMicroelectronics DELTA Support
Enables support of STMicroelectronics STiH4xx SoC series DELTA multi-format video decoder V4L2 driver. Signed-off-by: Hugues Fruchet--- arch/arm/configs/multi_v7_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index b01a438..5dff8fe 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -569,6 +569,7 @@ CONFIG_VIDEO_SAMSUNG_S5P_MFC=m CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC=m CONFIG_VIDEO_STI_BDISP=m CONFIG_VIDEO_STI_HVA=m +CONFIG_VIDEO_STI_DELTA=m CONFIG_VIDEO_RENESAS_JPU=m CONFIG_VIDEO_RENESAS_VSP1=m CONFIG_V4L_TEST_DRIVERS=y -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 02/10] ARM: dts: STiH410: add DELTA dt node
This patch adds DT node for STMicroelectronics DELTA V4L2 video decoder Signed-off-by: Hugues Fruchet--- arch/arm/boot/dts/stih410.dtsi | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/boot/dts/stih410.dtsi b/arch/arm/boot/dts/stih410.dtsi index 281a124..42e070c 100644 --- a/arch/arm/boot/dts/stih410.dtsi +++ b/arch/arm/boot/dts/stih410.dtsi @@ -259,5 +259,15 @@ clocks = <_sysin>; interrupts = ; }; + delta0 { + compatible = "st,st-delta"; + clock-names = "delta", + "delta-st231", + "delta-flash-promip"; + clocks = <_s_c0_flexgen CLK_VID_DMU>, +<_s_c0_flexgen CLK_ST231_DMU>, +<_s_c0_flexgen CLK_FLASH_PROMIP>; + }; + }; }; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 04/10] [media] MAINTAINERS: add st-delta driver
Add entry for the STMicroelectronics DELTA driver. Signed-off-by: Hugues Fruchet--- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index cfff2c9..38cc652 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2429,6 +2429,14 @@ W: https://linuxtv.org S: Supported F: drivers/media/platform/sti/bdisp +DELTA ST MEDIA DRIVER +M: Hugues Fruchet +L: linux-media@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +W: https://linuxtv.org +S: Supported +F: drivers/media/platform/sti/delta + BEFS FILE SYSTEM M: Luis de Bethencourt M: Salah Triki -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: DRM Atomic property for color-space conversion
On Tue, Jan 31, 2017 at 05:15:46PM +0200, Ville Syrjälä wrote: On Tue, Jan 31, 2017 at 12:33:29PM +, Brian Starkey wrote: Hi, On Mon, Jan 30, 2017 at 03:35:13PM +0200, Ville Syrjälä wrote: >On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote: >> Hi, >> >> We're looking to enable the per-plane color management hardware in >> Mali-DP with atomic properties, which has sparked some conversation >> around how to handle YCbCr formats. >> >> As it stands today, it's assumed that a driver will implicitly "do the >> right thing" to display a YCbCr buffer. >> >> YCbCr data often uses different gamma curves and signal ranges (e.g. >> BT.609, BT.701, BT.2020, studio range, full-range), so its desirable >> to be able to explicitly control the YCbCr to RGB conversion process >> from userspace. >> >> We're proposing adding a "CSC" (color-space conversion) property to >> control this - primarily per-plane for framebuffer->pipeline CSC, but >> perhaps one per CRTC too for devices which have an RGB pipeline and >> want to output in YUV to the display: >> >> Name: "CSC" >> Type: ENUM | ATOMIC; >> Enum values (representative): >> "default": >>Same behaviour as now. "Some kind" of YCbCr->RGB conversion >>for YCbCr buffers, bypass for RGB buffers >> "disable": >>Explicitly disable all colorspace conversion (i.e. use an >>identity matrix). >> "YCbCr to RGB: BT.709": >>Only valid for YCbCr formats. CSC in accordance with BT.709 >>using [16..235] for (8-bit) luma values, and [16..240] for >>8-bit chroma values. For 10-bit formats, the range limits are >>multiplied by 4. >> "YCbCr to RGB: BT.709 full-swing": >>Only valid for YCbCr formats. CSC in accordance with BT.709, >>but using the full range of each channel. >> "YCbCr to RGB: Use CTM":* >>Only valid for YCbCr formats. Use the matrix applied via the >>plane's CTM property >> "RGB to RGB: Use CTM":* >>Only valid for RGB formats. Use the matrix applied via the >>plane's CTM property >> "Use CTM":* >>Valid for any format. Use the matrix applied via the plane's >>CTM property >> ... any other values for BT.601, BT.2020, RGB to YCbCr etc. etc. as >> they are required. > >Having some RGB2RGB and YCBCR2RGB things in the same property seems >weird. I would just go with something very simple like: > >YCBCR_TO_RGB_CSC: >* BT.601 >* BT.709 >* custom matrix > I think we've agreed in #dri-devel that this CSC property can't/shouldn't be mapped on-to the existing (hardware implementing the) CTM property - even in the case of per-plane color management - because CSC needs to be done before DEGAMMA. So, I'm in favour of going with what you suggested in the first place: A new YCBCR_TO_RGB_CSC property, enum type, with a list of fixed conversions. I'd drop the custom matrix for now, as we'd need to add another property to attach the custom matrix blob too. I still think we need a way to specify whether the source data range is broadcast/full-range, so perhaps the enum list should be expanded to all combinations of BT.601/BT.709 + broadcast/full-range. Sounds reasonable. Not that much full range YCbCr stuff out there perhaps. Well, apart from jpegs I suppose. But no harm in being able to deal with it. (I'm not sure what the canonical naming for broadcast/full-range is, we call them narrow and wide) We tend to call them full vs. limited range. That's how our "Broadcast RGB" property is defined as well. OK, using the same ones sounds sensible. >And trying to use the same thing for the crtc stuff is probably not >going to end well. Like Daniel said we already have the >'Broadcast RGB' property muddying the waters there, and that stuff >also ties in with what colorspace we signal to the sink via >infoframes/whatever the DP thing was called. So my gut feeling is >that trying to use the same property everywhere will just end up >messy. Yeah, agreed. If/when someone wants to add CSC on the output of a CRTC (after GAMMA), we can add a new property. That makes me wonder about calling this one SOURCE_YCBCR_TO_RGB_CSC to be explicit that it describes the source data. Then we can later add SINK_RGB_TO_YCBCR_CSC, and it will be reasonably obvious that its value describes the output data rather than the input data. Source and sink have a slight connotation in my mind wrt. the box that produces the display signal and the box that eats the signal. So trying to use the same terms to describe the internals of the pipeline inside the "source box" migth lead to some confusion. But we do probably need some decent names for these to make the layout of the pipeline clear. Input/output are the other names that popped to my mind but those aren't necessarily any better. But in the end I think I could live with whatever names we happen to pick, as long as we document the pipeline clearly. Long ago I did wonder if we should just start indexing these things
Re: [PATCH v3 17/24] media: imx: Add CSI subdev driver
On Fri, 2017-01-06 at 18:11 -0800, Steve Longerbeam wrote: [...] > +static int csi_set_fmt(struct v4l2_subdev *sd, > +struct v4l2_subdev_pad_config *cfg, > +struct v4l2_subdev_format *sdformat) > +{ > + struct csi_priv *priv = v4l2_get_subdevdata(sd); > + struct v4l2_mbus_framefmt *infmt, *outfmt; > + struct v4l2_rect crop; > + int ret; > + > + if (sdformat->pad >= CSI_NUM_PADS) > + return -EINVAL; > + > + if (priv->stream_on) > + return -EBUSY; > + > + infmt = >format_mbus[priv->input_pad]; > + outfmt = >format_mbus[priv->output_pad]; > + > + if (sdformat->pad == priv->output_pad) { > + sdformat->format.code = infmt->code; > + sdformat->format.field = infmt->field; > + crop.left = priv->crop.left; > + crop.top = priv->crop.top; > + crop.width = sdformat->format.width; > + crop.height = sdformat->format.height; > + ret = csi_try_crop(priv, ); This is the wrong way around, see also below. Here the the output sdformat->format.width/height should be set to the priv->crop.width/height (or priv->crop.width/height / 2, to enable downscaling). The crop rectangle should not be changed by an output pad set_fmt. > + if (ret) > + return ret; > + sdformat->format.width = crop.width; > + sdformat->format.height = crop.height; > + } > + > + if (sdformat->which == V4L2_SUBDEV_FORMAT_TRY) { > + cfg->try_fmt = sdformat->format; > + } else { > + priv->format_mbus[sdformat->pad] = sdformat->format; > + /* Update the crop window if this is output pad */ > + if (sdformat->pad == priv->output_pad) > + priv->crop = crop; The crop rectangle instead should be reset to the full input frame when the input pad format is set. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/11] media: rcar-vin: split rvin_s_fmt_vid_cap()
To support unbind and rebinding of subdevices rvin_s_fmt_vid_cap() needs to be called from with in the driver itself. Rename the function __rvin_s_fmt_vid_cap() and create a wrapper which can be used by vidioc_s_fmt_vid_cap. Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-v4l2.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 51324c6d826f76ea..929f58b49b06154d 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -313,10 +313,8 @@ static int rvin_try_fmt_vid_cap(struct file *file, void *priv, ); } -static int rvin_s_fmt_vid_cap(struct file *file, void *priv, - struct v4l2_format *f) +static int __rvin_s_fmt_vid_cap(struct rvin_dev *vin, struct v4l2_format *f) { - struct rvin_dev *vin = video_drvdata(file); struct rvin_source_fmt source; int ret; @@ -338,6 +336,14 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv, return 0; } +static int rvin_s_fmt_vid_cap(struct file *file, void *priv, + struct v4l2_format *f) +{ + struct rvin_dev *vin = video_drvdata(file); + + return __rvin_s_fmt_vid_cap(vin, f); +} + static int rvin_g_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f) { -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/11] media: rcar-vin: use pad information when verifying media bus format
Use information about pad index when enumerating mbus codes. Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c index e9373d9ab97fb827..50058fe9e37d8771 100644 --- a/drivers/media/platform/rcar-vin/rcar-core.c +++ b/drivers/media/platform/rcar-vin/rcar-core.c @@ -48,6 +48,7 @@ static bool rvin_mbus_supported(struct rvin_graph_entity *entity) }; code.index = 0; + code.pad = entity->source_pad_idx; while (!v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, )) { code.index++; switch (code.code) { -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/11] media: rcar-vin: fix standard in input enumeration
If the subdevice supports dv_timings_cap the driver should not fill in the standard. Also don't use the standard from probe time ask the subdevice each time. Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-v4l2.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 610f59e2a9142622..f9218f230322eb0b 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -483,10 +483,16 @@ static int rvin_enum_input(struct file *file, void *priv, return ret; i->type = V4L2_INPUT_TYPE_CAMERA; - i->std = vin->vdev.tvnorms; - if (v4l2_subdev_has_op(sd, pad, dv_timings_cap)) + if (v4l2_subdev_has_op(sd, pad, dv_timings_cap)) { i->capabilities = V4L2_IN_CAP_DV_TIMINGS; + i->std = 0; + } else { + i->capabilities = V4L2_IN_CAP_STD; + ret = v4l2_subdev_call(sd, video, g_tvnorms, >std); + if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV) + return ret; + } strlcpy(i->name, "Camera", sizeof(i->name)); -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/11] media: rcar-vin: add wrapper to get rvin_graph_entity
Update the driver to retrieve the code and mbus_cfg values from a rvin_graph_entity retrieved from a wrapper function instead of directly accessing the entity for the digital port. This is done to prepare for Gen3 support where the subdeivce might change during runtime, so to directly accesses a specific rvin_graph_entity is bad. Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-core.c | 9 + drivers/media/platform/rcar-vin/rcar-dma.c | 15 ++- drivers/media/platform/rcar-vin/rcar-v4l2.c | 6 +- drivers/media/platform/rcar-vin/rcar-vin.h | 1 + 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c index 098a0b1cc10a26ba..89a9280efa05aa0c 100644 --- a/drivers/media/platform/rcar-vin/rcar-core.c +++ b/drivers/media/platform/rcar-vin/rcar-core.c @@ -26,6 +26,15 @@ #include "rcar-vin.h" /* - + * Subdevice helpers + */ + +struct rvin_graph_entity *vin_to_entity(struct rvin_dev *vin) +{ + return >digital; +} + +/* - * Async notifier */ diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c index 9ccd5ff55e192514..eac5c192b887ea45 100644 --- a/drivers/media/platform/rcar-vin/rcar-dma.c +++ b/drivers/media/platform/rcar-vin/rcar-dma.c @@ -131,10 +131,15 @@ static u32 rvin_read(struct rvin_dev *vin, u32 offset) static int rvin_setup(struct rvin_dev *vin) { + struct rvin_graph_entity *rent; u32 vnmc, dmr, dmr2, interrupts; v4l2_std_id std; bool progressive = false, output_is_yuv = false, input_is_yuv = false; + rent = vin_to_entity(vin); + if (!rent) + return -ENODEV; + switch (vin->format.field) { case V4L2_FIELD_TOP: vnmc = VNMC_IM_ODD; @@ -174,7 +179,7 @@ static int rvin_setup(struct rvin_dev *vin) /* * Input interface */ - switch (vin->digital.code) { + switch (rent->code) { case MEDIA_BUS_FMT_YUYV8_1X16: /* BT.601/BT.1358 16bit YCbCr422 */ vnmc |= VNMC_INF_YUV16; @@ -182,7 +187,7 @@ static int rvin_setup(struct rvin_dev *vin) break; case MEDIA_BUS_FMT_UYVY8_2X8: /* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */ - vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ? + vnmc |= rent->mbus_cfg.type == V4L2_MBUS_BT656 ? VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601; input_is_yuv = true; break; @@ -191,7 +196,7 @@ static int rvin_setup(struct rvin_dev *vin) break; case MEDIA_BUS_FMT_UYVY10_2X10: /* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */ - vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ? + vnmc |= rent->mbus_cfg.type == V4L2_MBUS_BT656 ? VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601; input_is_yuv = true; break; @@ -203,11 +208,11 @@ static int rvin_setup(struct rvin_dev *vin) dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1); /* Hsync Signal Polarity Select */ - if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)) + if (!(rent->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)) dmr2 |= VNDMR2_HPS; /* Vsync Signal Polarity Select */ - if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)) + if (!(rent->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)) dmr2 |= VNDMR2_VPS; /* diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index f9218f230322eb0b..370bb184e5faace7 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -164,6 +164,7 @@ static int __rvin_try_format_source(struct rvin_dev *vin, { struct v4l2_subdev *sd; struct v4l2_subdev_pad_config *pad_cfg; + struct rvin_graph_entity *rent; struct v4l2_subdev_format format = { .which = which, }; @@ -171,8 +172,11 @@ static int __rvin_try_format_source(struct rvin_dev *vin, int ret; sd = vin_to_source(vin); + rent = vin_to_entity(vin); + if (!rent) + return -ENODEV; - v4l2_fill_mbus_format(, pix, vin->digital.code); + v4l2_fill_mbus_format(, pix, rent->code); pad_cfg = v4l2_subdev_alloc_pad_config(sd); if (pad_cfg == NULL) diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h index 727e215c08718eb9..daec26a38d896b21 100644 ---
[PATCH 01/11] media: rcar-vin: reset bytesperline and sizeimage when resetting format
These two fields where forgotten when refactoring the format reset code path. If they are not also reset at the same time as width and hight the format read using G_FMT will not match reality. Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-v4l2.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 2bbe6d495fa634da..69bc4cfea6a8aeb5 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -151,6 +151,9 @@ static int rvin_reset_format(struct rvin_dev *vin) rvin_reset_crop_compose(vin); + vin->format.bytesperline = rvin_format_bytesperline(>format); + vin->format.sizeimage = rvin_format_sizeimage(>format); + return 0; } -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/11] media: rcar-vin: move pad index discovery to async complete handler
To fix support for unbind and rebinding of subdevices the rvin_v4l2_probe() needs to be called before there might be any subdevice bound. Move pad index discovery to when we know the subdevice is present. Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-core.c | 23 +++ drivers/media/platform/rcar-vin/rcar-v4l2.c | 18 +- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c index 89a9280efa05aa0c..2c40b6a1a93f108c 100644 --- a/drivers/media/platform/rcar-vin/rcar-core.c +++ b/drivers/media/platform/rcar-vin/rcar-core.c @@ -68,6 +68,8 @@ static bool rvin_mbus_supported(struct rvin_graph_entity *entity) static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier) { struct rvin_dev *vin = notifier_to_vin(notifier); + struct v4l2_subdev *sd = vin->digital.subdev; + unsigned int pad_idx; int ret; /* Verify subdevices mbus format */ @@ -80,6 +82,27 @@ static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier) vin_dbg(vin, "Found media bus format for %s: %d\n", vin->digital.subdev->name, vin->digital.code); + /* Figure out source and sink pad ids */ + vin->digital.source_pad_idx = 0; + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++) + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SOURCE) + break; + if (pad_idx >= sd->entity.num_pads) + return -EINVAL; + + vin->digital.source_pad_idx = pad_idx; + + vin->digital.sink_pad_idx = 0; + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++) + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) { + vin->digital.sink_pad_idx = pad_idx; + break; + } + + vin_dbg(vin, "Found media pads for %s source: %d sink %d\n", + vin->digital.subdev->name, vin->digital.source_pad_idx, + vin->digital.sink_pad_idx); + ret = v4l2_device_register_subdev_nodes(>v4l2_dev); if (ret < 0) { vin_err(vin, "Failed to register subdev nodes\n"); diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index f8ff7c43944dd64a..51324c6d826f76ea 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -905,7 +905,7 @@ int rvin_v4l2_probe(struct rvin_dev *vin) { struct video_device *vdev = >vdev; struct v4l2_subdev *sd = vin_to_source(vin); - int pad_idx, ret; + int ret; v4l2_set_subdev_hostdata(sd, vin); @@ -951,22 +951,6 @@ int rvin_v4l2_probe(struct rvin_dev *vin) vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING | V4L2_CAP_READWRITE; - vin->digital.source_pad_idx = 0; - for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++) - if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SOURCE) - break; - if (pad_idx >= sd->entity.num_pads) - return -EINVAL; - - vin->digital.source_pad_idx = pad_idx; - - vin->digital.sink_pad_idx = 0; - for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++) - if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) { - vin->digital.sink_pad_idx = pad_idx; - break; - } - vin->format.pixelformat = RVIN_DEFAULT_FORMAT; rvin_reset_format(vin); -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/11] media: rcar-vin: fix OPS and format/pad index issues
Hi, This series address issues with the R-Car Gen2 VIN driver. The most serious issue is the OPS when unbind and rebinding the i2c driver for the video source subdevice which have popped up as a blocker for other work. This series is broken out of my much larger R-Car Gen3 enablement series '[PATCHv2 00/32] rcar-vin: Add Gen3 with media controller support'. I plan to remove that series form patchwork and focus on these fixes first as they are blocking other development. Once the blocking issues are removed I will rebase and repost the larger Gen3 series. Patch 1-4 fix simple problems found while testing 1-2 Fix format problems when the format is (re)set. 3 Fix media pad errors 4 Fix standard enumeration problem Patch 5 adds a wrapper function to retrieve the active video source subdevice. This is strictly not needed on Gen2 which only have one possible video source per VIN instance (This will change on Gen3). But patch 6-8,11 which fixes real issues on Gen2 make use of this wrapper, as not risk breaking things by removing this wrapper in this series and then readding it in a later Gen3 series I have chosen to keep the patch. Please let me know if I should drop it and rewrite patch 6-11 (if possible I would like to avoid that). Patch 6-8 deals with video source subdevice pad index handling by moving the information from struct rvin_dev to struct rvin_graph_entity and moving the pad index probing to the struct v4l2_async_notifier complete callback. This is needed to allow the bind/unbind fix in patch 10-11. Patch 9 use the pad information when calling enum_mbus_code. Patch 10-11 fix a OPS when unbinding/binding the video source subdevice. # echo 2-0020 > /sys/bus/i2c/drivers/adv7180/unbind # echo 2-0020 > /sys/bus/i2c/drivers/adv7180/bind adv7180 2-0020: chip found @ 0x20 (e653.i2c) kobject (eaaab118): tried to init an initialized object, something is seriously wrong. CPU: 0 PID: 1640 Comm: bash Not tainted 4.10.0-rc4-00029-g19b80f8913cad837 #1 Hardware name: Generic R8A7791 (Flattened Device Tree) Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r7:0016 r6:60070013 r5: r4:c0a14dd8 [] (show_stack) from [] (dump_stack+0x84/0xa0) [] (dump_stack) from [] (kobject_init+0x3c/0x98) r7:0016 r6:eaaab2e4 r5:c0a1f4dc r4:eaaab118 [] (kobject_init) from [] (device_initialize+0x28/0xb0) r5:c0a70be8 r4:eaaab110 [] (device_initialize) from [] (device_register+0x14/0x20) r5:eaaab110 r4:eaaab110 [] (device_register) from [] (__video_register_device+0xb38/0x11cc) r5:eaaab110 r4:eaaab020 [] (__video_register_device) from [] (rvin_v4l2_probe+0x17c/0x1e8) r10: r9:eaa3c050 r8:c0a270a8 r7:eaaab3a0 r6:eaaab020 r5:c0790068 r4:eaaab010 [] (rvin_v4l2_probe) from [] (rvin_digital_notify_complete+0x174/0x184) r7:2006 r6:eaaab010 r5: r4:eaaab3e0 [] (rvin_digital_notify_complete) from [] (v4l2_async_test_notify+0xe8/0xf0) r7:eaaab410 r6:eaa3c050 r5:c04c6c2c r4:eaaab3e0 [] (v4l2_async_test_notify) from [] (v4l2_async_register_subdev+0xa4/0xcc) r7:eaa3c0fc r6:c0a27094 r5:eaaab3e0 r4:eaa3c050 [] (v4l2_async_register_subdev) from [] (adv7180_probe+0x350/0x3e0) r9:eaa3c050 r8: r7: r6: r5:eb2cbe00 r4:eaa3c010 [] (adv7180_probe) from [] (i2c_device_probe+0x238/0x250) r9:000e r8:c0a264dc r7:eb2cbe20 r6:c0a264dc r5:c04973f0 r4:eb2cbe00 [] (i2c_device_probe) from [] (driver_probe_device+0x1f8/0x2c0) r9:000e r8:c0a264dc r7: r6:c0a70c18 r5:c0a70c0c r4:eb2cbe20 [] (driver_probe_device) from [] (bind_store+0x94/0xe8) r10: r9:0051 r8:0007 r7:c0a26058 r6:eb2cbe54 r5:c0a264dc r4:eb2cbe20 r3:ea60b000 [] (bind_store) from [] (drv_attr_store+0x2c/0x38) r9:0051 r8:eb2daa0c r7:ea58ff80 r6:eb2daa00 r5:ea87a4c0 r4:c03bbc3c [] (drv_attr_store) from [] (sysfs_kf_write+0x40/0x4c) r5:ea87a4c0 r4:c03bb6e4 [] (sysfs_kf_write) from [] (kernfs_fop_write+0x13c/0x1ac) r5:ea87a4c0 r4:0007 [] (kernfs_fop_write) from [] (__vfs_write+0x34/0x114) r9:ea58e000 r8: r7:0007 r6:ea58ff80 r5:ea52a480 r4:c023db14 [] (__vfs_write) from [] (vfs_write+0xc4/0x150) r7:ea58ff80 r6:00167028 r5:0007 r4:ea52a480 [] (vfs_write) from [] (SyS_write+0x48/0x80) r9:ea58e000 r8:c0106ee4 r7:0007 r6:00167028 r5:ea52a480 r4:ea52a480 [] (SyS_write) from [] (ret_fast_syscall+0x0/0x3c) r7:0004 r6:b6dfed50 r5:00167028 r4:0007 Niklas Söderlund (11): media: rcar-vin: reset bytesperline and sizeimage when resetting format media: rcar-vin: use rvin_reset_format() in S_DV_TIMINGS media: rcar-vin: fix how pads are handled for v4l2 subdeivce operations media: rcar-vin: fix standard in input enumeration media: rcar-vin: add wrapper to get rvin_graph_entity media: rcar-vin: move subdev source and sink pad index to rvin_graph_entity media: rcar-vin: move pad index discovery to async complete handler media: rcar-vin: refactor
[PATCH 06/11] media: rcar-vin: move subdev source and sink pad index to rvin_graph_entity
It makes more sens to store the sink and source pad index in struct rvin_graph_entity since that contains other subdevice related information. Add complete documentation for struct rvin_graph_entity while we are at it. Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-v4l2.c | 45 ++--- drivers/media/platform/rcar-vin/rcar-vin.h | 17 ++- 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 370bb184e5faace7..f8ff7c43944dd64a 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -109,9 +109,14 @@ static int rvin_reset_format(struct rvin_dev *vin) .which = V4L2_SUBDEV_FORMAT_ACTIVE, }; struct v4l2_mbus_framefmt *mf = + struct rvin_graph_entity *rent; int ret; - fmt.pad = vin->src_pad_idx; + rent = vin_to_entity(vin); + if (!rent) + return -ENODEV; + + fmt.pad = rent->source_pad_idx; ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, ); if (ret) @@ -182,7 +187,7 @@ static int __rvin_try_format_source(struct rvin_dev *vin, if (pad_cfg == NULL) return -ENOMEM; - format.pad = vin->src_pad_idx; + format.pad = rent->source_pad_idx; field = pix->field; @@ -560,12 +565,17 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh, { struct rvin_dev *vin = video_drvdata(file); struct v4l2_subdev *sd = vin_to_source(vin); + struct rvin_graph_entity *rent; int ret; + rent = vin_to_entity(vin); + if (!rent) + return -ENODEV; + if (timings->pad) return -EINVAL; - timings->pad = vin->sink_pad_idx; + timings->pad = rent->sink_pad_idx; ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings); @@ -612,12 +622,17 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh, { struct rvin_dev *vin = video_drvdata(file); struct v4l2_subdev *sd = vin_to_source(vin); + struct rvin_graph_entity *rent; int ret; + rent = vin_to_entity(vin); + if (!rent) + return -ENODEV; + if (cap->pad) return -EINVAL; - cap->pad = vin->sink_pad_idx; + cap->pad = rent->sink_pad_idx; ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap); @@ -630,12 +645,17 @@ static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid) { struct rvin_dev *vin = video_drvdata(file); struct v4l2_subdev *sd = vin_to_source(vin); + struct rvin_graph_entity *rent; int ret; + rent = vin_to_entity(vin); + if (!rent) + return -ENODEV; + if (edid->pad) return -EINVAL; - edid->pad = vin->sink_pad_idx; + edid->pad = rent->sink_pad_idx; ret = v4l2_subdev_call(sd, pad, get_edid, edid); @@ -648,12 +668,17 @@ static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid) { struct rvin_dev *vin = video_drvdata(file); struct v4l2_subdev *sd = vin_to_source(vin); + struct rvin_graph_entity *rent; int ret; + rent = vin_to_entity(vin); + if (!rent) + return -ENODEV; + if (edid->pad) return -EINVAL; - edid->pad = vin->sink_pad_idx; + edid->pad = rent->sink_pad_idx; ret = v4l2_subdev_call(sd, pad, set_edid, edid); @@ -926,19 +951,19 @@ int rvin_v4l2_probe(struct rvin_dev *vin) vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING | V4L2_CAP_READWRITE; - vin->src_pad_idx = 0; + vin->digital.source_pad_idx = 0; for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++) if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SOURCE) break; if (pad_idx >= sd->entity.num_pads) return -EINVAL; - vin->src_pad_idx = pad_idx; + vin->digital.source_pad_idx = pad_idx; - vin->sink_pad_idx = 0; + vin->digital.sink_pad_idx = 0; for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++) if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) { - vin->sink_pad_idx = pad_idx; + vin->digital.sink_pad_idx = pad_idx; break; } diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h index daec26a38d896b21..d31212a992e15506 100644 --- a/drivers/media/platform/rcar-vin/rcar-vin.h +++ b/drivers/media/platform/rcar-vin/rcar-vin.h @@ -70,10 +70,12 @@ struct rvin_video_format { /** * struct rvin_graph_entity - Video
[PATCH 03/11] media: rcar-vin: fix how pads are handled for v4l2 subdeivce operations
The rcar-vin driver only uses one pad, pad number 0. All v4l2 operations which did not check that the requested operation was for pad 0 have been updated with a check to enforce this. All v4l2 operations that stored (and later restore) the requested pad before substituting it for the subdeivce pad number have been update not store the incoming pad and simply restore it to 0 after the subdevice operation is complete. Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-v4l2.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 7ca27599b9982ffc..610f59e2a9142622 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -550,14 +550,16 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh, { struct rvin_dev *vin = video_drvdata(file); struct v4l2_subdev *sd = vin_to_source(vin); - int pad, ret; + int ret; + + if (timings->pad) + return -EINVAL; - pad = timings->pad; timings->pad = vin->sink_pad_idx; ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings); - timings->pad = pad; + timings->pad = 0; return ret; } @@ -600,14 +602,16 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh, { struct rvin_dev *vin = video_drvdata(file); struct v4l2_subdev *sd = vin_to_source(vin); - int pad, ret; + int ret; + + if (cap->pad) + return -EINVAL; - pad = cap->pad; cap->pad = vin->sink_pad_idx; ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap); - cap->pad = pad; + cap->pad = 0; return ret; } @@ -616,17 +620,16 @@ static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid) { struct rvin_dev *vin = video_drvdata(file); struct v4l2_subdev *sd = vin_to_source(vin); - int input, ret; + int ret; if (edid->pad) return -EINVAL; - input = edid->pad; edid->pad = vin->sink_pad_idx; ret = v4l2_subdev_call(sd, pad, get_edid, edid); - edid->pad = input; + edid->pad = 0; return ret; } @@ -635,17 +638,16 @@ static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid) { struct rvin_dev *vin = video_drvdata(file); struct v4l2_subdev *sd = vin_to_source(vin); - int input, ret; + int ret; if (edid->pad) return -EINVAL; - input = edid->pad; edid->pad = vin->sink_pad_idx; ret = v4l2_subdev_call(sd, pad, set_edid, edid); - edid->pad = input; + edid->pad = 0; return ret; } -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/11] media: rcar-vin: register the video device early
To support unbind and rebinding of video source subdevices while keeping a constant video device the subdevice needs to be attached when the first user opens the video device and detached when the last user closes it. This changes the rcar-vin behavior in such way that the video device is registered at probe time while attaching to a subdeivce is done only when the video device is opened. If at that time there is no subdevice bound to the rcar-vin driver the open will fail with -EBUSY until a subdevice are bound to the rcar-vin driver using the V4L2 asynchronous subdevice API. This changes fix an OPS when first unbinding a subdevice and later rebinding it. # echo 2-0020 > /sys/bus/i2c/drivers/adv7180/unbind # echo 2-0020 > /sys/bus/i2c/drivers/adv7180/bind adv7180 2-0020: chip found @ 0x20 (e653.i2c) kobject (eaaab118): tried to init an initialized object, something is seriously wrong. CPU: 0 PID: 1640 Comm: bash Not tainted 4.10.0-rc4-00029-g19b80f8913cad837 #1 Hardware name: Generic R8A7791 (Flattened Device Tree) Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r7:0016 r6:60070013 r5: r4:c0a14dd8 [] (show_stack) from [] (dump_stack+0x84/0xa0) [] (dump_stack) from [] (kobject_init+0x3c/0x98) r7:0016 r6:eaaab2e4 r5:c0a1f4dc r4:eaaab118 [] (kobject_init) from [] (device_initialize+0x28/0xb0) r5:c0a70be8 r4:eaaab110 [] (device_initialize) from [] (device_register+0x14/0x20) r5:eaaab110 r4:eaaab110 [] (device_register) from [] (__video_register_device+0xb38/0x11cc) r5:eaaab110 r4:eaaab020 [] (__video_register_device) from [] (rvin_v4l2_probe+0x17c/0x1e8) r10: r9:eaa3c050 r8:c0a270a8 r7:eaaab3a0 r6:eaaab020 r5:c0790068 r4:eaaab010 [] (rvin_v4l2_probe) from [] (rvin_digital_notify_complete+0x174/0x184) r7:2006 r6:eaaab010 r5: r4:eaaab3e0 [] (rvin_digital_notify_complete) from [] (v4l2_async_test_notify+0xe8/0xf0) r7:eaaab410 r6:eaa3c050 r5:c04c6c2c r4:eaaab3e0 [] (v4l2_async_test_notify) from [] (v4l2_async_register_subdev+0xa4/0xcc) r7:eaa3c0fc r6:c0a27094 r5:eaaab3e0 r4:eaa3c050 [] (v4l2_async_register_subdev) from [] (adv7180_probe+0x350/0x3e0) r9:eaa3c050 r8: r7: r6: r5:eb2cbe00 r4:eaa3c010 [] (adv7180_probe) from [] (i2c_device_probe+0x238/0x250) r9:000e r8:c0a264dc r7:eb2cbe20 r6:c0a264dc r5:c04973f0 r4:eb2cbe00 [] (i2c_device_probe) from [] (driver_probe_device+0x1f8/0x2c0) r9:000e r8:c0a264dc r7: r6:c0a70c18 r5:c0a70c0c r4:eb2cbe20 [] (driver_probe_device) from [] (bind_store+0x94/0xe8) r10: r9:0051 r8:0007 r7:c0a26058 r6:eb2cbe54 r5:c0a264dc r4:eb2cbe20 r3:ea60b000 [] (bind_store) from [] (drv_attr_store+0x2c/0x38) r9:0051 r8:eb2daa0c r7:ea58ff80 r6:eb2daa00 r5:ea87a4c0 r4:c03bbc3c [] (drv_attr_store) from [] (sysfs_kf_write+0x40/0x4c) r5:ea87a4c0 r4:c03bb6e4 [] (sysfs_kf_write) from [] (kernfs_fop_write+0x13c/0x1ac) r5:ea87a4c0 r4:0007 [] (kernfs_fop_write) from [] (__vfs_write+0x34/0x114) r9:ea58e000 r8: r7:0007 r6:ea58ff80 r5:ea52a480 r4:c023db14 [] (__vfs_write) from [] (vfs_write+0xc4/0x150) r7:ea58ff80 r6:00167028 r5:0007 r4:ea52a480 [] (vfs_write) from [] (SyS_write+0x48/0x80) r9:ea58e000 r8:c0106ee4 r7:0007 r6:00167028 r5:ea52a480 r4:ea52a480 [] (SyS_write) from [] (ret_fast_syscall+0x0/0x3c) r7:0004 r6:b6dfed50 r5:00167028 r4:0007 Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-core.c | 10 +- drivers/media/platform/rcar-vin/rcar-v4l2.c | 226 ++-- drivers/media/platform/rcar-vin/rcar-vin.h | 2 + 3 files changed, 121 insertions(+), 117 deletions(-) diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c index 50058fe9e37d8771..c86e71ad369cb929 100644 --- a/drivers/media/platform/rcar-vin/rcar-core.c +++ b/drivers/media/platform/rcar-vin/rcar-core.c @@ -107,7 +107,7 @@ static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier) return ret; } - return rvin_v4l2_probe(vin); + return 0; } static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier, @@ -118,7 +118,6 @@ static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier, if (vin->digital.subdev == subdev) { vin_dbg(vin, "unbind digital subdev %s\n", subdev->name); - rvin_v4l2_remove(vin); vin->digital.subdev = NULL; return; } @@ -283,6 +282,7 @@ static int rcar_vin_probe(struct platform_device *pdev) vin->dev = >dev; vin->chip = (enum chip_id)match->data; + vin->last_input = NULL; mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (mem == NULL) @@ -304,6 +304,10 @@ static int rcar_vin_probe(struct platform_device *pdev) if (ret < 0)
[PATCH 02/11] media: rcar-vin: use rvin_reset_format() in S_DV_TIMINGS
Use rvin_reset_format() in rvin_s_dv_timings() instead if just resetting a few fields. This fixes an issue where the field format was not properly set after S_DV_TIMINGS. Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-v4l2.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 69bc4cfea6a8aeb5..7ca27599b9982ffc 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -573,12 +573,8 @@ static int rvin_s_dv_timings(struct file *file, void *priv_fh, if (ret) return ret; - vin->source.width = timings->bt.width; - vin->source.height = timings->bt.height; - vin->format.width = timings->bt.width; - vin->format.height = timings->bt.height; - - return 0; + /* Changing the timings will change the width/height */ + return rvin_reset_format(vin); } static int rvin_g_dv_timings(struct file *file, void *priv_fh, -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/11] media: rcar-vin: refactor pad lookup code
If the subdeivce did not supply pad information the driver will return -EINVAL, this is not what we want so remove that check. The code can then be broken out to a helper function reducing duplication. Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-core.c | 29 + 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c index 2c40b6a1a93f108c..e9373d9ab97fb827 100644 --- a/drivers/media/platform/rcar-vin/rcar-core.c +++ b/drivers/media/platform/rcar-vin/rcar-core.c @@ -65,11 +65,21 @@ static bool rvin_mbus_supported(struct rvin_graph_entity *entity) return false; } +static unsigned int rvin_pad_idx(struct v4l2_subdev *sd, int direction) +{ + unsigned int pad_idx; + + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++) + if (sd->entity.pads[pad_idx].flags == direction) + return pad_idx; + + return 0; +} + static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier) { struct rvin_dev *vin = notifier_to_vin(notifier); struct v4l2_subdev *sd = vin->digital.subdev; - unsigned int pad_idx; int ret; /* Verify subdevices mbus format */ @@ -83,21 +93,8 @@ static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier) vin->digital.subdev->name, vin->digital.code); /* Figure out source and sink pad ids */ - vin->digital.source_pad_idx = 0; - for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++) - if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SOURCE) - break; - if (pad_idx >= sd->entity.num_pads) - return -EINVAL; - - vin->digital.source_pad_idx = pad_idx; - - vin->digital.sink_pad_idx = 0; - for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++) - if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) { - vin->digital.sink_pad_idx = pad_idx; - break; - } + vin->digital.source_pad_idx = rvin_pad_idx(sd, MEDIA_PAD_FL_SOURCE); + vin->digital.sink_pad_idx = rvin_pad_idx(sd, MEDIA_PAD_FL_SINK); vin_dbg(vin, "Found media pads for %s source: %d sink %d\n", vin->digital.subdev->name, vin->digital.source_pad_idx, -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL FOR v4.11] New st-delta driver
On 01/30/2017 08:18 PM, Mauro Carvalho Chehab wrote: > Em Mon, 30 Jan 2017 17:15:36 -0200 > Mauro Carvalho Chehabescreveu: > >> Em Mon, 9 Jan 2017 14:23:33 +0100 >> Hans Verkuil escreveu: >> >>> See the v4 series for details: >>> >>> https://www.spinics.net/lists/linux-media/msg108737.html >>> >>> Regards, >>> >>> Hans >>> >>> The following changes since commit 40eca140c404505c09773d1c6685d818cb55ab1a: >>> >>> [media] mn88473: add DVB-T2 PLP support (2016-12-27 14:00:15 -0200) >>> >>> are available in the git repository at: >>> >>> git://linuxtv.org/hverkuil/media_tree.git delta >>> >>> for you to fetch changes up to e6f199d01e7b8bc4436738b6c666fda31b9f3340: >>> >>> st-delta: debug: trace stream/frame information & summary (2017-01-09 >>> 14:16:45 +0100) >>> >>> >>> Hugues Fruchet (10): >>> Documentation: DT: add bindings for ST DELTA >>> ARM: dts: STiH410: add DELTA dt node >>> ARM: multi_v7_defconfig: enable STMicroelectronics DELTA Support >>> MAINTAINERS: add st-delta driver >>> st-delta: STiH4xx multi-format video decoder v4l2 driver >>> st-delta: add memory allocator helper functions >>> st-delta: rpmsg ipc support >>> st-delta: EOS (End Of Stream) support >>> st-delta: add mjpeg support >>> st-delta: debug: trace stream/frame information & summary >> >> There is something wrong on this driver... even after applying all >> patches, it complains that there's a for there that does nothing: >> >> drivers/media/platform/sti/delta/delta-v4l2.c:322 register_decoders() warn: >> we never enter this loop >> drivers/media/platform/sti/delta/delta-v4l2.c: In function >> 'register_decoders': >> drivers/media/platform/sti/delta/delta-v4l2.c:322:16: warning: comparison of >> unsigned expression < 0 is always false [-Wtype-limits] >> for (i = 0; i < ARRAY_SIZE(delta_decoders); i++) { >> ^ Hi Mauro, It's strange that you face this warning, code is like that: /* registry of available decoders */ static const struct delta_dec *delta_decoders[] = { #ifdef CONFIG_VIDEO_STI_DELTA_MJPEG , #endif }; and MJPEG config is enabled by default: config VIDEO_STI_DELTA_MJPEG bool "STMicroelectronics DELTA MJPEG support" default y so you should not encounter this warning. On the other hand, you face issue on line 322 of delta-v4l2.c but in my codebase, and also in Hans' git tree (git://linuxtv.org/hverkuil/media_tree.git delta), this code is at line 323. Anyway, in order to prevent such warning even if no decoder are selected in config, I have reworked the code in v5 adding a "NULL" element at the end of decoder array out of any config switch: static const struct delta_dec *delta_decoders[] = { #ifdef CONFIG_VIDEO_STI_DELTA_MJPEG , #endif NULL, }; >> >> On a first glance, it seems that the register_decoders() function is >> reponsible to register the format decoders that the hardware >> recognizes. If so, I suspect that this driver is deadly broken. >> >> Please be sure that the upstream driver works properly before >> submitting it upstream. >> >> Also, please fix the comments to match the Kernel standard. E. g. >> instead of: >> >> /* guard output frame count: >> * - at least 1 frame needed for display >> * - at worst 21 >> * ( max h264 dpb (16) + >> * decoding peak smoothing (2) + >> * user display pipeline (3) ) >> */ >> >> It should be: >> >> /* >> * guard output frame count: >> * - at least 1 frame needed for display >> * - at worst 21 >> * ( max h264 dpb (16) + >> * decoding peak smoothing (2) + >> * user display pipeline (3) ) >> */ >> >> There are several similar occurrences among this patch series. I apologize for this -unfortunately not raised by checkpatch, I will have a look to fix it- Multiple lines comments are now fixed in v5. > > Ah, forgot to comment, but it mentions a firmware. Does such firmware > reside on some RAM memory? If so, how such firmware is loaded? Firmware is loaded in coprocessor at system startup by remoteproc framework: From "[GIT PULL] STi DT update for v4.11 round 1" https://lkml.org/lkml/2017/1/12/525: https://kernel.googlesource.com/pub/scm/linux/kernel/git/pchotard/sti/+/sti-dt-for-v4.11/arch/arm/boot/dts/stih407-family.dtsi st231_delta: remote-processor { compatible = "st,st231-rproc"; memory-region = <_reserved>; resets = < STIH407_ST231_DMU_SOFTRESET>; reset-names = "sw_reset"; clocks = <_s_c0_flexgen CLK_ST231_DMU>; clock-frequency = <6>; st,syscfg = <_core 0x224>; #mbox-cells = <1>; mbox-names = "vq0_rx", "vq0_tx", "vq1_rx",
Re: DRM Atomic property for color-space conversion
On Tue, Jan 31, 2017 at 12:33:29PM +, Brian Starkey wrote: > Hi, > > On Mon, Jan 30, 2017 at 03:35:13PM +0200, Ville Syrjälä wrote: > >On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote: > >> Hi, > >> > >> We're looking to enable the per-plane color management hardware in > >> Mali-DP with atomic properties, which has sparked some conversation > >> around how to handle YCbCr formats. > >> > >> As it stands today, it's assumed that a driver will implicitly "do the > >> right thing" to display a YCbCr buffer. > >> > >> YCbCr data often uses different gamma curves and signal ranges (e.g. > >> BT.609, BT.701, BT.2020, studio range, full-range), so its desirable > >> to be able to explicitly control the YCbCr to RGB conversion process > >> from userspace. > >> > >> We're proposing adding a "CSC" (color-space conversion) property to > >> control this - primarily per-plane for framebuffer->pipeline CSC, but > >> perhaps one per CRTC too for devices which have an RGB pipeline and > >> want to output in YUV to the display: > >> > >> Name: "CSC" > >> Type: ENUM | ATOMIC; > >> Enum values (representative): > >> "default": > >>Same behaviour as now. "Some kind" of YCbCr->RGB conversion > >>for YCbCr buffers, bypass for RGB buffers > >> "disable": > >>Explicitly disable all colorspace conversion (i.e. use an > >>identity matrix). > >> "YCbCr to RGB: BT.709": > >>Only valid for YCbCr formats. CSC in accordance with BT.709 > >>using [16..235] for (8-bit) luma values, and [16..240] for > >>8-bit chroma values. For 10-bit formats, the range limits are > >>multiplied by 4. > >> "YCbCr to RGB: BT.709 full-swing": > >>Only valid for YCbCr formats. CSC in accordance with BT.709, > >>but using the full range of each channel. > >> "YCbCr to RGB: Use CTM":* > >>Only valid for YCbCr formats. Use the matrix applied via the > >>plane's CTM property > >> "RGB to RGB: Use CTM":* > >>Only valid for RGB formats. Use the matrix applied via the > >>plane's CTM property > >> "Use CTM":* > >>Valid for any format. Use the matrix applied via the plane's > >>CTM property > >> ... any other values for BT.601, BT.2020, RGB to YCbCr etc. etc. as > >> they are required. > > > >Having some RGB2RGB and YCBCR2RGB things in the same property seems > >weird. I would just go with something very simple like: > > > >YCBCR_TO_RGB_CSC: > >* BT.601 > >* BT.709 > >* custom matrix > > > > I think we've agreed in #dri-devel that this CSC property > can't/shouldn't be mapped on-to the existing (hardware implementing > the) CTM property - even in the case of per-plane color management - > because CSC needs to be done before DEGAMMA. > > So, I'm in favour of going with what you suggested in the first place: > > A new YCBCR_TO_RGB_CSC property, enum type, with a list of fixed > conversions. I'd drop the custom matrix for now, as we'd need to add > another property to attach the custom matrix blob too. > > I still think we need a way to specify whether the source data range > is broadcast/full-range, so perhaps the enum list should be expanded > to all combinations of BT.601/BT.709 + broadcast/full-range. Sounds reasonable. Not that much full range YCbCr stuff out there perhaps. Well, apart from jpegs I suppose. But no harm in being able to deal with it. > > (I'm not sure what the canonical naming for broadcast/full-range is, > we call them narrow and wide) We tend to call them full vs. limited range. That's how our "Broadcast RGB" property is defined as well. > > >And trying to use the same thing for the crtc stuff is probably not > >going to end well. Like Daniel said we already have the > >'Broadcast RGB' property muddying the waters there, and that stuff > >also ties in with what colorspace we signal to the sink via > >infoframes/whatever the DP thing was called. So my gut feeling is > >that trying to use the same property everywhere will just end up > >messy. > > Yeah, agreed. If/when someone wants to add CSC on the output of a CRTC > (after GAMMA), we can add a new property. > > That makes me wonder about calling this one SOURCE_YCBCR_TO_RGB_CSC to > be explicit that it describes the source data. Then we can later add > SINK_RGB_TO_YCBCR_CSC, and it will be reasonably obvious that its > value describes the output data rather than the input data. Source and sink have a slight connotation in my mind wrt. the box that produces the display signal and the box that eats the signal. So trying to use the same terms to describe the internals of the pipeline inside the "source box" migth lead to some confusion. But we do probably need some decent names for these to make the layout of the pipeline clear. Input/output are the other names that popped to my mind but those aren't necessarily any better. But in the end I think I could live with whatever names we happen to pick, as long as we document the pipeline clearly. Long ago I did wonder if we should just start indexing these
Re: [PATCH v3 00/24] i.MX Media Driver
On Tue, Jan 31, 2017 at 03:25:10PM +0100, Philipp Zabel wrote: > On Tue, 2017-01-31 at 14:54 +0100, Philipp Zabel wrote: > > Hi Steve, > > > > I have just tested the imx-media-staging-md-wip branch on a Nitrogen6X > > with a tc358743 (BD_HDMI_MIPI HDMI to MIPI CSI-2 receiver board). Some > > observations: > > > > # Link pipeline > > media-ctl -l "'tc358743 1-000f':0->'imx6-mipi-csi2':0[1]" > > media-ctl -l "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]" > > media-ctl -l "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]" > > media-ctl -l "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]" > > > > # Provide an EDID to the HDMI source > > v4l2-ctl -d /dev/v4l-subdev2 --set-edid=file=edid-1080p.hex > > # At this point the HDMI source is enabled and sends a 1080p60 signal > > # Configure detected DV timings > > media-ctl --set-dv "'tc358743 1-000f':0" > > > > # Set pad formats > > media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]" > > media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]" > > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]" > > I noticed this seems to get ignored. The format is incorrectly set to > UYVY even though I request UYVY2X8 (see CSI2IPU chapter, Figure 19-10. > YUV422-8 data reception in the reference manual), but it seems to work > anyway. That's because the driver at imx-csi level bypasses the proper media pad formats on its sink pads, and instead goes poking about at the "sensor" to get the format. This is one of the reasons it wants to know which entity is the "sensor". The "sensor" stuff in there needs to be scrapped... -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] hva-v4l2: hva_dbg_summary() should be static
As reported by gcc: drivers/media/platform/sti/hva/hva-v4l2.c:227:6: warning: no previous prototype for 'hva_dbg_summary' [-Wmissing-prototypes] void hva_dbg_summary(struct hva_ctx *ctx) ^~~ This function is used only internally, so make it static. Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/sti/hva/hva-v4l2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/sti/hva/hva-v4l2.c b/drivers/media/platform/sti/hva/hva-v4l2.c index 052f2feebc86..1c4fc33cbcb5 100644 --- a/drivers/media/platform/sti/hva/hva-v4l2.c +++ b/drivers/media/platform/sti/hva/hva-v4l2.c @@ -224,7 +224,7 @@ static int hva_open_encoder(struct hva_ctx *ctx, u32 streamformat, return ret; } -void hva_dbg_summary(struct hva_ctx *ctx) +static void hva_dbg_summary(struct hva_ctx *ctx) { struct device *dev = ctx_to_dev(ctx); struct hva_streaminfo *stream = >streaminfo; -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/24] i.MX Media Driver
On Tue, 2017-01-31 at 14:54 +0100, Philipp Zabel wrote: > Hi Steve, > > I have just tested the imx-media-staging-md-wip branch on a Nitrogen6X > with a tc358743 (BD_HDMI_MIPI HDMI to MIPI CSI-2 receiver board). Some > observations: > > # Link pipeline > media-ctl -l "'tc358743 1-000f':0->'imx6-mipi-csi2':0[1]" > media-ctl -l "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]" > media-ctl -l "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]" > media-ctl -l "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]" > > # Provide an EDID to the HDMI source > v4l2-ctl -d /dev/v4l-subdev2 --set-edid=file=edid-1080p.hex > # At this point the HDMI source is enabled and sends a 1080p60 signal > # Configure detected DV timings > media-ctl --set-dv "'tc358743 1-000f':0" > > # Set pad formats > media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]" > media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]" > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]" I noticed this seems to get ignored. The format is incorrectly set to UYVY even though I request UYVY2X8 (see CSI2IPU chapter, Figure 19-10. YUV422-8 data reception in the reference manual), but it seems to work anyway. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] media: fix semicolon.cocci warnings
drivers/media/i2c/et8ek8/et8ek8_driver.c:1112:3-4: Unneeded semicolon Remove unneeded semicolon. Generated by: scripts/coccinelle/misc/semicolon.cocci CC: Pavel MachekSigned-off-by: Fengguang Wu --- et8ek8_driver.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c @@ -1109,7 +1109,7 @@ static int et8ek8_g_priv_mem(struct v4l2 if (!(status & 0x08)) break; usleep_range(1000, 2000); - }; + } if (i == 1000) return -EIO; -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[linuxtv-media:master 1136/1158] drivers/media/i2c/et8ek8/et8ek8_driver.c:1112:3-4: Unneeded semicolon
tree: git://linuxtv.org/media_tree.git master head: 0df03379c541ec027c94b025f2ef259f243eeb58 commit: c5254e72b8edc2ca0a98703e92e8c34959343d2c [1136/1158] [media] media: Driver for Toshiba et8ek8 5MP sensor coccinelle warnings: (new ones prefixed by >>) >> drivers/media/i2c/et8ek8/et8ek8_driver.c:1112:3-4: Unneeded semicolon Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/24] i.MX Media Driver
On Tue, Jan 31, 2017 at 02:35:00PM +0100, Philipp Zabel wrote: > On Tue, 2017-01-31 at 13:14 +, Russell King - ARM Linux wrote: > > This isn't limited to the serial side - the parallel bus side between > > the CSI2 interface and CSI2IPU wrapper, and the CSI2IPU wrapper and > > the CS0/1 interfaces is much the same with 10-bit bayer. > > > > Think of the CSI2 <-> CSI2IPU bit as the 4-lane case, lane 0 ending > > up on the least significant 8 bits of the 32-bit bus, lane 3 on the > > top 8-bits. > > > > Post CSI2IPU, it talks about a 16-bit bus in the diagrams, so that's > > kind of the 2-lane case above... > > You are right, on the parallel buses the format most definitely is not > MEDIA_BUS_FMT_SBGGR10_1X10. We don't have any representation of the > 32-bit bus between CSI2 host and CSI2IPU gasket because we model the two > as a single entity, but the four 16-bit parallel buses between the > CSI2IPU gasket and the IPU1/2 CSI0/1 probably should be set to a custom > format describing this accurately. Yep. I should also point out that there's a very odd transformation going on somewhere, and I don't yet know where. The sensor is definitely outputting GBRG format, but what seems to get written into memory is RGGB format. It's somewhere post CSI, because when I was using the (broken) CSI compander with 10 bit bayer, the red compander channel affected the red channel output from the camera, but changed the green component written to memory... it's very much like either the first line gets lost somewhere, or the odd/even lines are transposed. It could also be a gstreamer bug. As I say, it's not something I've looked into deeply enough yet... too many other issues to chase down! -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] st-hva: encoding summary at instance release
On 31/01/17 11:37, Jean-Christophe Trotin wrote: This patch adds a short summary about the encoding operation at each instance closing, for debug purpose (through dev_dbg()): - information about the frame (format, resolution) - information about the stream (format, profile, level, resolution) - number of encoded frames - potential (system, encoding...) errors Signed-off-by: Yannick FertreSigned-off-by: Jean-Christophe Trotin Acked-by: Hans Verkuil Mauro, Can you take this v4 series? I'm OK with this change. Regards, Hans --- drivers/media/platform/sti/hva/hva-h264.c | 6 drivers/media/platform/sti/hva/hva-hw.c | 5 drivers/media/platform/sti/hva/hva-mem.c | 5 +++- drivers/media/platform/sti/hva/hva-v4l2.c | 49 --- drivers/media/platform/sti/hva/hva.h | 8 + 5 files changed, 62 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/sti/hva/hva-h264.c b/drivers/media/platform/sti/hva/hva-h264.c index 8cc8467..e6f247a 100644 --- a/drivers/media/platform/sti/hva/hva-h264.c +++ b/drivers/media/platform/sti/hva/hva-h264.c @@ -607,6 +607,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, "%s width(%d) or height(%d) exceeds limits (%dx%d)\n", pctx->name, frame_width, frame_height, H264_MAX_SIZE_W, H264_MAX_SIZE_H); + pctx->frame_errors++; return -EINVAL; } @@ -717,6 +718,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, default: dev_err(dev, "%s invalid source pixel format\n", pctx->name); + pctx->frame_errors++; return -EINVAL; } @@ -741,6 +743,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, if (td->framerate_den == 0) { dev_err(dev, "%s invalid framerate\n", pctx->name); + pctx->frame_errors++; return -EINVAL; } @@ -831,6 +834,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, (payload > MAX_SPS_PPS_SIZE)) { dev_err(dev, "%s invalid sps/pps size %d\n", pctx->name, payload); + pctx->frame_errors++; return -EINVAL; } @@ -842,6 +846,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, (u8 *)stream->vaddr, )) { dev_err(dev, "%s fail to get SEI nal\n", pctx->name); + pctx->frame_errors++; return -EINVAL; } @@ -963,6 +968,7 @@ static int hva_h264_open(struct hva_ctx *pctx) err_ctx: devm_kfree(dev, ctx); err: + pctx->sys_errors++; return ret; } diff --git a/drivers/media/platform/sti/hva/hva-hw.c b/drivers/media/platform/sti/hva/hva-hw.c index 68d625b..5104068 100644 --- a/drivers/media/platform/sti/hva/hva-hw.c +++ b/drivers/media/platform/sti/hva/hva-hw.c @@ -470,6 +470,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd, if (pm_runtime_get_sync(dev) < 0) { dev_err(dev, "%s failed to get pm_runtime\n", ctx->name); + ctx->sys_errors++; ret = -EFAULT; goto out; } @@ -481,6 +482,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd, break; default: dev_dbg(dev, "%s unknown command 0x%x\n", ctx->name, cmd); + ctx->encode_errors++; ret = -EFAULT; goto out; } @@ -511,6 +513,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd, msecs_to_jiffies(2000))) { dev_err(dev, "%s %s: time out on completion\n", ctx->name, __func__); + ctx->encode_errors++; ret = -EFAULT; goto out; } @@ -518,6 +521,8 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd, /* get encoding status */ ret = ctx->hw_err ? -EFAULT : 0; + ctx->encode_errors += ctx->hw_err ? 1 : 0; + out: disable_irq(hva->irq_its); disable_irq(hva->irq_err); diff --git a/drivers/media/platform/sti/hva/hva-mem.c b/drivers/media/platform/sti/hva/hva-mem.c index 649f660..821c78e 100644 --- a/drivers/media/platform/sti/hva/hva-mem.c +++ b/drivers/media/platform/sti/hva/hva-mem.c @@ -17,14 +17,17 @@ int hva_mem_alloc(struct hva_ctx *ctx, u32 size, const char *name, void *base; b = devm_kzalloc(dev, sizeof(*b), GFP_KERNEL); - if (!b) + if (!b) { + ctx->sys_errors++; return -ENOMEM; + } base = dma_alloc_attrs(dev,
Re: [PATCH v3 00/24] i.MX Media Driver
Hi Steve, I have just tested the imx-media-staging-md-wip branch on a Nitrogen6X with a tc358743 (BD_HDMI_MIPI HDMI to MIPI CSI-2 receiver board). Some observations: # Link pipeline media-ctl -l "'tc358743 1-000f':0->'imx6-mipi-csi2':0[1]" media-ctl -l "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]" media-ctl -l "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]" media-ctl -l "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]" # Provide an EDID to the HDMI source v4l2-ctl -d /dev/v4l-subdev2 --set-edid=file=edid-1080p.hex # At this point the HDMI source is enabled and sends a 1080p60 signal # Configure detected DV timings media-ctl --set-dv "'tc358743 1-000f':0" # Set pad formats media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY/1920x1080]" media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/1920x1080]" media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080]" media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080]" v4l2-ctl -d /dev/video4 -V # This still is configured to 640x480, which is inconsistent with # the 'ipu1_csi0':2 pad format. The pad set_fmt above should # have set this, too. v4l2-ctl --list-formats -d /dev/video4 # This lists all the RGB formats, which it shouldn't. There is # no CSC in this pipeline, so we should be limited to YUV formats # only. # Set capture format v4l2-ctl -d /dev/video4 -v width=1920,height=1080,pixelformat=UYVY v4l2-ctl -d /dev/video4 -V # Now the capture format is correctly configured to 1920x1080. v4l2-ctl -d 4 --list-frameintervals=width=1920,height=1080,pixelformat=UYVY # This lists nothing. We should at least provide the 'ipu1_csi0':2 pad # frame interval. In the future this should list fractions achievable # via frame skipping. v4l2-compliance -d /dev/video4 # This fails two tests: # fail: v4l2-test-input-output.cpp(383): std == 0 # fail: v4l2-test-input-output.cpp(449): invalid attributes for input 0 # test VIDIOC_G/S/ENUMINPUT: FAIL # and # fail: v4l2-test-controls.cpp(782): subscribe event for control 'User Controls' failed # test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL # (Slowly) stream JPEG images to a display host: gst-launch-1.0 -v v4l2src device=/dev/video4 ! jpegenc ! rtpjpegpay ! udpsink I've done this a few times, and sometimes I only get a "ipu1_csi0: EOF timeout" message when starting streaming. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] st-hva: encoding summary at instance release
Em Tue, 31 Jan 2017 08:50:38 + Jean Christophe TROTINescreveu: > On 01/30/2017 06:28 PM, Mauro Carvalho Chehab wrote: > > Em Mon, 28 Nov 2016 11:30:52 +0100 > > Jean-Christophe Trotin escreveu: > > > >> This patch prints unconditionnaly a short summary > > > > Why? Is this driver so broken that everyone would need an > > unconditional "short summary" about what happened there? > > > > If not, then please use dev_dbg() or debugfs instead. If yes, then > > we should move this driver to staging. > > > Hi Mauro, > > The unconditional character of this "short summary" was a "facility" that our > customers used to get (it doesn't mean that this driver is broken). > That's said, I understand your comment, and I will propose today a new > version > of this patch with dev_dbg() instead of dev_info(). Thanks, the new version looks OK on my eyes. As I got it from Hans tree, I'll wait for either his ack/SOB or for him to merge on his tree. Regards, Mauro > > Regards, > Jean-Christophe. > > >> about the encoding > >> operation at each instance closing, for debug purpose: > >> - information about the frame (format, resolution) > >> - information about the stream (format, profile, level, resolution) > >> - number of encoded frames > >> - potential (system, encoding...) errors > >> > >> Signed-off-by: Yannick Fertre > >> Signed-off-by: Jean-Christophe Trotin > >> --- > >> drivers/media/platform/sti/hva/hva-h264.c | 6 > >> drivers/media/platform/sti/hva/hva-hw.c | 5 > >> drivers/media/platform/sti/hva/hva-mem.c | 5 +++- > >> drivers/media/platform/sti/hva/hva-v4l2.c | 49 > >> --- > >> drivers/media/platform/sti/hva/hva.h | 8 + > >> 5 files changed, 62 insertions(+), 11 deletions(-) > >> > >> diff --git a/drivers/media/platform/sti/hva/hva-h264.c > >> b/drivers/media/platform/sti/hva/hva-h264.c > >> index 8cc8467..e6f247a 100644 > >> --- a/drivers/media/platform/sti/hva/hva-h264.c > >> +++ b/drivers/media/platform/sti/hva/hva-h264.c > >> @@ -607,6 +607,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, > >>"%s width(%d) or height(%d) exceeds limits (%dx%d)\n", > >>pctx->name, frame_width, frame_height, > >>H264_MAX_SIZE_W, H264_MAX_SIZE_H); > >> + pctx->frame_errors++; > >>return -EINVAL; > >>} > >> > >> @@ -717,6 +718,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, > >>default: > >>dev_err(dev, "%s invalid source pixel format\n", > >>pctx->name); > >> + pctx->frame_errors++; > >>return -EINVAL; > >>} > >> > >> @@ -741,6 +743,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, > >> > >>if (td->framerate_den == 0) { > >>dev_err(dev, "%s invalid framerate\n", pctx->name); > >> + pctx->frame_errors++; > >>return -EINVAL; > >>} > >> > >> @@ -831,6 +834,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, > >>(payload > MAX_SPS_PPS_SIZE)) { > >>dev_err(dev, "%s invalid sps/pps size %d\n", pctx->name, > >>payload); > >> + pctx->frame_errors++; > >>return -EINVAL; > >>} > >> > >> @@ -842,6 +846,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, > >> (u8 *)stream->vaddr, > >> )) { > >>dev_err(dev, "%s fail to get SEI nal\n", pctx->name); > >> + pctx->frame_errors++; > >>return -EINVAL; > >>} > >> > >> @@ -963,6 +968,7 @@ static int hva_h264_open(struct hva_ctx *pctx) > >> err_ctx: > >>devm_kfree(dev, ctx); > >> err: > >> + pctx->sys_errors++; > >>return ret; > >> } > >> > >> diff --git a/drivers/media/platform/sti/hva/hva-hw.c > >> b/drivers/media/platform/sti/hva/hva-hw.c > >> index 68d625b..5104068 100644 > >> --- a/drivers/media/platform/sti/hva/hva-hw.c > >> +++ b/drivers/media/platform/sti/hva/hva-hw.c > >> @@ -470,6 +470,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum > >> hva_hw_cmd_type cmd, > >> > >>if (pm_runtime_get_sync(dev) < 0) { > >>dev_err(dev, "%s failed to get pm_runtime\n", ctx->name); > >> + ctx->sys_errors++; > >>ret = -EFAULT; > >>goto out; > >>} > >> @@ -481,6 +482,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum > >> hva_hw_cmd_type cmd, > >>break; > >>default: > >>dev_dbg(dev, "%s unknown command 0x%x\n", ctx->name, cmd); > >> + ctx->encode_errors++; > >>ret = -EFAULT; > >>goto out; > >>} > >> @@ -511,6 +513,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum > >> hva_hw_cmd_type cmd, > >>
Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver
On Fri, Jan 20, 2017 at 03:38:28PM +0100, Hans Verkuil wrote: > Should be set to something like 'platform:imx-media-camif'. v4l2-compliance > should complain about this. ... and more. Driver Info: Driver name : imx-media-camif Card type : imx-media-camif Bus info : Driver version: 4.10.0 Capabilities : 0x8421 Video Capture Streaming Extended Pix Format Device Capabilities Device Caps : 0x0421 Video Capture Streaming Extended Pix Format Compliance test for device /dev/video3 (not using libv4l2): Required ioctls: fail: v4l2-compliance.cpp(244): string empty fail: v4l2-compliance.cpp(297): check_ustring(vcap.bus_info, sizeof(vcap.bus_info)) test VIDIOC_QUERYCAP: FAIL Allow for multiple opens: test second video open: OK fail: v4l2-compliance.cpp(244): string empty fail: v4l2-compliance.cpp(297): check_ustring(vcap.bus_info, sizeof(vcap.bus_info)) test VIDIOC_QUERYCAP: FAIL test VIDIOC_G/S_PRIORITY: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) fail: v4l2-test-input-output.cpp(382): std == 0 fail: v4l2-test-input-output.cpp(437): invalid attributes for input 0 test VIDIOC_G/S/ENUMINPUT: FAIL test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK fail: v4l2-test-controls.cpp(779): subscribe event for control 'Camera Controls' failed test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 13 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) fail: v4l2-test-formats.cpp(414): unknown pixelformat 42474752 for buftype 1 test VIDIOC_G_FMT: FAIL test VIDIOC_TRY_FMT: OK (Not Supported) test VIDIOC_S_FMT: OK (Not Supported) test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK fail: v4l2-test-buffers.cpp(500): q.has_expbuf(node) test VIDIOC_EXPBUF: FAIL Total: 39, Succeeded: 33, Failed: 6, Warnings: 0 Not all of these may be a result of Steve's code - this is running against my gradually modified version to support bayer formats (which seems to be the cause of the v4l2-test-formats.cpp failure... for some reason the driver isn't enumerating all the formats.) And that reason is the way that the formats are enumerated: static int camif_enum_fmt_vid_cap(struct file *file, void *fh, struct v4l2_fmtdesc *f) { const struct imx_media_pixfmt *cc; u32 code; int ret; ret = imx_media_enum_format(, f->index, true, true); if (ret) return ret; cc = imx_media_find_format(0, code, true, true); if (!cc) return -EINVAL; When imx_media_enum_format() hits this entry in the table: }, { .fourcc = V4L2_PIX_FMT_BGR24, .cs = IPUV3_COLORSPACE_RGB, .bpp= 24, }, { becaues there's no .codes defined: int imx_media_enum_format(u32 *code, u32 index, bool allow_rgb, bool allow_planar) {
Re: [PATCH v3 00/24] i.MX Media Driver
On Tue, 2017-01-31 at 13:14 +, Russell King - ARM Linux wrote: > On Tue, Jan 31, 2017 at 11:09:24AM +0100, Philipp Zabel wrote: > > On Mon, 2017-01-30 at 13:06 +, Russell King - ARM Linux wrote: > > > To help illustrate my point, consider the difference between > > > MEDIA_BUS_FMT_RGB565_1X16 and MEDIA_BUS_FMT_RGB565_2X8_BE or > > > MEDIA_BUS_FMT_RGB565_2X8_LE. RGB565_1X16 means 1 pixel over an effective > > > 16-bit wide bus (if it's not 16-bit, then it has to be broken up into > > > separate "samples".) RGB565_2X8 means 1 pixel as two 8-bit samples. > > > > > > So, the 10-bit bayer is 1 pixel as 1.25 bytes. Or is it, over a serial > > > bus. Using the RGB565 case, 10-bit bayer over a 4 lane CSI bus becomes > > > interesting: > > > > > > first byte 2nd 3rd > > > lane 1P0 9:2 S0 P7 9:2 > > > lane 2P1 9:2 P4 9:2 S1 > > > lane 3P2 9:2 P5 9:2 P8 9:2 > > > lane 4P3 9:2 P6 9:2 P9 9:2 > > > > > > S0 = P0/P1/P2/P3 least significant two bits > > > S1 = P4/P5/P6/P7 least significant two bits > > > > > > or 2 lane CSI: > > > first byte 2nd 3rd 4th 5th > > > lane 1P0 9:2 P2 S0 P5 P7 > > > lane 2P1 9:2 P3 P4 P6 S1 > > > > > > or 1 lane CSI: > > > lane 1P0 P1 P2 P3 S0 P4 P5 P6 P7 S1 P8 P9 ... > > > > These do look like three different media bus formats to me. > > This isn't limited to the serial side - the parallel bus side between > the CSI2 interface and CSI2IPU wrapper, and the CSI2IPU wrapper and > the CS0/1 interfaces is much the same with 10-bit bayer. > > Think of the CSI2 <-> CSI2IPU bit as the 4-lane case, lane 0 ending > up on the least significant 8 bits of the 32-bit bus, lane 3 on the > top 8-bits. > > Post CSI2IPU, it talks about a 16-bit bus in the diagrams, so that's > kind of the 2-lane case above... You are right, on the parallel buses the format most definitely is not MEDIA_BUS_FMT_SBGGR10_1X10. We don't have any representation of the 32-bit bus between CSI2 host and CSI2IPU gasket because we model the two as a single entity, but the four 16-bit parallel buses between the CSI2IPU gasket and the IPU1/2 CSI0/1 probably should be set to a custom format describing this accurately. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/24] i.MX Media Driver
On Tue, Jan 31, 2017 at 11:09:24AM +0100, Philipp Zabel wrote: > On Mon, 2017-01-30 at 13:06 +, Russell King - ARM Linux wrote: > > To help illustrate my point, consider the difference between > > MEDIA_BUS_FMT_RGB565_1X16 and MEDIA_BUS_FMT_RGB565_2X8_BE or > > MEDIA_BUS_FMT_RGB565_2X8_LE. RGB565_1X16 means 1 pixel over an effective > > 16-bit wide bus (if it's not 16-bit, then it has to be broken up into > > separate "samples".) RGB565_2X8 means 1 pixel as two 8-bit samples. > > > > So, the 10-bit bayer is 1 pixel as 1.25 bytes. Or is it, over a serial > > bus. Using the RGB565 case, 10-bit bayer over a 4 lane CSI bus becomes > > interesting: > > > > first byte 2nd 3rd > > lane 1 P0 9:2 S0 P7 9:2 > > lane 2 P1 9:2 P4 9:2 S1 > > lane 3 P2 9:2 P5 9:2 P8 9:2 > > lane 4 P3 9:2 P6 9:2 P9 9:2 > > > > S0 = P0/P1/P2/P3 least significant two bits > > S1 = P4/P5/P6/P7 least significant two bits > > > > or 2 lane CSI: > > first byte 2nd 3rd 4th 5th > > lane 1 P0 9:2 P2 S0 P5 P7 > > lane 2 P1 9:2 P3 P4 P6 S1 > > > > or 1 lane CSI: > > lane 1 P0 P1 P2 P3 S0 P4 P5 P6 P7 S1 P8 P9 ... > > These do look like three different media bus formats to me. This isn't limited to the serial side - the parallel bus side between the CSI2 interface and CSI2IPU wrapper, and the CSI2IPU wrapper and the CS0/1 interfaces is much the same with 10-bit bayer. Think of the CSI2 <-> CSI2IPU bit as the 4-lane case, lane 0 ending up on the least significant 8 bits of the 32-bit bus, lane 3 on the top 8-bits. Post CSI2IPU, it talks about a 16-bit bus in the diagrams, so that's kind of the 2-lane case above... -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL FOR v4.11] Add et8ek8 driver
Em Tue, 31 Jan 2017 14:45:34 +0200 Sakari Ailusescreveu: > Hi Mauro, > > On Tue, Jan 31, 2017 at 10:42:48AM -0200, Mauro Carvalho Chehab wrote: > > That added a new warning: > > > > drivers/media/i2c/et8ek8/et8ek8_driver.c: In function 'et8ek8_registered': > > drivers/media/i2c/et8ek8/et8ek8_driver.c:1262:29: warning: variable > > 'format' set but not used [-Wunused-but-set-variable] > > struct v4l2_mbus_framefmt *format; > > ^~ > > compilation succeeded > > > > > > The driver is calling this function and storing it on a var > > that is not used: > > > > format = __et8ek8_get_pad_format(sensor, NULL, 0, > > V4L2_SUBDEV_FORMAT_ACTIVE); > > return 0; > > > > Please send a fixup patch. > > I compiled it, too, but I guess I had a GCC version that didn't complain > about this particular matter. I'll send you a fix. I run make with "W=1", to enable a few extra warnings that are usually troubles, like the above. W=2 would point some other things, but IMHO, it is not worth trying to fix the extra warnings, as it will enable a lot of signed/unsigned errors with are usually OK. Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[linuxtv-media:master 1123/1134] drivers/media/platform/coda/imx-vdoa.c:333:1: note: in expansion of macro 'module_platform_driver'
tree: git://linuxtv.org/media_tree.git master head: a2fafda66dccb84592d4b9e42e429471c356c4fc commit: 126f52b02e6ec6a25f0b32058a91648304922d4a [1123/1134] [media] coda/imx-vdoa: constify structs config: arm-allmodconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 126f52b02e6ec6a25f0b32058a91648304922d4a # save the attached .config to linux build tree make.cross ARCH=arm All warnings (new ones prefixed by >>): In file included from drivers/media/platform/coda/imx-vdoa.c:22:0: drivers/media/platform/coda/imx-vdoa.c: In function 'vdoa_driver_init': >> include/linux/device.h:1461:20: warning: passing argument 1 of >> '__platform_driver_register' discards 'const' qualifier from pointer target >> type [-Wdiscarded-qualifiers] return __register(&(__driver) , ##__VA_ARGS__); \ ^ include/linux/platform_device.h:198:29: note: in definition of macro 'platform_driver_register' __platform_driver_register(drv, THIS_MODULE) ^~~ include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver' module_driver(__platform_driver, platform_driver_register, \ ^ >> drivers/media/platform/coda/imx-vdoa.c:333:1: note: in expansion of macro >> 'module_platform_driver' module_platform_driver(vdoa_driver); ^~ include/linux/platform_device.h:199:12: note: expected 'struct platform_driver *' but argument is of type 'const struct platform_driver *' extern int __platform_driver_register(struct platform_driver *, ^~ In file included from drivers/media/platform/coda/imx-vdoa.c:18:0: drivers/media/platform/coda/imx-vdoa.c: In function 'vdoa_driver_exit': >> include/linux/device.h:1466:15: warning: passing argument 1 of >> 'platform_driver_unregister' discards 'const' qualifier from pointer target >> type [-Wdiscarded-qualifiers] __unregister(&(__driver) , ##__VA_ARGS__); \ ^ include/linux/platform_device.h:228:2: note: in expansion of macro 'module_driver' module_driver(__platform_driver, platform_driver_register, \ ^ >> drivers/media/platform/coda/imx-vdoa.c:333:1: note: in expansion of macro >> 'module_platform_driver' module_platform_driver(vdoa_driver); ^~ In file included from drivers/media/platform/coda/imx-vdoa.c:22:0: include/linux/platform_device.h:201:13: note: expected 'struct platform_driver *' but argument is of type 'const struct platform_driver *' extern void platform_driver_unregister(struct platform_driver *); ^~ vim +/module_platform_driver +333 drivers/media/platform/coda/imx-vdoa.c b0444f18 Philipp Zabel 2017-01-20 317 126f52b0 Mauro Carvalho Chehab 2017-01-31 318 static const struct of_device_id vdoa_dt_ids[] = { b0444f18 Philipp Zabel 2017-01-20 319 { .compatible = "fsl,imx6q-vdoa" }, b0444f18 Philipp Zabel 2017-01-20 320 {} b0444f18 Philipp Zabel 2017-01-20 321 }; b0444f18 Philipp Zabel 2017-01-20 322 MODULE_DEVICE_TABLE(of, vdoa_dt_ids); b0444f18 Philipp Zabel 2017-01-20 323 126f52b0 Mauro Carvalho Chehab 2017-01-31 324 static const struct platform_driver vdoa_driver = { b0444f18 Philipp Zabel 2017-01-20 325 .probe = vdoa_probe, b0444f18 Philipp Zabel 2017-01-20 326 .remove = vdoa_remove, b0444f18 Philipp Zabel 2017-01-20 327 .driver = { b0444f18 Philipp Zabel 2017-01-20 328 .name = VDOA_NAME, b0444f18 Philipp Zabel 2017-01-20 329 .of_match_table = vdoa_dt_ids, b0444f18 Philipp Zabel 2017-01-20 330 }, b0444f18 Philipp Zabel 2017-01-20 331 }; b0444f18 Philipp Zabel 2017-01-20 332 b0444f18 Philipp Zabel 2017-01-20 @333 module_platform_driver(vdoa_driver); b0444f18 Philipp Zabel 2017-01-20 334 b0444f18 Philipp Zabel 2017-01-20 335 MODULE_DESCRIPTION("Video Data Order Adapter"); b0444f18 Philipp Zabel 2017-01-20 336 MODULE_AUTHOR("Philipp Zabel"); b0444f18 Philipp Zabel 2017-01-20 337 MODULE_ALIAS("platform:imx-vdoa"); b0444f18 Philipp Zabel 2017-01-20 338 MODULE_LICENSE("GPL"); :: The code at line 333 was first introduced by commit :: b0444f18e0b18abce566e9e023d52e77e9cb68e1 [media] coda: add i.MX6 VDOA driver :: TO: Philipp Zabel :: CC: Mauro Carvalho Chehab --- 0-DAY kernel test infrastructureOpen Source
Re: [GIT PULL FOR v4.11] Add et8ek8 driver
Hi Mauro, On Tue, Jan 31, 2017 at 10:42:48AM -0200, Mauro Carvalho Chehab wrote: > That added a new warning: > > drivers/media/i2c/et8ek8/et8ek8_driver.c: In function 'et8ek8_registered': > drivers/media/i2c/et8ek8/et8ek8_driver.c:1262:29: warning: variable 'format' > set but not used [-Wunused-but-set-variable] > struct v4l2_mbus_framefmt *format; > ^~ > compilation succeeded > > > The driver is calling this function and storing it on a var > that is not used: > > format = __et8ek8_get_pad_format(sensor, NULL, 0, > V4L2_SUBDEV_FORMAT_ACTIVE); > return 0; > > Please send a fixup patch. I compiled it, too, but I guess I had a GCC version that didn't complain about this particular matter. I'll send you a fix. -- Cheers, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: DRM Atomic property for color-space conversion
Hi, On Mon, Jan 30, 2017 at 03:35:13PM +0200, Ville Syrjälä wrote: On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote: Hi, We're looking to enable the per-plane color management hardware in Mali-DP with atomic properties, which has sparked some conversation around how to handle YCbCr formats. As it stands today, it's assumed that a driver will implicitly "do the right thing" to display a YCbCr buffer. YCbCr data often uses different gamma curves and signal ranges (e.g. BT.609, BT.701, BT.2020, studio range, full-range), so its desirable to be able to explicitly control the YCbCr to RGB conversion process from userspace. We're proposing adding a "CSC" (color-space conversion) property to control this - primarily per-plane for framebuffer->pipeline CSC, but perhaps one per CRTC too for devices which have an RGB pipeline and want to output in YUV to the display: Name: "CSC" Type: ENUM | ATOMIC; Enum values (representative): "default": Same behaviour as now. "Some kind" of YCbCr->RGB conversion for YCbCr buffers, bypass for RGB buffers "disable": Explicitly disable all colorspace conversion (i.e. use an identity matrix). "YCbCr to RGB: BT.709": Only valid for YCbCr formats. CSC in accordance with BT.709 using [16..235] for (8-bit) luma values, and [16..240] for 8-bit chroma values. For 10-bit formats, the range limits are multiplied by 4. "YCbCr to RGB: BT.709 full-swing": Only valid for YCbCr formats. CSC in accordance with BT.709, but using the full range of each channel. "YCbCr to RGB: Use CTM":* Only valid for YCbCr formats. Use the matrix applied via the plane's CTM property "RGB to RGB: Use CTM":* Only valid for RGB formats. Use the matrix applied via the plane's CTM property "Use CTM":* Valid for any format. Use the matrix applied via the plane's CTM property ... any other values for BT.601, BT.2020, RGB to YCbCr etc. etc. as they are required. Having some RGB2RGB and YCBCR2RGB things in the same property seems weird. I would just go with something very simple like: YCBCR_TO_RGB_CSC: * BT.601 * BT.709 * custom matrix I think we've agreed in #dri-devel that this CSC property can't/shouldn't be mapped on-to the existing (hardware implementing the) CTM property - even in the case of per-plane color management - because CSC needs to be done before DEGAMMA. So, I'm in favour of going with what you suggested in the first place: A new YCBCR_TO_RGB_CSC property, enum type, with a list of fixed conversions. I'd drop the custom matrix for now, as we'd need to add another property to attach the custom matrix blob too. I still think we need a way to specify whether the source data range is broadcast/full-range, so perhaps the enum list should be expanded to all combinations of BT.601/BT.709 + broadcast/full-range. (I'm not sure what the canonical naming for broadcast/full-range is, we call them narrow and wide) And trying to use the same thing for the crtc stuff is probably not going to end well. Like Daniel said we already have the 'Broadcast RGB' property muddying the waters there, and that stuff also ties in with what colorspace we signal to the sink via infoframes/whatever the DP thing was called. So my gut feeling is that trying to use the same property everywhere will just end up messy. Yeah, agreed. If/when someone wants to add CSC on the output of a CRTC (after GAMMA), we can add a new property. That makes me wonder about calling this one SOURCE_YCBCR_TO_RGB_CSC to be explicit that it describes the source data. Then we can later add SINK_RGB_TO_YCBCR_CSC, and it will be reasonably obvious that its value describes the output data rather than the input data. I want to avoid confusion caused by ending up with two {CS}_TO_{CS}_CSC properties, where one is describing the data to the left of it, and the other describing the data to the right of it, with no real way of telling which way around it is. Cheers, Brian -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL FOR v4.11] Add et8ek8 driver
Em Wed, 25 Jan 2017 16:07:45 +0200 Sakari Ailusescreveu: > Hi Mauro, > > This pull request adds the sensor et8ek8 driver which is used on the Nokia > N900. Please pull. > > > The following changes since commit 40eca140c404505c09773d1c6685d818cb55ab1a: > > [media] mn88473: add DVB-T2 PLP support (2016-12-27 14:00:15 -0200) > > are available in the git repository at: > > ssh://linuxtv.org/git/sailus/media_tree.git et8ek8 > > for you to fetch changes up to 1d26b93d5341d36cdd45b4d801f85d6c35128385: > > mark myself as mainainer for camera on N900 (2017-01-25 15:49:45 +0200) > > > Pavel Machek (3): > media: et8ek8: add device tree binding documentation > media: Driver for Toshiba et8ek8 5MP sensor > mark myself as mainainer for camera on N900 > > .../bindings/media/i2c/toshiba,et8ek8.txt | 48 + > MAINTAINERS|8 + > drivers/media/i2c/Kconfig |1 + > drivers/media/i2c/Makefile |1 + > drivers/media/i2c/et8ek8/Kconfig |6 + > drivers/media/i2c/et8ek8/Makefile |2 + > drivers/media/i2c/et8ek8/et8ek8_driver.c | 1515 > > drivers/media/i2c/et8ek8/et8ek8_mode.c | 587 > drivers/media/i2c/et8ek8/et8ek8_reg.h | 96 ++ > 9 files changed, 2264 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt > create mode 100644 drivers/media/i2c/et8ek8/Kconfig > create mode 100644 drivers/media/i2c/et8ek8/Makefile > create mode 100644 drivers/media/i2c/et8ek8/et8ek8_driver.c > create mode 100644 drivers/media/i2c/et8ek8/et8ek8_mode.c > create mode 100644 drivers/media/i2c/et8ek8/et8ek8_reg.h > That added a new warning: drivers/media/i2c/et8ek8/et8ek8_driver.c: In function 'et8ek8_registered': drivers/media/i2c/et8ek8/et8ek8_driver.c:1262:29: warning: variable 'format' set but not used [-Wunused-but-set-variable] struct v4l2_mbus_framefmt *format; ^~ compilation succeeded The driver is calling this function and storing it on a var that is not used: format = __et8ek8_get_pad_format(sensor, NULL, 0, V4L2_SUBDEV_FORMAT_ACTIVE); return 0; Please send a fixup patch. Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] media: exynos4-is: add flags to dummy Exynos IS i2c adapter
Add simple 'functionality' member to dummy Exynos IS i2c adapter to make i2c core happy and get rid of NULL pointer dereference during Exynos4 IS probe since v4.10-rc1: Unable to handle kernel NULL pointer dereference at virtual address pgd = c0004000 [] *pgd= Internal error: Oops: 8005 [#1] PREEMPT SMP ARM Modules linked in: CPU: 1 PID: 100 Comm: kworker/1:2 Not tainted 4.10.0-rc6-next-20170131-00054-g39e6e4233de6 #1921 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) Workqueue: events deferred_probe_work_func task: ef2e task.stack: ef2ec000 PC is at 0x0 LR is at i2c_register_adapter+0x98/0x5cc ... [] (i2c_register_adapter) from [] (fimc_is_i2c_probe+0x84/0xe4) [] (fimc_is_i2c_probe) from [] (platform_drv_probe+0x50/0xb0) [] (platform_drv_probe) from [] (driver_probe_device+0x234/0x2dc) [] (driver_probe_device) from [] (bus_for_each_drv+0x44/0x8c) [] (bus_for_each_drv) from [] (__device_attach+0x9c/0x100) [] (__device_attach) from [] (bus_probe_device+0x84/0x8c) [] (bus_probe_device) from [] (device_add+0x380/0x528) [] (device_add) from [] (of_platform_device_create_pdata+0x70/0xa4) [] (of_platform_device_create_pdata) from [] (of_platform_bus_create+0xec/0x320) [] (of_platform_bus_create) from [] (of_platform_populate+0x5c/0xac) [] (of_platform_populate) from [] (fimc_is_probe+0x1c0/0x4cc) [] (fimc_is_probe) from [] (platform_drv_probe+0x50/0xb0) [] (platform_drv_probe) from [] (driver_probe_device+0x234/0x2dc) [] (driver_probe_device) from [] (bus_for_each_drv+0x44/0x8c) [] (bus_for_each_drv) from [] (__device_attach+0x9c/0x100) [] (__device_attach) from [] (bus_probe_device+0x84/0x8c) [] (bus_probe_device) from [] (deferred_probe_work_func+0x60/0x8c) [] (deferred_probe_work_func) from [] (process_one_work+0x120/0x31c) [] (process_one_work) from [] (process_scheduled_works+0x28/0x38) [] (process_scheduled_works) from [] (worker_thread+0x204/0x4ac) [] (worker_thread) from [] (kthread+0xfc/0x134) [] (kthread) from [] (ret_from_fork+0x14/0x3c) Signed-off-by: Marek Szyprowski <m.szyprow...@samsung.com> --- drivers/media/platform/exynos4-is/fimc-is-i2c.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/exynos4-is/fimc-is-i2c.c b/drivers/media/platform/exynos4-is/fimc-is-i2c.c index 6bba4ca022be..2f559663e51e 100644 --- a/drivers/media/platform/exynos4-is/fimc-is-i2c.c +++ b/drivers/media/platform/exynos4-is/fimc-is-i2c.c @@ -28,7 +28,14 @@ struct fimc_is_i2c { * is implemented in the FIMC-IS subsystem firmware and the host CPU * doesn't access the I2C bus controller. */ -static const struct i2c_algorithm fimc_is_i2c_algorithm; +static u32 is_i2c_func(struct i2c_adapter *adap) +{ + return I2C_FUNC_I2C; +} + +static const struct i2c_algorithm fimc_is_i2c_algorithm = { + .functionality = is_i2c_func, +}; static int fimc_is_i2c_probe(struct platform_device *pdev) { -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] v4l: of: check for unique lanes in data-lanes and clock-lanes
On Tue, Jan 31, 2017 at 01:08:31PM +0100, Niklas Söderlund wrote: > All lanes in data-lanes and clock-lanes properties should be unique. Add > a check for this in v4l2_of_parse_csi_bus() and print a warning if > duplicated lanes are found. > > Signed-off-by: Niklas SöderlundAcked-by: Sakari Ailus -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] v4l: of: check for unique lanes in data-lanes and clock-lanes
All lanes in data-lanes and clock-lanes properties should be unique. Add a check for this in v4l2_of_parse_csi_bus() and print a warning if duplicated lanes are found. Signed-off-by: Niklas Söderlund--- Changes since v1: - Do not return -EINVAL if a duplicate is found. Sakari pointed out there are drivers where the number of lanes matter but not the actual lane numbers. Updated commit message to highlight that only a warning is printed. - Switched to a bitmask to track lanes used instead of a nested loop, thanks Laurent for the suggestion. drivers/media/v4l2-core/v4l2-of.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c index 93b33681776ca427..4f59f442dd0a64c9 100644 --- a/drivers/media/v4l2-core/v4l2-of.c +++ b/drivers/media/v4l2-core/v4l2-of.c @@ -26,7 +26,7 @@ static int v4l2_of_parse_csi_bus(const struct device_node *node, struct v4l2_of_bus_mipi_csi2 *bus = >bus.mipi_csi2; struct property *prop; bool have_clk_lane = false; - unsigned int flags = 0; + unsigned int flags = 0, lanes_used = 0; u32 v; prop = of_find_property(node, "data-lanes", NULL); @@ -38,6 +38,12 @@ static int v4l2_of_parse_csi_bus(const struct device_node *node, lane = of_prop_next_u32(prop, lane, ); if (!lane) break; + + if (lanes_used & BIT(v)) + pr_warn("%s: duplicated lane %u in data-lanes\n", + node->full_name, v); + lanes_used |= BIT(v); + bus->data_lanes[i] = v; } bus->num_data_lanes = i; @@ -63,6 +69,11 @@ static int v4l2_of_parse_csi_bus(const struct device_node *node, } if (!of_property_read_u32(node, "clock-lanes", )) { + if (lanes_used & BIT(v)) + pr_warn("%s: duplicated lane %u in clock-lanes\n", + node->full_name, v); + lanes_used |= BIT(v); + bus->clock_lane = v; have_clk_lane = true; } -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 17/24] media: imx: Add CSI subdev driver
On Fri, Jan 06, 2017 at 06:11:35PM -0800, Steve Longerbeam wrote: > +static int csi_link_validate(struct v4l2_subdev *sd, > + struct media_link *link, > + struct v4l2_subdev_format *source_fmt, > + struct v4l2_subdev_format *sink_fmt) > +{ > + struct csi_priv *priv = v4l2_get_subdevdata(sd); > + bool is_csi2; > + int ret; > + > + ret = v4l2_subdev_link_validate_default(sd, link, source_fmt, sink_fmt); > + if (ret) > + return ret; > + > + priv->sensor = __imx_media_find_sensor(priv->md, >sd.entity); > + if (IS_ERR(priv->sensor)) { > + v4l2_err(>sd, "no sensor attached\n"); > + ret = PTR_ERR(priv->sensor); > + priv->sensor = NULL; > + return ret; > + } > + > + ret = v4l2_subdev_call(priv->sensor->sd, video, g_mbus_config, > +>sensor_mbus_cfg); > + if (ret) > + return ret; > + > + is_csi2 = (priv->sensor_mbus_cfg.type == V4L2_MBUS_CSI2); > + > + if (is_csi2) { > + int vc_num = 0; > + /* > + * NOTE! It seems the virtual channels from the mipi csi-2 > + * receiver are used only for routing by the video mux's, > + * or for hard-wired routing to the CSI's. Once the stream > + * enters the CSI's however, they are treated internally > + * in the IPU as virtual channel 0. > + */ > +#if 0 > + vc_num = imx_media_find_mipi_csi2_channel(priv->md, > + >sd.entity); > + if (vc_num < 0) > + return vc_num; > +#endif > + ipu_csi_set_mipi_datatype(priv->csi, vc_num, > + >format_mbus[priv->input_pad]); This seems (at least to me) to go against the spirit of the subdev negotiation. Here, you seem to bypass the negotiation performed between the CSI and the attached device, wanting to grab the format from the CSI2 sensor directly. That bypasses negotiation performed at the CSI2 subdev and video muxes. The same goes for the frame rate monitoring code - imho, the frame rate should be something that results from negotiation with the neighbouring element, not by poking at some entity that is several entities away. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/24] i.MX Media Driver
On Mon, Jan 30, 2017 at 05:22:01PM -0800, Steve Longerbeam wrote: > I'm also having trouble finding a datasheet for it, but from what > I've read, it has a MIPI CSI-2 interface. It should work fine as long > as it presents a single source pad, registers asynchronously, and > sets its entity function to MEDIA_ENT_F_CAM_SENSOR. Yes, it is MIPI CSI-2, and yes it has a single source pad, registers asynchronously, but that's about as far as it goes. The structure is a camera sensor followed by some processing. So just like the smiapp code, I've ended up with multiple subdevs describing each stage of the sensors pipeline. Just like smiapp, the camera sensor block (which is the very far end of the pipeline) is marked with MEDIA_ENT_F_CAM_SENSOR. However, in front of that is the binner, which just like smiapp gets a separate entity. It's this entity which is connected to the mipi-csi2 subdev. Unlike smiapp, which does not set an entity function, I set my binner entity as MEDIA_ENT_F_PROC_VIDEO_SCALER on the basis that that is what V4L2 documentation recommend: - .. row 27 .. _MEDIA-ENT-F-PROC-VIDEO-SCALER: - ``MEDIA_ENT_F_PROC_VIDEO_SCALER`` - Video scaler. An entity capable of video scaling must have at least one sink pad and one source pad, and scale the video frame(s) received on its sink pad(s) to a different resolution output on its source pad(s). The range of supported scaling ratios is entity-specific and can differ between the horizontal and vertical directions (in particular scaling can be supported in one direction only). Binning and skipping are considered as scaling. This causes attempts to configure the ipu1_csi0 interface to fail: media-ctl -v -d /dev/media1 --set-v4l2 '"ipu1_csi0":1[fmt:SGBRG8/512x512@1/30]' Opening media device /dev/media1 Enumerating entities Found 29 entities Enumerating pads and links Setting up format SGBRG8 512x512 on pad ipu1_csi0/1 Unable to set format: No such device (-19) Unable to setup formats: No such device (19) and in the kernel log: ipu1_csi0: no sensor attached And yes, I already know that my next problem is going to be that the bayer formats are not supported in your driver (just like Philipp's driver) but adding them should not be difficult... but only once this issue is resolved. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/11] [media] s5p-mfc: Rename IS_MFCV8 macro
On Wed, 2017-01-18 at 15:51 +0100, Andrzej Hajda wrote: > On 18.01.2017 11:01, Smitha T Murthy wrote: > > This patch renames macro IS_MFCV8 to IS_MFCV8_PLUS so that the MFCv8 > > code can be resued for MFCv10.10 support. Since the MFCv8 specific code > > holds good for MFC v10.10 also. > > > > Signed-off-by: Smitha T Murthy> > Acked-by: Andrzej Hajda Thanks for review and ack. Regards, Smitha > -- > Regards > Andrzej > > > --- > > drivers/media/platform/s5p-mfc/s5p_mfc_common.h |2 +- > > drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c |2 +- > > drivers/media/platform/s5p-mfc/s5p_mfc_dec.c|2 +- > > drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 18 +- > > 4 files changed, 12 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h > > b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h > > index ab23236..b45d18c 100644 > > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h > > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h > > @@ -722,7 +722,7 @@ struct mfc_control { > > #define IS_TWOPORT(dev)(dev->variant->port_num == 2 ? 1 : 0) > > #define IS_MFCV6_PLUS(dev) (dev->variant->version >= 0x60 ? 1 : 0) > > #define IS_MFCV7_PLUS(dev) (dev->variant->version >= 0x70 ? 1 : 0) > > -#define IS_MFCV8(dev) (dev->variant->version >= 0x80 ? 1 : 0) > > +#define IS_MFCV8_PLUS(dev) (dev->variant->version >= 0x80 ? 1 : 0) > > > > #define MFC_V5_BIT BIT(0) > > #define MFC_V6_BIT BIT(1) > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c > > b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c > > index cc88871..484af6b 100644 > > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c > > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c > > @@ -427,7 +427,7 @@ int s5p_mfc_wakeup(struct s5p_mfc_dev *dev) > > s5p_mfc_clear_cmds(dev); > > s5p_mfc_clean_dev_int_flags(dev); > > /* 3. Send MFC wakeup command and wait for completion*/ > > - if (IS_MFCV8(dev)) > > + if (IS_MFCV8_PLUS(dev)) > > ret = s5p_mfc_v8_wait_wakeup(dev); > > else > > ret = s5p_mfc_wait_wakeup(dev); > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c > > b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c > > index 367ef8e..0ec2928 100644 > > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c > > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c > > @@ -1177,7 +1177,7 @@ void s5p_mfc_dec_init(struct s5p_mfc_ctx *ctx) > > struct v4l2_format f; > > f.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_H264; > > ctx->src_fmt = find_format(, MFC_FMT_DEC); > > - if (IS_MFCV8(ctx->dev)) > > + if (IS_MFCV8_PLUS(ctx->dev)) > > f.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_NV12M; > > else if (IS_MFCV6_PLUS(ctx->dev)) > > f.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_NV12MT_16X16; > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c > > b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c > > index 57da798..0572521 100644 > > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c > > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c > > @@ -74,7 +74,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct > > s5p_mfc_ctx *ctx) > > ctx->luma_size, ctx->chroma_size, ctx->mv_size); > > mfc_debug(2, "Totals bufs: %d\n", ctx->total_dpb_count); > > } else if (ctx->type == MFCINST_ENCODER) { > > - if (IS_MFCV8(dev)) > > + if (IS_MFCV8_PLUS(dev)) > > ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 * > > ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V8(mb_width, mb_height), > > S5P_FIMV_TMV_BUFFER_ALIGN_V6); > > @@ -89,7 +89,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct > > s5p_mfc_ctx *ctx) > > ctx->chroma_dpb_size = ALIGN((mb_width * mb_height) * > > S5P_FIMV_CHROMA_MB_TO_PIXEL_V6, > > S5P_FIMV_CHROMA_DPB_BUFFER_ALIGN_V6); > > - if (IS_MFCV8(dev)) > > + if (IS_MFCV8_PLUS(dev)) > > ctx->me_buffer_size = ALIGN(S5P_FIMV_ME_BUFFER_SIZE_V8( > > ctx->img_width, ctx->img_height, > > mb_width, mb_height), > > @@ -110,7 +110,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct > > s5p_mfc_ctx *ctx) > > switch (ctx->codec_mode) { > > case S5P_MFC_CODEC_H264_DEC: > > case S5P_MFC_CODEC_H264_MVC_DEC: > > - if (IS_MFCV8(dev)) > > + if (IS_MFCV8_PLUS(dev)) > > ctx->scratch_buf_size = > > S5P_FIMV_SCRATCH_BUF_SIZE_H264_DEC_V8( > > mb_width, > > @@ -167,7 +167,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct > > s5p_mfc_ctx *ctx) > > ctx->bank1.size = ctx->scratch_buf_size; > > break;
Re: [PATCH] Staging: omap4iss: fix coding style issues
On Tuesday 31 Jan 2017 12:42:51 Sakari Ailus wrote: > On Mon, Jan 30, 2017 at 07:47:40PM +0200, Laurent Pinchart wrote: > > > @@ -678,8 +679,8 @@ iss_video_get_selection(struct file *file, void *fh, > > > struct v4l2_selection *sel) if (subdev == NULL) > > > > > > return -EINVAL; > > > > > > - /* Try the get selection operation first and fallback to get format if > > > > not > > > > > - * implemented. > > > + /* Try the get selection operation first and fallback to get format if > > > + * not implemented. > > > > > >*/ > > /* > * Multi line > * comment. > */ Then let's patch the whole driver in one go :-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Staging: omap4iss: fix coding style issues
On Mon, Jan 30, 2017 at 07:47:40PM +0200, Laurent Pinchart wrote: > > @@ -678,8 +679,8 @@ iss_video_get_selection(struct file *file, void *fh, > > struct v4l2_selection *sel) if (subdev == NULL) > > return -EINVAL; > > > > - /* Try the get selection operation first and fallback to get format if > not > > -* implemented. > > + /* Try the get selection operation first and fallback to get format if > > +* not implemented. > > */ /* * Multi line * comment. */ -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/2] add debug capabilities to v4l2 encoder for STMicroelectronics SOC
version 4: - As suggested by Mauro, the encoding summary (first patch) is no more unconditionallly added: dev_dbg() is used instead of dev_info(). version 3: - the encoding summary (first patch) is moved from hva-debug.c to hva-v4l2.c. As suggested by Hans, dev_info() is used instead of snprintf() in the hva_dbg_summary() function. - About the debugfs entries for HVA (second patch), a driver-specific Kconfig option (VIDEO_STI_HVA_DEBUGFS) is added as suggested by Hans. This option depends both on VIDEO_STI_HVA and on DEBUG_FS. - The third (new) patch enables STMicroelectronics HVA debugfs in multi_v7_defconfig (VIDEO_STI_HVA_DEBUGFS). version 2: - the encoding summary (first patch) doesn't include any longer information about the encoding performance. Thus, after each frame encoding, only one or two variables are increased (number of encoded frames, number of encoding errors), but no computation is executed (as it was in version 1). When the encoding instance is closed, the short summary that is printed (dev_info), also doesn't require any computation, and gives a useful brief status about the last operation: that are the reasons why there's no Kconfig option to explicitly enable this summary. - the second patch enables the computation of the performances (hva_dbg_perf_begin and hva_dbg_perf_end) only if DEBUG_FS is enabled. The functions that create or remove the debugfs entries (hva_debugfs_create, hva_debugfs_remove, hva_dbg_ctx_create, hva_dbg_ctx_remove) are not under CONFIG_DEBUG_FS switch: if DEBUG_FS is disabled, the debugfs functions (debugfs_create_dir and debugfs_create_file) are available, but no entry is created. The "show" operations (hva_dbg_device, hva_dbg_encoders, hva_dbg_last, hva_dbg_regs, hva_dbg_ctx) are also not under CONFIG_DEBUG_FS switch: if DEBUG_FS is disabled, they will never be called. So, with this version 2, no new Kconfig option is introduced, but the performance computations and the debugfs entries depend on whether DEBUG_FS is enabled or not. version 1: - Initial submission With the first patch, a short summary about the encoding operation is added at each instance closing, for debug purpose (through dev_dbg()): - information about the frame (format, resolution) - information about the stream (format, profile, level, resolution) - number of encoded frames - potential (system, encoding...) errors With the second patch, 4 static debugfs entries are created to dump: - the device-related information ("st-hva/device") - the list of registered encoders ("st-hva/encoders") - the current values of the hva registers ("st-hva/regs") - the information about the last closed instance ("st-hva/last") Moreover, a debugfs entry is dynamically created for each opened instance, ("st-hva/") to dump: - the information about the frame (format, resolution) - the information about the stream (format, profile, level, resolution) - the control parameters (bitrate mode, framerate, GOP size...) - the potential (system, encoding...) errors - the performance information about the encoding (HW processing duration, average bitrate, average framerate...) Each time a running instance is closed, its context (including the debug information) is saved to feed, on demand, the last closed instance debugfs entry. These debug capabilities are mainly implemented in the hva-debugfs.c file. Jean-Christophe Trotin (2): st-hva: encoding summary at instance release st-hva: add debug file system drivers/media/platform/Kconfig | 11 + drivers/media/platform/sti/hva/Makefile | 1 + drivers/media/platform/sti/hva/hva-debugfs.c | 422 +++ drivers/media/platform/sti/hva/hva-h264.c| 6 + drivers/media/platform/sti/hva/hva-hw.c | 48 +++ drivers/media/platform/sti/hva/hva-hw.h | 3 + drivers/media/platform/sti/hva/hva-mem.c | 5 +- drivers/media/platform/sti/hva/hva-v4l2.c| 78 - drivers/media/platform/sti/hva/hva.h | 96 +- 9 files changed, 656 insertions(+), 14 deletions(-) create mode 100644 drivers/media/platform/sti/hva/hva-debugfs.c -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/2] st-hva: encoding summary at instance release
This patch adds a short summary about the encoding operation at each instance closing, for debug purpose (through dev_dbg()): - information about the frame (format, resolution) - information about the stream (format, profile, level, resolution) - number of encoded frames - potential (system, encoding...) errors Signed-off-by: Yannick FertreSigned-off-by: Jean-Christophe Trotin --- drivers/media/platform/sti/hva/hva-h264.c | 6 drivers/media/platform/sti/hva/hva-hw.c | 5 drivers/media/platform/sti/hva/hva-mem.c | 5 +++- drivers/media/platform/sti/hva/hva-v4l2.c | 49 --- drivers/media/platform/sti/hva/hva.h | 8 + 5 files changed, 62 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/sti/hva/hva-h264.c b/drivers/media/platform/sti/hva/hva-h264.c index 8cc8467..e6f247a 100644 --- a/drivers/media/platform/sti/hva/hva-h264.c +++ b/drivers/media/platform/sti/hva/hva-h264.c @@ -607,6 +607,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, "%s width(%d) or height(%d) exceeds limits (%dx%d)\n", pctx->name, frame_width, frame_height, H264_MAX_SIZE_W, H264_MAX_SIZE_H); + pctx->frame_errors++; return -EINVAL; } @@ -717,6 +718,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, default: dev_err(dev, "%s invalid source pixel format\n", pctx->name); + pctx->frame_errors++; return -EINVAL; } @@ -741,6 +743,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, if (td->framerate_den == 0) { dev_err(dev, "%s invalid framerate\n", pctx->name); + pctx->frame_errors++; return -EINVAL; } @@ -831,6 +834,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, (payload > MAX_SPS_PPS_SIZE)) { dev_err(dev, "%s invalid sps/pps size %d\n", pctx->name, payload); + pctx->frame_errors++; return -EINVAL; } @@ -842,6 +846,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, (u8 *)stream->vaddr, )) { dev_err(dev, "%s fail to get SEI nal\n", pctx->name); + pctx->frame_errors++; return -EINVAL; } @@ -963,6 +968,7 @@ static int hva_h264_open(struct hva_ctx *pctx) err_ctx: devm_kfree(dev, ctx); err: + pctx->sys_errors++; return ret; } diff --git a/drivers/media/platform/sti/hva/hva-hw.c b/drivers/media/platform/sti/hva/hva-hw.c index 68d625b..5104068 100644 --- a/drivers/media/platform/sti/hva/hva-hw.c +++ b/drivers/media/platform/sti/hva/hva-hw.c @@ -470,6 +470,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd, if (pm_runtime_get_sync(dev) < 0) { dev_err(dev, "%s failed to get pm_runtime\n", ctx->name); + ctx->sys_errors++; ret = -EFAULT; goto out; } @@ -481,6 +482,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd, break; default: dev_dbg(dev, "%s unknown command 0x%x\n", ctx->name, cmd); + ctx->encode_errors++; ret = -EFAULT; goto out; } @@ -511,6 +513,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd, msecs_to_jiffies(2000))) { dev_err(dev, "%s %s: time out on completion\n", ctx->name, __func__); + ctx->encode_errors++; ret = -EFAULT; goto out; } @@ -518,6 +521,8 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd, /* get encoding status */ ret = ctx->hw_err ? -EFAULT : 0; + ctx->encode_errors += ctx->hw_err ? 1 : 0; + out: disable_irq(hva->irq_its); disable_irq(hva->irq_err); diff --git a/drivers/media/platform/sti/hva/hva-mem.c b/drivers/media/platform/sti/hva/hva-mem.c index 649f660..821c78e 100644 --- a/drivers/media/platform/sti/hva/hva-mem.c +++ b/drivers/media/platform/sti/hva/hva-mem.c @@ -17,14 +17,17 @@ int hva_mem_alloc(struct hva_ctx *ctx, u32 size, const char *name, void *base; b = devm_kzalloc(dev, sizeof(*b), GFP_KERNEL); - if (!b) + if (!b) { + ctx->sys_errors++; return -ENOMEM; + } base = dma_alloc_attrs(dev, size, , GFP_KERNEL | GFP_DMA, DMA_ATTR_WRITE_COMBINE); if (!base) { dev_err(dev, "%s %s : dma_alloc_attrs failed for %s
[PATCH v4 2/2] st-hva: add debug file system
This patch creates 4 static debugfs entries to dump: - the device-related information ("st-hva/device") - the list of registered encoders ("st-hva/encoders") - the current values of the hva registers ("st-hva/regs") - the information about the last closed instance ("st-hva/last") It also creates dynamically a debugfs entry for each opened instance, ("st-hva/") to dump: - the information about the frame (format, resolution) - the information about the stream (format, profile, level, resolution) - the control parameters (bitrate mode, framerate, GOP size...) - the potential (system, encoding...) errors - the performance information about the encoding (HW processing duration, average bitrate, average framerate...) Each time a running instance is closed, its context (including the debug information) is saved to feed, on demand, the last closed instance debugfs entry. Signed-off-by: Yannick FertreSigned-off-by: Jean-Christophe Trotin --- drivers/media/platform/Kconfig | 11 + drivers/media/platform/sti/hva/Makefile | 1 + drivers/media/platform/sti/hva/hva-debugfs.c | 422 +++ drivers/media/platform/sti/hva/hva-hw.c | 43 +++ drivers/media/platform/sti/hva/hva-hw.h | 3 + drivers/media/platform/sti/hva/hva-v4l2.c| 29 +- drivers/media/platform/sti/hva/hva.h | 88 +- 7 files changed, 594 insertions(+), 3 deletions(-) create mode 100644 drivers/media/platform/sti/hva/hva-debugfs.c diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index d944421..af72641 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -298,6 +298,17 @@ config VIDEO_STI_HVA To compile this driver as a module, choose M here: the module will be called st-hva. +config VIDEO_STI_HVA_DEBUGFS + bool "Export STMicroelectronics HVA internals in debugfs" + depends on VIDEO_STI_HVA + depends on DEBUG_FS + help + Select this to see information about the internal state and the last + operation of STMicroelectronics HVA multi-format video encoder in + debugfs. + + Choose N unless you know you need this. + config VIDEO_SH_VEU tristate "SuperH VEU mem2mem video processing driver" depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA diff --git a/drivers/media/platform/sti/hva/Makefile b/drivers/media/platform/sti/hva/Makefile index ffb69ce..e3ebe968 100644 --- a/drivers/media/platform/sti/hva/Makefile +++ b/drivers/media/platform/sti/hva/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_VIDEO_STI_HVA) := st-hva.o st-hva-y := hva-v4l2.o hva-hw.o hva-mem.o hva-h264.o +st-hva-$(CONFIG_VIDEO_STI_HVA_DEBUGFS) += hva-debugfs.o diff --git a/drivers/media/platform/sti/hva/hva-debugfs.c b/drivers/media/platform/sti/hva/hva-debugfs.c new file mode 100644 index 000..83a6258 --- /dev/null +++ b/drivers/media/platform/sti/hva/hva-debugfs.c @@ -0,0 +1,422 @@ +/* + * Copyright (C) STMicroelectronics SA 2015 + * Authors: Yannick Fertre + * Hugues Fruchet + * License terms: GNU General Public License (GPL), version 2 + */ + +#include + +#include "hva.h" +#include "hva-hw.h" + +static void format_ctx(struct seq_file *s, struct hva_ctx *ctx) +{ + struct hva_streaminfo *stream = >streaminfo; + struct hva_frameinfo *frame = >frameinfo; + struct hva_controls *ctrls = >ctrls; + struct hva_ctx_dbg *dbg = >dbg; + u32 bitrate_mode, aspect, entropy, vui_sar, sei_fp; + + seq_printf(s, "|-%s\n |\n", ctx->name); + + seq_printf(s, " |-[%sframe info]\n", + ctx->flags & HVA_FLAG_FRAMEINFO ? "" : "default "); + seq_printf(s, " | |- pixel format=%4.4s\n" + " | |- wxh=%dx%d\n" + " | |- wxh (w/ encoder alignment constraint)=%dx%d\n" + " |\n", + (char *)>pixelformat, + frame->width, frame->height, + frame->aligned_width, frame->aligned_height); + + seq_printf(s, " |-[%sstream info]\n", + ctx->flags & HVA_FLAG_STREAMINFO ? "" : "default "); + seq_printf(s, " | |- stream format=%4.4s\n" + " | |- wxh=%dx%d\n" + " | |- %s\n" + " | |- %s\n" + " |\n", + (char *)>streamformat, + stream->width, stream->height, + stream->profile, stream->level); + + bitrate_mode = V4L2_CID_MPEG_VIDEO_BITRATE_MODE; + aspect = V4L2_CID_MPEG_VIDEO_ASPECT; + seq_puts(s, " |-[parameters]\n"); + seq_printf(s, " | |- %s\n" + " | |- bitrate=%d bps\n" + " | |- GOP size=%d\n" + " | |- video aspect=%s\n" + " | |-
Re: [PATCHv2 10/16] ov2640: enable clock and fix power/reset
Hi Hans, Thank you for the patchset! On Mon, Jan 30, 2017 at 03:06:22PM +0100, Hans Verkuil wrote: > From: Hans Verkuil> > Convert v4l2_clk to normal clk, enable the clock and fix the power/reset > handling. > > Signed-off-by: Hans Verkuil > --- > drivers/media/i2c/ov2640.c | 80 > +- > 1 file changed, 29 insertions(+), 51 deletions(-) > > diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c > index 83f88ef..565742b 100644 > --- a/drivers/media/i2c/ov2640.c > +++ b/drivers/media/i2c/ov2640.c > @@ -16,15 +16,14 @@ > #include > #include > #include > +#include > #include > #include > #include > #include > -#include > #include > #include > > -#include > #include > #include > #include > @@ -284,7 +283,7 @@ struct ov2640_priv { > struct v4l2_subdev subdev; > struct v4l2_ctrl_handlerhdl; > u32 cfmt_code; > - struct v4l2_clk *clk; > + struct clk *clk; Nice! > const struct ov2640_win_size*win; > > struct gpio_desc *resetb_gpio; > @@ -656,8 +655,9 @@ static int ov2640_mask_set(struct i2c_client *client, > return i2c_smbus_write_byte_data(client, reg, val); > } > > -static int ov2640_reset(struct i2c_client *client) > +static int ov2640_reset(struct v4l2_subdev *sd, u32 val) > { > + struct i2c_client *client = v4l2_get_subdevdata(sd); > int ret; > const struct regval_list reset_seq[] = { > {BANK_SEL, BANK_SEL_SENS}, > @@ -735,21 +735,6 @@ static int ov2640_s_register(struct v4l2_subdev *sd, > } > #endif > > -static int ov2640_s_power(struct v4l2_subdev *sd, int on) > -{ > - struct i2c_client *client = v4l2_get_subdevdata(sd); > - struct ov2640_priv *priv = to_ov2640(client); > - > - gpiod_direction_output(priv->pwdn_gpio, !on); > - if (on && priv->resetb_gpio) { > - /* Active the resetb pin to perform a reset pulse */ > - gpiod_direction_output(priv->resetb_gpio, 1); > - usleep_range(3000, 5000); > - gpiod_direction_output(priv->resetb_gpio, 0); Do you still perform the reset sequence somewhere? This could be crucial for reliability. > - } > - return 0; > -} > - > /* Select the nearest higher resolution for capture */ > static const struct ov2640_win_size *ov2640_select_win(u32 *width, u32 > *height) > { > @@ -769,9 +754,10 @@ static const struct ov2640_win_size > *ov2640_select_win(u32 *width, u32 *height) > return _supported_win_sizes[default_size]; > } > > -static int ov2640_set_params(struct i2c_client *client, u32 *width, u32 > *height, > +static int ov2640_set_params(struct v4l2_subdev *sd, u32 *width, u32 *height, >u32 code) > { > + struct i2c_client *client = v4l2_get_subdevdata(sd); > struct ov2640_priv *priv = to_ov2640(client); > const struct regval_list *selected_cfmt_regs; > int ret; > @@ -802,7 +788,7 @@ static int ov2640_set_params(struct i2c_client *client, > u32 *width, u32 *height, > } > > /* reset hardware */ > - ov2640_reset(client); > + ov2640_reset(sd, 0); > > /* initialize the sensor with default data */ > dev_dbg(>dev, "%s: Init default", __func__); > @@ -840,7 +826,7 @@ static int ov2640_set_params(struct i2c_client *client, > u32 *width, u32 *height, > > err: > dev_err(>dev, "%s: Error %d", __func__, ret); > - ov2640_reset(client); > + ov2640_reset(sd, 0); > priv->win = NULL; > > return ret; > @@ -877,7 +863,6 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd, > struct v4l2_subdev_format *format) > { > struct v4l2_mbus_framefmt *mf = >format; > - struct i2c_client *client = v4l2_get_subdevdata(sd); > > if (format->pad) > return -EINVAL; > @@ -902,7 +887,7 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd, > } > > if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) > - return ov2640_set_params(client, >width, > + return ov2640_set_params(sd, >width, >>height, mf->code); > cfg->try_fmt = *mf; > return 0; > @@ -947,10 +932,6 @@ static int ov2640_video_probe(struct i2c_client *client) > const char *devname; > int ret; > > - ret = ov2640_s_power(>subdev, 1); > - if (ret < 0) > - return ret; > - > /* >* check and show product ID and manufacturer ID >*/ > @@ -978,7 +959,6 @@ static int ov2640_video_probe(struct i2c_client *client) > ret = v4l2_ctrl_handler_setup(>hdl); > > done: > - ov2640_s_power(>subdev, 0); > return ret; > } > > @@ -991,7 +971,7 @@ static struct v4l2_subdev_core_ops ov2640_subdev_core_ops > = { > .g_register = ov2640_g_register, > .s_register
Re: [PATCH v3 17/24] media: imx: Add CSI subdev driver
On Fri, Jan 06, 2017 at 06:11:35PM -0800, Steve Longerbeam wrote: > This is a media entity subdevice for the i.MX Camera > Serial Interface module. > > Signed-off-by: Steve Longerbeam> --- The lack of s_frame_interval/g_frame_interval in this driver means: media-ctl -v -d /dev/media1 --set-v4l2 '"imx6-mipi-csi2":1[fmt:SGBRG8/512x512@1/30]' Opening media device /dev/media1 Enumerating entities Found 29 entities Enumerating pads and links Setting up format SGBRG8 512x512 on pad imx6-mipi-csi2/1 Format set: SGBRG8 512x512 Setting up frame interval 1/30 on entity imx6-mipi-csi2 Unable to set frame interval: Inappropriate ioctl for device (-25)Unable to setup formats: Inappropriate ioctl for device (25) which causes the setup of the next element in the chain to also fail (because the above media-ctl command doesn't set the sink on the ipu1 csi0 mux.) It seems to me that without the frame interval supported throughout the chain, there's no way for an application to properly negotiate the capture parameters via the "try" mechanism, since it has no idea whether the frame rate it wants is supported throughout the subdev chain. Eg, the sensor may be able to do 100fps but there could be something in the pipeline that restricts it due to bandwidth. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/24] i.MX Media Driver
On Mon, Jan 30, 2017 at 05:22:01PM -0800, Steve Longerbeam wrote: > Edit: I see a subdev that is missing: the video mux. Did you enable > CONFIG_VIDEO_MULTIPLEXER? Yes, and that's where the problem is - the video-multiplexer is missing the module aliases to allow it to be automatically loaded. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] [media] v4l2-async: Delete an error message for a failed memory allocation in v4l2_async_notifier_unregister()
Em Mon, 26 Dec 2016 21:45:50 +0100 SF Markus Elfringescreveu: > From: Markus Elfring > Date: Mon, 26 Dec 2016 19:19:49 +0100 > > The script "checkpatch.pl" pointed information out like the following. > > WARNING: Possible unnecessary 'out of memory' message > > Thus fix the affected source code place. > > Signed-off-by: Markus Elfring > --- > drivers/media/v4l2-core/v4l2-async.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-async.c > b/drivers/media/v4l2-core/v4l2-async.c > index 277183f2d514..812d0b2a2f73 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -203,11 +203,6 @@ void v4l2_async_notifier_unregister(struct > v4l2_async_notifier *notifier) > return; > > dev = kmalloc_array(n_subdev, sizeof(*dev), GFP_KERNEL); > - if (!dev) { > - dev_err(notifier->v4l2_dev->dev, > - "Failed to allocate device cache!\n"); > - } > - In this specific case, we should keep it, as the message means that the unregister logic won't work properly, as this loop won't run: /* * Call device_attach() to reprobe devices * * NOTE: If dev allocation fails, i is 0, and the whole loop won't be * executed. */ while (i--) { struct device *d = dev[i]; if (d && device_attach(d) < 0) { const char *name = "(none)"; int lock = device_trylock(d); if (lock && d->driver) name = d->driver->name; dev_err(d, "Failed to re-probe to %s\n", name); if (lock) device_unlock(d); } put_device(d); } So, IMHO, the proper patch would be to change the message to be more comprehensive, describing the consequences of not being able to allocate the dev cache. > mutex_lock(_lock); > > list_del(>list); Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] coda/imx-vdoa: constify structs
As warned by checkpatch: WARNING: struct of_device_id should normally be const #318: FILE: drivers/media/platform/coda/imx-vdoa.c:318: +static struct of_device_id vdoa_dt_ids[] = { So, constify structs. Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/coda/imx-vdoa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/coda/imx-vdoa.c b/drivers/media/platform/coda/imx-vdoa.c index f61baf7dcbc1..67fd8ffa60a4 100644 --- a/drivers/media/platform/coda/imx-vdoa.c +++ b/drivers/media/platform/coda/imx-vdoa.c @@ -315,13 +315,13 @@ static int vdoa_remove(struct platform_device *pdev) return 0; } -static struct of_device_id vdoa_dt_ids[] = { +static const struct of_device_id vdoa_dt_ids[] = { { .compatible = "fsl,imx6q-vdoa" }, {} }; MODULE_DEVICE_TABLE(of, vdoa_dt_ids); -static struct platform_driver vdoa_driver = { +static const struct platform_driver vdoa_driver = { .probe = vdoa_probe, .remove = vdoa_remove, .driver = { -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/24] i.MX Media Driver
On Mon, 2017-01-30 at 13:06 +, Russell King - ARM Linux wrote: > > The central issue seems to be that I think media pad links / media bus > > formats should describe physical links, such as parallel or serial > > buses, and the formats of pixels flowing through them, whereas Steve > > would like to extend them to describe software transports and in-memory > > formats. > > This probably isn't the right place to attach this comment in this > thread, but... the issue of media bus formats matching physical buses > is an argument that I think is already lost. It is unfortunate that the parallel format definitions have been reused for the MIPI logical formats, but I suppose we have to live with that. Still, I think this is not a reason to open the floodgates and start describing in-memory formats using MEDIA_BUS_FMT_* Does at least the combination of logical format and number of lanes uniquiely describe the physical format? For the 4-lane LVDS bus formats there are JEIDA/VESA variants where just the bit ordering is different (MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA). > For example, take the 10-bit bayer formats: > > #define MEDIA_BUS_FMT_SBGGR10_1X10 0x3007 > #define MEDIA_BUS_FMT_SGBRG10_1X10 0x300e > #define MEDIA_BUS_FMT_SGRBG10_1X10 0x300a > #define MEDIA_BUS_FMT_SRGGB10_1X10 0x300f > > These are commonly used on CSI serial buses (see the smiapp driver for > example). From the description at the top of the file, it says the > 1X10 means that one pixel is transferred as one 10-bit sample. > > However, the format on wire is somewhat different - four pixels are > transmitted over five bytes: > > P0 P1 P2 P3 P0 P1 P2 P3 > 8-bit 8-bit 8-bit 8-bit 2-bit 2-bit 2-bit 2-bit > > This gives two problems: > 1) it doesn't fit in any sensible kind of "one pixel transferred as >N M-bit samples" description because the pixel/sample values >(depending how you look at them) are broken up. > > 2) changing this will probably be a user visible change, as things >like smiapp are already in use. > > So, I think what we actually have is the media bus formats describing > the _logical_ bus format. Yes, one pixel is transferred as one 10-bit > sample in this case. Yes. I suppose one way to look at it is that the MIPI CSI-2 specified formats are representations of corresponding parallel bus formats. > To help illustrate my point, consider the difference between > MEDIA_BUS_FMT_RGB565_1X16 and MEDIA_BUS_FMT_RGB565_2X8_BE or > MEDIA_BUS_FMT_RGB565_2X8_LE. RGB565_1X16 means 1 pixel over an effective > 16-bit wide bus (if it's not 16-bit, then it has to be broken up into > separate "samples".) RGB565_2X8 means 1 pixel as two 8-bit samples. > > So, the 10-bit bayer is 1 pixel as 1.25 bytes. Or is it, over a serial > bus. Using the RGB565 case, 10-bit bayer over a 4 lane CSI bus becomes > interesting: > > first byte 2nd 3rd > lane 1P0 9:2 S0 P7 9:2 > lane 2P1 9:2 P4 9:2 S1 > lane 3P2 9:2 P5 9:2 P8 9:2 > lane 4P3 9:2 P6 9:2 P9 9:2 > > S0 = P0/P1/P2/P3 least significant two bits > S1 = P4/P5/P6/P7 least significant two bits > > or 2 lane CSI: > first byte 2nd 3rd 4th 5th > lane 1P0 9:2 P2 S0 P5 P7 > lane 2P1 9:2 P3 P4 P6 S1 > > or 1 lane CSI: > lane 1P0 P1 P2 P3 S0 P4 P5 P6 P7 S1 P8 P9 ... These do look like three different media bus formats to me. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] doc-rst: fixed cleandoc target when used with O=dir
The cleandocs target won't work if I use a different output folder:: $ make O=/tmp/kernel SPHINXDIRS="process" cleandocs make[1]: Entering directory '/tmp/kernel' make[3]: *** No rule to make target 'clean'. Stop. ... Documentation/Makefile.sphinx:100: recipe for target 'cleandocs' failed Signed-off-by: Markus Heiser--- Documentation/Makefile.sphinx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Makefile.sphinx b/Documentation/Makefile.sphinx index e14d82a..be1936e 100644 --- a/Documentation/Makefile.sphinx +++ b/Documentation/Makefile.sphinx @@ -98,7 +98,7 @@ installmandocs: cleandocs: $(Q)rm -rf $(BUILDDIR) - $(Q)$(MAKE) BUILDDIR=$(abspath $(BUILDDIR)) -C Documentation/media clean + $(Q)$(MAKE) BUILDDIR=$(abspath $(BUILDDIR)) $(build)=Documentation/media clean endif # HAVE_SPHINX -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/24] i.MX Media Driver
On Mon, 2017-01-30 at 17:22 -0800, Steve Longerbeam wrote: > > On 01/30/2017 04:45 PM, Russell King - ARM Linux wrote: > > > > Hi, > > > > Trying this driver with an imx219 camera (which works with Philipp's > > driver) results in not much happening... no /dev/media* node for it, > > no subdevs, no nothing. No clues as to what's missing either. Only > > messages from imx-media are from registering the various subdevs. > > > > [ 37.444877] imx-media: Registered subdev imx6-mipi-csi2 > > [ 37.444973] imx-media: Registered subdev imx219 0-0010 > > [ 38.868740] imx-media: Registered subdev ipu1_ic_prpenc > > [ 38.869265] imx-media: Registered subdev ipu1_ic_prpvf > > [ 38.869425] imx-media: Registered subdev ipu1_ic_pp0 > > [ 38.870086] imx-media: Registered subdev ipu1_ic_pp1 > > [ 38.871510] imx-media: Registered subdev ipu2_ic_prpenc > > [ 38.871743] imx-media: Registered subdev ipu1_smfc0 > > [ 38.873043] imx-media: Registered subdev ipu1_smfc1 > > [ 38.873225] imx-media: Registered subdev ipu2_ic_prpvf > > [ 38.875027] imx-media: Registered subdev ipu2_smfc0 > > [ 38.875320] imx-media: Registered subdev ipu2_ic_pp0 > > [ 38.877148] imx-media: Registered subdev ipu2_smfc1 > > [ 38.877436] imx-media: Registered subdev ipu2_ic_pp1 > > [ 38.932089] imx-media: Registered subdev camif0 > > [ 38.956538] imx-media: Registered subdev camif1 > > [ 38.959148] imx-media: Registered subdev camif2 > > [ 38.964353] imx-media: Registered subdev camif3 > > [ 206.502077] imx-media: Registered subdev ipu1_csi0 > > [ 206.503304] imx-media: Registered subdev ipu1_csi1 > > [ 206.503814] imx-media: Registered subdev ipu2_csi0 > > [ 206.504281] imx-media: Registered subdev ipu2_csi1 > > > > I also get: > > > > [ 37.200072] imx6-mipi-csi2: data lanes: 2 > > [ 37.200077] imx6-mipi-csi2: flags: 0x0200 > > > > and from what I can see, all modules from drivers/staging/media/imx/ are > > loaded (had to load imx-csi by hand because of the brokenness in the > > drivers/gpu/ipu code attaching an device_node pointer after registering > > the platform device, which changes what userspace sees in the modalias > > file.) > > > > Any clues at what to look at? > > Hi Russell, > > I'm not familiar with IMX219, can you send me the source for the > imx219 subdev? I don't see it in 4.10-rc1. > > I'm also having trouble finding a datasheet for it, but from what > I've read, it has a MIPI CSI-2 interface. It should work fine as long > as it presents a single source pad, registers asynchronously, and > sets its entity function to MEDIA_ENT_F_CAM_SENSOR. What about MEDIA_ENT_F_VID_IF_BRIDGE? regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver
On Tue, 2017-01-31 at 00:01 +, Russell King - ARM Linux wrote: [...] > The iMX6 manuals call for a very specific seven sequence of initialisation > for CSI2, which begins with: > > 1. reset the D-PHY. > 2. place MIPI sensor in LP-11 state > 3. perform D-PHY initialisation > 4. configure CSI2 lanes and de-assert resets and shutdown signals > > Since you reset the CSI2 at power up and then release it, how do you > guarantee that the published sequence is followed? > > With Philipp's driver, this is easy, because there is a prepare_stream > callback which gives the sensor an opportunity to get everything > correctly configured according to the negotiated parameters, and place > the sensor in LP-11 state. > > Some sensors do not power up in LP-11 state, but need to be programmed > fully before being asked to momentarily stream. Only at that point is > the sensor guaranteed to be in the required LP-11 state. Do you expect that 1. and 2. could depend on the negotiated parameters in any way on some hardware? I had removed the prepare_stream callback from my driver in v2 because for my use case at least the above sequence could be realized by 1. in imx-mipi-csi2 s_power(1) 2. in MIPI sensor s_power(1) 3./4. in imx-mipi-csi2 s_stream(1) 4. in MIPI sensor s_stream(1) as long as the sensor is correctly put back into LP-11 in s_stream(0). regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] s5p-mfc: Align stream buffer and CPB buffer to 512
On Wed, 2017-01-18 at 15:37 +0100, Andrzej Hajda wrote: > Hi Smitha, > > On 18.01.2017 10:37, Smitha T Murthy wrote: > > >From MFCv6 onwards encoder stream buffer and decoder CPB buffer > > Unexpected char at the beginning. > > > need to be aligned with 512. > > Patch below adds checks only if buffer size is multiple of 512, am I right? > If yes, please precise the subject, for example "...CPB buffer size need > to be...". > Thank you for the review, after further analysis I found this patch is not required. So I will drop it. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/11] [media] s5p-mfc: Adding initial support for MFC v10.10
On Wed, 2017-01-18 at 16:10 +0100, Andrzej Hajda wrote: > On 18.01.2017 11:02, Smitha T Murthy wrote: > > Adding the support for MFC v10.10, with new register file and > > necessary hw control, decoder, encoder and structural changes. > > > > CC: Rob Herring> > CC: devicet...@vger.kernel.org > > Signed-off-by: Smitha T Murthy > > --- > > .../devicetree/bindings/media/s5p-mfc.txt |1 + > > drivers/media/platform/s5p-mfc/regs-mfc-v10.h | 36 > > drivers/media/platform/s5p-mfc/s5p_mfc.c | 30 + > > drivers/media/platform/s5p-mfc/s5p_mfc_common.h|4 +- > > drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c |4 ++ > > drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 44 > > +++- > > drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 21 + > > drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c|9 +++- > > drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h|2 + > > 9 files changed, 118 insertions(+), 33 deletions(-) > > create mode 100644 drivers/media/platform/s5p-mfc/regs-mfc-v10.h > > > > diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt > > b/Documentation/devicetree/bindings/media/s5p-mfc.txt > > index 2c90128..b70c613 100644 > > --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt > > +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt > > @@ -13,6 +13,7 @@ Required properties: > > (c) "samsung,mfc-v7" for MFC v7 present in Exynos5420 SoC > > (d) "samsung,mfc-v8" for MFC v8 present in Exynos5800 SoC > > (e) "samsung,exynos5433-mfc" for MFC v8 present in Exynos5433 SoC > > + (f) "samsung,mfc-v10" for MFC v10 present in a variant of Exynos7 SoC > > Could you specify explicitly SoC version(s), Exynos7 is misleading. > Btw are there plans to upstream platforms using this MFC? MFCv10.10 is used in Exynos7880. There are other variants of MFCv10 used in Exynos8890 and Exynos7870. I have no plans to upstream the platform support for this SoC, may be other members of Samsung may take it up. But I will mention the SoCs in the next version. > > > > >- reg : Physical base address of the IP registers and length of memory > > mapped region. > > diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h > > b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h > > new file mode 100644 > > index 000..bd671a5 > > --- /dev/null > > +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h > > @@ -0,0 +1,36 @@ > > +/* > > + * Register definition file for Samsung MFC V10.x Interface (FIMV) driver > > + * > > + * Copyright (c) 2017 Samsung Electronics Co., Ltd. > > + * http://www.samsung.com/ > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + */ > > + > > +#ifndef _REGS_MFC_V10_H > > +#define _REGS_MFC_V10_H > > + > > +#include > > +#include "regs-mfc-v8.h" > > + > > +/* MFCv10 register definitions*/ > > +#define S5P_FIMV_MFC_CLOCK_OFF_V10 0x7120 > > +#define S5P_FIMV_MFC_STATE_V10 0x7124 > > + > > +/* MFCv10 Context buffer sizes */ > > +#define MFC_CTX_BUF_SIZE_V10 (30 * SZ_1K)/* 30KB */ > > +#define MFC_H264_DEC_CTX_BUF_SIZE_V10 (2 * SZ_1M) /* 2MB */ > > +#define MFC_OTHER_DEC_CTX_BUF_SIZE_V10 (20 * SZ_1K)/* 20KB */ > > +#define MFC_H264_ENC_CTX_BUF_SIZE_V10 (100 * SZ_1K) /* 100KB */ > > +#define MFC_OTHER_ENC_CTX_BUF_SIZE_V10 (15 * SZ_1K)/* 15KB */ > > + > > +/* MFCv10 variant defines */ > > +#define MAX_FW_SIZE_V10(SZ_1M) /* 1MB */ > > +#define MAX_CPB_SIZE_V10 (3 * SZ_1M) /* 3MB */ > > +#define MFC_VERSION_V100xA0 > > +#define MFC_NUM_PORTS_V10 1 > > + > > +#endif /*_REGS_MFC_V10_H*/ > > + > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c > > b/drivers/media/platform/s5p-mfc/s5p_mfc.c > > index bb0a588..a043cce 100644 > > --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c > > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c > > @@ -1542,6 +1542,33 @@ static int s5p_mfc_resume(struct device *dev) > > .num_clocks = 3, > > }; > > > > +static struct s5p_mfc_buf_size_v6 mfc_buf_size_v10 = { > > + .dev_ctx= MFC_CTX_BUF_SIZE_V10, > > + .h264_dec_ctx = MFC_H264_DEC_CTX_BUF_SIZE_V10, > > + .other_dec_ctx = MFC_OTHER_DEC_CTX_BUF_SIZE_V10, > > + .h264_enc_ctx = MFC_H264_ENC_CTX_BUF_SIZE_V10, > > + .other_enc_ctx = MFC_OTHER_ENC_CTX_BUF_SIZE_V10, > > +}; > > + > > +static struct s5p_mfc_buf_size buf_size_v10 = { > > + .fw = MAX_FW_SIZE_V10, > > + .cpb= MAX_CPB_SIZE_V10, > > + .priv = _buf_size_v10, > > +}; > > + > > +static struct s5p_mfc_buf_align mfc_buf_align_v10 = { > > + .base = 0, > > +}; > > + > > +static struct s5p_mfc_variant mfc_drvdata_v10 = { > > +
Re: [PATCH v3 1/3] st-hva: encoding summary at instance release
On 01/30/2017 06:28 PM, Mauro Carvalho Chehab wrote: > Em Mon, 28 Nov 2016 11:30:52 +0100 > Jean-Christophe Trotinescreveu: > >> This patch prints unconditionnaly a short summary > > Why? Is this driver so broken that everyone would need an > unconditional "short summary" about what happened there? > > If not, then please use dev_dbg() or debugfs instead. If yes, then > we should move this driver to staging. > Hi Mauro, The unconditional character of this "short summary" was a "facility" that our customers used to get (it doesn't mean that this driver is broken). That's said, I understand your comment, and I will propose today a new version of this patch with dev_dbg() instead of dev_info(). Regards, Jean-Christophe. >> about the encoding >> operation at each instance closing, for debug purpose: >> - information about the frame (format, resolution) >> - information about the stream (format, profile, level, resolution) >> - number of encoded frames >> - potential (system, encoding...) errors >> >> Signed-off-by: Yannick Fertre >> Signed-off-by: Jean-Christophe Trotin >> --- >> drivers/media/platform/sti/hva/hva-h264.c | 6 >> drivers/media/platform/sti/hva/hva-hw.c | 5 >> drivers/media/platform/sti/hva/hva-mem.c | 5 +++- >> drivers/media/platform/sti/hva/hva-v4l2.c | 49 >> --- >> drivers/media/platform/sti/hva/hva.h | 8 + >> 5 files changed, 62 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/media/platform/sti/hva/hva-h264.c >> b/drivers/media/platform/sti/hva/hva-h264.c >> index 8cc8467..e6f247a 100644 >> --- a/drivers/media/platform/sti/hva/hva-h264.c >> +++ b/drivers/media/platform/sti/hva/hva-h264.c >> @@ -607,6 +607,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, >> "%s width(%d) or height(%d) exceeds limits (%dx%d)\n", >> pctx->name, frame_width, frame_height, >> H264_MAX_SIZE_W, H264_MAX_SIZE_H); >> +pctx->frame_errors++; >> return -EINVAL; >> } >> >> @@ -717,6 +718,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, >> default: >> dev_err(dev, "%s invalid source pixel format\n", >> pctx->name); >> +pctx->frame_errors++; >> return -EINVAL; >> } >> >> @@ -741,6 +743,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, >> >> if (td->framerate_den == 0) { >> dev_err(dev, "%s invalid framerate\n", pctx->name); >> +pctx->frame_errors++; >> return -EINVAL; >> } >> >> @@ -831,6 +834,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, >> (payload > MAX_SPS_PPS_SIZE)) { >> dev_err(dev, "%s invalid sps/pps size %d\n", pctx->name, >> payload); >> +pctx->frame_errors++; >> return -EINVAL; >> } >> >> @@ -842,6 +846,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx, >> (u8 *)stream->vaddr, >> )) { >> dev_err(dev, "%s fail to get SEI nal\n", pctx->name); >> +pctx->frame_errors++; >> return -EINVAL; >> } >> >> @@ -963,6 +968,7 @@ static int hva_h264_open(struct hva_ctx *pctx) >> err_ctx: >> devm_kfree(dev, ctx); >> err: >> +pctx->sys_errors++; >> return ret; >> } >> >> diff --git a/drivers/media/platform/sti/hva/hva-hw.c >> b/drivers/media/platform/sti/hva/hva-hw.c >> index 68d625b..5104068 100644 >> --- a/drivers/media/platform/sti/hva/hva-hw.c >> +++ b/drivers/media/platform/sti/hva/hva-hw.c >> @@ -470,6 +470,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum >> hva_hw_cmd_type cmd, >> >> if (pm_runtime_get_sync(dev) < 0) { >> dev_err(dev, "%s failed to get pm_runtime\n", ctx->name); >> +ctx->sys_errors++; >> ret = -EFAULT; >> goto out; >> } >> @@ -481,6 +482,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum >> hva_hw_cmd_type cmd, >> break; >> default: >> dev_dbg(dev, "%s unknown command 0x%x\n", ctx->name, cmd); >> +ctx->encode_errors++; >> ret = -EFAULT; >> goto out; >> } >> @@ -511,6 +513,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum >> hva_hw_cmd_type cmd, >> msecs_to_jiffies(2000))) { >> dev_err(dev, "%s %s: time out on completion\n", ctx->name, >> __func__); >> +ctx->encode_errors++; >> ret = -EFAULT; >> goto out; >> } >> @@ -518,6 +521,8 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum >> hva_hw_cmd_type cmd, >> /* get encoding status */ >> ret = ctx->hw_err ?