Re: [PATCH v4 5/5] drm: adv7511: Add support for i2c_new_secondary_device
On Tuesday 13 February 2018 11:18 PM, Kieran Bingham wrote: From: Kieran BinghamThe ADV7511 has four 256-byte maps that can be accessed via the main I2C ports. Each map has it own I2C address and acts as a standard slave device on the I2C bus. Allow a device tree node to override the default addresses so that address conflicts with other devices on the same bus may be resolved at the board description level. Queued to drm-misc-next Signed-off-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- v2: - Update missing edid-i2c address setting - Split out DT bindings - Rename and move the I2C default addresses to their own section v3: - No change v4: - Change registration order of packet/cec to fix error path and simplify code change. - Collect Laurent's RB tag drivers/gpu/drm/bridge/adv7511/adv7511.h | 6 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 42 ++-- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index d034b2cb5eee..73d8ccb97742 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -93,6 +93,11 @@ #define ADV7511_REG_CHIP_ID_HIGH 0xf5 #define ADV7511_REG_CHIP_ID_LOW 0xf6 +/* Hardware defined default addresses for I2C register maps */ +#define ADV7511_CEC_I2C_ADDR_DEFAULT 0x3c +#define ADV7511_EDID_I2C_ADDR_DEFAULT 0x3f +#define ADV7511_PACKET_I2C_ADDR_DEFAULT0x38 + #define ADV7511_CSC_ENABLEBIT(7) #define ADV7511_CSC_UPDATE_MODE BIT(5) @@ -321,6 +326,7 @@ enum adv7511_type { struct adv7511 { struct i2c_client *i2c_main; struct i2c_client *i2c_edid; + struct i2c_client *i2c_packet; struct i2c_client *i2c_cec; struct regmap *regmap; diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index efa29db5fc2b..802bc433f54a 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -586,7 +586,7 @@ static int adv7511_get_modes(struct adv7511 *adv7511, /* Reading the EDID only works if the device is powered */ if (!adv7511->powered) { unsigned int edid_i2c_addr = - (adv7511->i2c_main->addr << 1) + 4; + (adv7511->i2c_edid->addr << 1); __adv7511_power_on(adv7511); @@ -969,10 +969,10 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv) { int ret; - adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter, -adv->i2c_main->addr - 1); + adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec", + ADV7511_CEC_I2C_ADDR_DEFAULT); if (!adv->i2c_cec) - return -ENOMEM; + return -EINVAL; i2c_set_clientdata(adv->i2c_cec, adv); adv->regmap_cec = devm_regmap_init_i2c(adv->i2c_cec, @@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) struct adv7511_link_config link_config; struct adv7511 *adv7511; struct device *dev = >dev; - unsigned int main_i2c_addr = i2c->addr << 1; - unsigned int edid_i2c_addr = main_i2c_addr + 4; unsigned int val; int ret; @@ -1153,23 +1151,34 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) if (ret) goto uninit_regulators; - regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); - regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, -main_i2c_addr - 0xa); - regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, -main_i2c_addr - 2); - adv7511_packet_disable(adv7511, 0x); - adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); + adv7511->i2c_edid = i2c_new_secondary_device(i2c, "edid", + ADV7511_EDID_I2C_ADDR_DEFAULT); if (!adv7511->i2c_edid) { - ret = -ENOMEM; + ret = -EINVAL; goto uninit_regulators; } + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, +adv7511->i2c_edid->addr << 1); + + adv7511->i2c_packet = i2c_new_secondary_device(i2c, "packet", + ADV7511_PACKET_I2C_ADDR_DEFAULT); + if (!adv7511->i2c_packet) { + ret = -EINVAL; + goto err_i2c_unregister_edid; + } + + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, +
Re: [PATCH v4 2/5] dt-bindings: adv7511: Extend bindings to allow specifying slave map addresses
On Tuesday 13 February 2018 11:18 PM, Kieran Bingham wrote: From: Kieran BinghamThe ADV7511 has four 256-byte maps that can be accessed via the main I2C ports. Each map has it own I2C address and acts as a standard slave device on the I2C bus. Extend the device tree node bindings to be able to override the default addresses so that address conflicts with other devices on the same bus may be resolved at the board description level. Queued to drm-misc-next Signed-off-by: Kieran Bingham Reviewed-by: Rob Herring Reviewed-by: Laurent Pinchart --- v2: - Fixed up reg: property description to account for multiple optional addresses. - Minor reword to commit message to account for DT only change - Collected Robs RB tag v3: - Split map register addresses into individual declarations. v4: - Update commit title - Collect Laurent's RB tag - Fix nitpickings - Normalise I2C usage (I²C is harder to grep for) .../devicetree/bindings/display/bridge/adi,adv7511.txt | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index 0047b1394c70..2c887536258c 100644 --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt @@ -14,7 +14,13 @@ Required properties: "adi,adv7513" "adi,adv7533" -- reg: I2C slave address +- reg: I2C slave addresses + The ADV7511 internal registers are split into four pages exposed through + different I2C addresses, creating four register maps. Each map has it own + I2C address and acts as a standard slave device on the I2C bus. The main + address is mandatory, others are optional and revert to defaults if not + specified. + The ADV7511 supports a large number of input data formats that differ by their color depth, color format, clock mode, bit justification and random @@ -70,6 +76,9 @@ Optional properties: rather than generate its own timings for HDMI output. - clocks: from common clock binding: reference to the CEC clock. - clock-names: from common clock binding: must be "cec". +- reg-names : Names of maps with programmable addresses. + It can contain any map needing a non-default address. + Possible maps names are : "main", "edid", "cec", "packet" Required nodes: @@ -88,7 +97,12 @@ Example adv7511w: hdmi@39 { compatible = "adi,adv7511w"; - reg = <39>; + /* +* The EDID page will be accessible on address 0x66 on the I2C +* bus. All other maps continue to use their default addresses. +*/ + reg = <0x39>, <0x66>; + reg-names = "main", "edid"; interrupt-parent = <>; interrupts = <29 IRQ_TYPE_EDGE_FALLING>; clocks = <_clock>;
Re: [PATCH 2/2] drm: adv7511: Add support for i2c_new_secondary_device
Hi, On 01/22/2018 06:20 PM, Kieran Bingham wrote: The ADV7511 has four 256-byte maps that can be accessed via the main I²C ports. Each map has it own I²C address and acts as a standard slave device on the I²C bus. Allow a device tree node to override the default addresses so that address conflicts with other devices on the same bus may be resolved at the board description level. Signed-off-by: Kieran Bingham--- .../bindings/display/bridge/adi,adv7511.txt| 10 +- drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 +++ drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 36 ++ 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index 0047b1394c70..f6bb9f6d3f48 100644 --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt @@ -70,6 +70,9 @@ Optional properties: rather than generate its own timings for HDMI output. - clocks: from common clock binding: reference to the CEC clock. - clock-names: from common clock binding: must be "cec". +- reg-names : Names of maps with programmable addresses. + It can contain any map needing a non-default address. + Possible maps names are : "main", "edid", "cec", "packet" Required nodes: @@ -88,7 +91,12 @@ Example adv7511w: hdmi@39 { compatible = "adi,adv7511w"; - reg = <39>; + /* +* The EDID page will be accessible on address 0x66 on the i2c +* bus. All other maps continue to use their default addresses. +*/ + reg = <0x39 0x66>; + reg-names = "main", "edid"; interrupt-parent = <>; interrupts = <29 IRQ_TYPE_EDGE_FALLING>; clocks = <_clock>; diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index d034b2cb5eee..7d81ce3808e0 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -53,8 +53,10 @@ #define ADV7511_REG_POWER 0x41 #define ADV7511_REG_STATUS0x42 #define ADV7511_REG_EDID_I2C_ADDR 0x43 +#define ADV7511_REG_EDID_I2C_ADDR_DEFAULT 0x3f #define ADV7511_REG_PACKET_ENABLE10x44 #define ADV7511_REG_PACKET_I2C_ADDR 0x45 +#define ADV7511_REG_PACKET_I2C_ADDR_DEFAULT0x38 #define ADV7511_REG_DSD_ENABLE0x46 #define ADV7511_REG_VIDEO_INPUT_CFG2 0x48 #define ADV7511_REG_INFOFRAME_UPDATE 0x4a @@ -89,6 +91,7 @@ #define ADV7511_REG_TMDS_CLOCK_INV0xde #define ADV7511_REG_ARC_CTRL 0xdf #define ADV7511_REG_CEC_I2C_ADDR 0xe1 +#define ADV7511_REG_CEC_I2C_ADDR_DEFAULT 0x3c Minor comment: The defines above make look like new register defines. It would be nice to remove the "REG_" from them, and place them somewhere after the register definitions. #define ADV7511_REG_CEC_CTRL 0xe2 #define ADV7511_REG_CHIP_ID_HIGH 0xf5 #define ADV7511_REG_CHIP_ID_LOW 0xf6 @@ -322,6 +325,7 @@ struct adv7511 { struct i2c_client *i2c_main; struct i2c_client *i2c_edid; struct i2c_client *i2c_cec; + struct i2c_client *i2c_packet; struct regmap *regmap; struct regmap *regmap_cec; diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index efa29db5fc2b..7ec33837752b 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -969,8 +969,8 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv) { int ret; - adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter, -adv->i2c_main->addr - 1); This patch avoids deriving the CEC/EDID map addresses from the main map. I think this would break what the original patch tried to do: d25a4cbba4b9da7c2d674b2f8ecf84af1b24988e "drm/bridge: adv7511: add support for the 2nd chip" Maybe the default macros can be a function of the main address? + adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec", + ADV7511_REG_CEC_I2C_ADDR_DEFAULT); Also, I'm a bit unclear on the default address values. For example, previously, the CEC address was calculated as "adv->i2c_main->addr - 1", which is 0x38. The new CEC_I2C_ADDR_DEFAULT define sets it to 0x3c. Thanks, Archit if (!adv->i2c_cec) return -ENOMEM; i2c_set_clientdata(adv->i2c_cec, adv); @@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
Re: [PATCH v4] drm: bridge: synopsys/dw-hdmi: Enable cec clock
On 11/26/2017 01:48 AM, Pierre-Hugues Husson wrote: Support the "cec" optional clock. The documentation already mentions "cec" optional clock and it is used by several boards, but currently the driver doesn't enable it, thus preventing cec from working on those boards. And even worse: a /dev/cecX device will appear for those boards, but it won't be functioning without configuring this clock. Thanks for the updating the commit message. I will queue this to drm-misc-fixes once it's updated with the 4.15-rc1 tag. Thanks, Archit Changes: v4: - Change commit message to stress the importance of this patch v3: - Drop useless braces v2: - Separate ENOENT errors from others - Propagate other errors (especially -EPROBE_DEFER) Signed-off-by: Pierre-Hugues Husson--- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 25 + 1 file changed, 25 insertions(+) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index bf14214fa464..d82b9747a979 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -138,6 +138,7 @@ struct dw_hdmi { struct device *dev; struct clk *isfr_clk; struct clk *iahb_clk; + struct clk *cec_clk; struct dw_hdmi_i2c *i2c; struct hdmi_data_info hdmi_data; @@ -2382,6 +2383,26 @@ __dw_hdmi_probe(struct platform_device *pdev, goto err_isfr; } + hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec"); + if (PTR_ERR(hdmi->cec_clk) == -ENOENT) { + hdmi->cec_clk = NULL; + } else if (IS_ERR(hdmi->cec_clk)) { + ret = PTR_ERR(hdmi->cec_clk); + if (ret != -EPROBE_DEFER) + dev_err(hdmi->dev, "Cannot get HDMI cec clock: %d\n", + ret); + + hdmi->cec_clk = NULL; + goto err_iahb; + } else { + ret = clk_prepare_enable(hdmi->cec_clk); + if (ret) { + dev_err(hdmi->dev, "Cannot enable HDMI cec clock: %d\n", + ret); + goto err_iahb; + } + } + /* Product and revision IDs */ hdmi->version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8) | (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0); @@ -2518,6 +2539,8 @@ __dw_hdmi_probe(struct platform_device *pdev, cec_notifier_put(hdmi->cec_notifier); clk_disable_unprepare(hdmi->iahb_clk); + if (hdmi->cec_clk) + clk_disable_unprepare(hdmi->cec_clk); err_isfr: clk_disable_unprepare(hdmi->isfr_clk); err_res: @@ -2541,6 +2564,8 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi) clk_disable_unprepare(hdmi->iahb_clk); clk_disable_unprepare(hdmi->isfr_clk); + if (hdmi->cec_clk) + clk_disable_unprepare(hdmi->cec_clk); if (hdmi->i2c) i2c_del_adapter(>i2c->adap); -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v3] drm: bridge: synopsys/dw-hdmi: Enable cec clock
Hi, On 11/20/2017 06:00 PM, Hans Verkuil wrote: I didn't see this merged for 4.15, is it too late to include this? All other changes needed to get CEC to work on rk3288 and rk3399 are all merged. Sorry for the late reply. I was out last week. Dave recently sent the second pull request for 4.15, so I think it would be hard to get it in the merge window now. We could target it for the 4.15-rcs since it is preventing the feature from working. Is it possible to rephrase the commit message a bit so that it's clear that we need it for CEC to work? Thanks, Archit Regards, Hans On 10/26/2017 08:19 PM, Pierre-Hugues Husson wrote: The documentation already mentions "cec" optional clock, but currently the driver doesn't enable it. Changes: v3: - Drop useless braces v2: - Separate ENOENT errors from others - Propagate other errors (especially -EPROBE_DEFER) Signed-off-by: Pierre-Hugues Husson--- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 25 + 1 file changed, 25 insertions(+) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index bf14214fa464..d82b9747a979 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -138,6 +138,7 @@ struct dw_hdmi { struct device *dev; struct clk *isfr_clk; struct clk *iahb_clk; + struct clk *cec_clk; struct dw_hdmi_i2c *i2c; struct hdmi_data_info hdmi_data; @@ -2382,6 +2383,26 @@ __dw_hdmi_probe(struct platform_device *pdev, goto err_isfr; } + hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec"); + if (PTR_ERR(hdmi->cec_clk) == -ENOENT) { + hdmi->cec_clk = NULL; + } else if (IS_ERR(hdmi->cec_clk)) { + ret = PTR_ERR(hdmi->cec_clk); + if (ret != -EPROBE_DEFER) + dev_err(hdmi->dev, "Cannot get HDMI cec clock: %d\n", + ret); + + hdmi->cec_clk = NULL; + goto err_iahb; + } else { + ret = clk_prepare_enable(hdmi->cec_clk); + if (ret) { + dev_err(hdmi->dev, "Cannot enable HDMI cec clock: %d\n", + ret); + goto err_iahb; + } + } + /* Product and revision IDs */ hdmi->version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8) | (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0); @@ -2518,6 +2539,8 @@ __dw_hdmi_probe(struct platform_device *pdev, cec_notifier_put(hdmi->cec_notifier); clk_disable_unprepare(hdmi->iahb_clk); + if (hdmi->cec_clk) + clk_disable_unprepare(hdmi->cec_clk); err_isfr: clk_disable_unprepare(hdmi->isfr_clk); err_res: @@ -2541,6 +2564,8 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi) clk_disable_unprepare(hdmi->iahb_clk); clk_disable_unprepare(hdmi->isfr_clk); + if (hdmi->cec_clk) + clk_disable_unprepare(hdmi->cec_clk); if (hdmi->i2c) i2c_del_adapter(>i2c->adap); -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCHv3 0/2] drm/bridge/adv7511: add CEC support
On 10/07/2017 04:16 PM, Hans Verkuil wrote: From: Hans VerkuilThis patch series adds CEC support to the drm adv7511/adv7533 drivers. I have tested this with the Qualcomm Dragonboard C410 (adv7533 based) and the Renesas R-Car Koelsch board (adv7511 based). I only have the Koelsch board to test with, but it looks like other R-Car boards use the same adv7511. It would be nice if someone can add CEC support to the other R-Car boards as well. The main thing to check is if they all use the same 12 MHz fixed CEC clock source. queued to drm-misc-next. Thanks, Archit Anyone who wants to test this will need the CEC utilities that are part of the v4l-utils git repository: git clone git://linuxtv.org/v4l-utils.git cd v4l-utils ./bootstrap.sh ./configure make sudo make install Now configure the CEC adapter as a Playback device: cec-ctl --playback Discover other CEC devices: cec-ctl -S Regards, Hans Changes since v2: - A small rewording of the 'clocks' property description in the bindings as per Sergei's comment. Changes since v1: - Incorporate Archit's comments: use defines for irq masks combine the adv7511/33 regmap_configs adv7511_cec_init now handles dt parsing & CEC registration - Use the new (4.14) CEC_CAP_DEFAULTS define Hans Verkuil (2): dt-bindings: adi,adv7511.txt: document cec clock drm: adv7511/33: add HDMI CEC support .../bindings/display/bridge/adi,adv7511.txt| 4 + drivers/gpu/drm/bridge/adv7511/Kconfig | 8 + drivers/gpu/drm/bridge/adv7511/Makefile| 1 + drivers/gpu/drm/bridge/adv7511/adv7511.h | 43 ++- drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 337 + drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 116 ++- drivers/gpu/drm/bridge/adv7511/adv7533.c | 38 +-- 7 files changed, 489 insertions(+), 58 deletions(-) create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_cec.c -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCHv2 2/2] drm: adv7511/33: add HDMI CEC support
Hi Hans, On 09/19/2017 01:03 PM, Hans Verkuil wrote: From: Hans VerkuilAdd support for HDMI CEC to the drm adv7511/adv7533 drivers. The CEC registers that we need to use are identical for both drivers, but they appear at different offsets in the register map. The patch looks good to me. I can go ahead an queue it to drm-misc-next. There were some minor comments on the DT bindings patch. Are you planning to send a new patch for that? Thanks, Archit Signed-off-by: Hans Verkuil --- drivers/gpu/drm/bridge/adv7511/Kconfig | 8 + drivers/gpu/drm/bridge/adv7511/Makefile | 1 + drivers/gpu/drm/bridge/adv7511/adv7511.h | 43 +++- drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 337 +++ drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 116 +++-- drivers/gpu/drm/bridge/adv7511/adv7533.c | 38 +-- 6 files changed, 485 insertions(+), 58 deletions(-) create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_cec.c diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig index 2fed567f9943..592b9d2ec034 100644 --- a/drivers/gpu/drm/bridge/adv7511/Kconfig +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig @@ -21,3 +21,11 @@ config DRM_I2C_ADV7533 default y help Support for the Analog Devices ADV7533 DSI to HDMI encoder. + +config DRM_I2C_ADV7511_CEC + bool "ADV7511/33 HDMI CEC driver" + depends on DRM_I2C_ADV7511 + select CEC_CORE + default y + help + When selected the HDMI transmitter will support the CEC feature. diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile b/drivers/gpu/drm/bridge/adv7511/Makefile index 5ba675534f6e..5bb384938a71 100644 --- a/drivers/gpu/drm/bridge/adv7511/Makefile +++ b/drivers/gpu/drm/bridge/adv7511/Makefile @@ -1,4 +1,5 @@ adv7511-y := adv7511_drv.o adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o +adv7511-$(CONFIG_DRM_I2C_ADV7511_CEC) += adv7511_cec.o adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index fe18a5d2d84b..543a5eb91624 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -195,6 +195,25 @@ #define ADV7511_PACKET_GM(x) ADV7511_PACKET(5, x) #define ADV7511_PACKET_SPARE(x) ADV7511_PACKET(6, x) +#define ADV7511_REG_CEC_TX_FRAME_HDR 0x00 +#define ADV7511_REG_CEC_TX_FRAME_DATA0 0x01 +#define ADV7511_REG_CEC_TX_FRAME_LEN 0x10 +#define ADV7511_REG_CEC_TX_ENABLE 0x11 +#define ADV7511_REG_CEC_TX_RETRY 0x12 +#define ADV7511_REG_CEC_TX_LOW_DRV_CNT 0x14 +#define ADV7511_REG_CEC_RX_FRAME_HDR 0x15 +#define ADV7511_REG_CEC_RX_FRAME_DATA0 0x16 +#define ADV7511_REG_CEC_RX_FRAME_LEN 0x25 +#define ADV7511_REG_CEC_RX_ENABLE 0x26 +#define ADV7511_REG_CEC_RX_BUFFERS 0x4a +#define ADV7511_REG_CEC_LOG_ADDR_MASK 0x4b +#define ADV7511_REG_CEC_LOG_ADDR_0_1 0x4c +#define ADV7511_REG_CEC_LOG_ADDR_2 0x4d +#define ADV7511_REG_CEC_CLK_DIV0x4e +#define ADV7511_REG_CEC_SOFT_RESET 0x50 + +#define ADV7533_REG_CEC_OFFSET 0x70 + enum adv7511_input_clock { ADV7511_INPUT_CLOCK_1X, ADV7511_INPUT_CLOCK_2X, @@ -297,6 +316,8 @@ enum adv7511_type { ADV7533, }; +#define ADV7511_MAX_ADDRS 3 + struct adv7511 { struct i2c_client *i2c_main; struct i2c_client *i2c_edid; @@ -343,15 +364,27 @@ struct adv7511 { enum adv7511_type type; struct platform_device *audio_pdev; + + struct cec_adapter *cec_adap; + u8 cec_addr[ADV7511_MAX_ADDRS]; + u8 cec_valid_addrs; + bool cec_enabled_adap; + struct clk *cec_clk; + u32 cec_clk_freq; }; +#ifdef CONFIG_DRM_I2C_ADV7511_CEC +int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511, +unsigned int offset); +void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1); +#endif + #ifdef CONFIG_DRM_I2C_ADV7533 void adv7533_dsi_power_on(struct adv7511 *adv); void adv7533_dsi_power_off(struct adv7511 *adv); void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode); int adv7533_patch_registers(struct adv7511 *adv); -void adv7533_uninit_cec(struct adv7511 *adv); -int adv7533_init_cec(struct adv7511 *adv); +int adv7533_patch_cec_registers(struct adv7511 *adv); int adv7533_attach_dsi(struct adv7511 *adv); void adv7533_detach_dsi(struct adv7511 *adv); int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv); @@ -374,11 +407,7 @@ static inline int adv7533_patch_registers(struct adv7511 *adv) return -ENODEV; } -static inline void adv7533_uninit_cec(struct adv7511 *adv) -{ -} - -static inline int adv7533_init_cec(struct adv7511 *adv) +static inline int adv7533_patch_cec_registers(struct adv7511 *adv) {
Re: [PATCH 0/4] drm/bridge/adv7511: add CEC support
On 08/10/2017 02:26 PM, Hans Verkuil wrote: On 10/08/17 10:49, Archit Taneja wrote: Hi Hans, On 07/30/2017 06:37 PM, Hans Verkuil wrote: From: Hans Verkuil <hans.verk...@cisco.com> This patch series adds CEC support to the drm adv7511/adv7533 drivers. I have tested this with the Qualcomm Dragonboard C410 (adv7533 based) and the Renesas R-Car Koelsch board (adv7511 based). Note: the Dragonboard needs this patch: https://patchwork.kernel.org/patch/9824773/ Archit, can you confirm that this patch will go to kernel 4.14? And the Koelsch board needs this 4.13 fix: https://patchwork.kernel.org/patch/9836865/ I only have the Koelsch board to test with, but it looks like other R-Car boards use the same adv7511. It would be nice if someone can add CEC support to the other R-Car boards as well. The main thing to check is if they all use the same 12 MHz fixed CEC clock source. Anyone who wants to test this will need the CEC utilities that are part of the v4l-utils git repository: git clone git://linuxtv.org/v4l-utils.git cd v4l-utils ./bootstrap.sh ./configure make sudo make install Now configure the CEC adapter as a Playback device: cec-ctl --playback Discover other CEC devices: cec-ctl -S I tried the instructions, and I get the following output. I don't think I have any CEC device connected, though. Is this the expected behaviour? #cec-ctl -S Driver Info: Driver Name: adv7511 Adapter Name : 3-0039 Capabilities : 0x000e Logical Addresses Transmit Passthrough Driver version : 4.13.0 Available Logical Addresses: 3 Physical Address : 1.0.0.0 Logical Address Mask : 0x CEC Version: 2.0 Logical Addresses : 0 #cec-ctl --playback [ 1038.761545] cec-3-0039: cec_thread_func: message 44 timed out! This isn't right. You shouldn't see this. It never receives an interrupt when the transmit has finished, which causes these time outs. What are you testing this on? The Dragonboard c410? Yes. Can you check the cec clock frequency? It should be 19.2 MHz. Remember to apply this patch: https://patchwork.kernel.org/patch/9824773/ You probably did, but just in case... clk_summary does show bb_clk2 to be set to 19.2 Mhz. I applied a newer version of this patch, which got merged in clk-next: https://patchwork.kernel.org/patch/9845493/ I will try to apply the patch you use and check again. Thanks, Archit Regards, Hans Driver Info: Driver Name: adv7511 Adapter Name : 3-0039 Capabilities : 0x000e Logical Addresses Transmit Passthrough Driver version : 4.13.0 Available Logical Addresses: 3 Physical Address : 1.0.0.0 Logical Address Mask : 0x0010 CEC Version: 2.0 Vendor ID : 0x000c03 Logical Addresses : 1 (Allow RC Passthrough) Logical Address : 4 Primary Device Type: Playback Logical Address Type : Playback All Device Types : Playback RC TV Profile : None Device Features: None [ 1041.063605] cec-3-0039: cec_thread_func: message 4f a6 06 10 00 00 timed out! [ 1043.367482] cec-3-0039: cec_thread_func: message 4f 84 10 00 04 timed out! Thanks, Archit Regards, Hans Hans Verkuil (4): dt-bindings: adi,adv7511.txt: document cec clock arm: dts: qcom: add cec clock for apq8016 board arm: dts: renesas: add cec clock for Koelsch board drm: adv7511/33: add HDMI CEC support .../bindings/display/bridge/adi,adv7511.txt| 4 + arch/arm/boot/dts/r8a7791-koelsch.dts | 8 + arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 2 + drivers/gpu/drm/bridge/adv7511/Kconfig | 8 + drivers/gpu/drm/bridge/adv7511/Makefile| 1 + drivers/gpu/drm/bridge/adv7511/adv7511.h | 45 ++- drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 314 + drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 152 +- drivers/gpu/drm/bridge/adv7511/adv7533.c | 30 +- 9 files changed, 514 insertions(+), 50 deletions(-) create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_cec.c -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 0/4] drm/bridge/adv7511: add CEC support
Hi Hans, On 07/30/2017 06:37 PM, Hans Verkuil wrote: From: Hans VerkuilThis patch series adds CEC support to the drm adv7511/adv7533 drivers. I have tested this with the Qualcomm Dragonboard C410 (adv7533 based) and the Renesas R-Car Koelsch board (adv7511 based). Note: the Dragonboard needs this patch: https://patchwork.kernel.org/patch/9824773/ Archit, can you confirm that this patch will go to kernel 4.14? And the Koelsch board needs this 4.13 fix: https://patchwork.kernel.org/patch/9836865/ I only have the Koelsch board to test with, but it looks like other R-Car boards use the same adv7511. It would be nice if someone can add CEC support to the other R-Car boards as well. The main thing to check is if they all use the same 12 MHz fixed CEC clock source. Anyone who wants to test this will need the CEC utilities that are part of the v4l-utils git repository: git clone git://linuxtv.org/v4l-utils.git cd v4l-utils ./bootstrap.sh ./configure make sudo make install Now configure the CEC adapter as a Playback device: cec-ctl --playback Discover other CEC devices: cec-ctl -S I tried the instructions, and I get the following output. I don't think I have any CEC device connected, though. Is this the expected behaviour? #cec-ctl -S Driver Info: Driver Name: adv7511 Adapter Name : 3-0039 Capabilities : 0x000e Logical Addresses Transmit Passthrough Driver version : 4.13.0 Available Logical Addresses: 3 Physical Address : 1.0.0.0 Logical Address Mask : 0x CEC Version: 2.0 Logical Addresses : 0 #cec-ctl --playback [ 1038.761545] cec-3-0039: cec_thread_func: message 44 timed out! Driver Info: Driver Name: adv7511 Adapter Name : 3-0039 Capabilities : 0x000e Logical Addresses Transmit Passthrough Driver version : 4.13.0 Available Logical Addresses: 3 Physical Address : 1.0.0.0 Logical Address Mask : 0x0010 CEC Version: 2.0 Vendor ID : 0x000c03 Logical Addresses : 1 (Allow RC Passthrough) Logical Address : 4 Primary Device Type: Playback Logical Address Type : Playback All Device Types : Playback RC TV Profile : None Device Features: None [ 1041.063605] cec-3-0039: cec_thread_func: message 4f a6 06 10 00 00 timed out! [ 1043.367482] cec-3-0039: cec_thread_func: message 4f 84 10 00 04 timed out! Thanks, Archit Regards, Hans Hans Verkuil (4): dt-bindings: adi,adv7511.txt: document cec clock arm: dts: qcom: add cec clock for apq8016 board arm: dts: renesas: add cec clock for Koelsch board drm: adv7511/33: add HDMI CEC support .../bindings/display/bridge/adi,adv7511.txt| 4 + arch/arm/boot/dts/r8a7791-koelsch.dts | 8 + arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 2 + drivers/gpu/drm/bridge/adv7511/Kconfig | 8 + drivers/gpu/drm/bridge/adv7511/Makefile| 1 + drivers/gpu/drm/bridge/adv7511/adv7511.h | 45 ++- drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 314 + drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 152 +- drivers/gpu/drm/bridge/adv7511/adv7533.c | 30 +- 9 files changed, 514 insertions(+), 50 deletions(-) create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_cec.c -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 4/4] drm: adv7511/33: add HDMI CEC support
On 07/30/2017 06:37 PM, Hans Verkuil wrote: From: Hans VerkuilAdd support for HDMI CEC to the drm adv7511/adv7533 drivers. The CEC registers that we need to use are identical for both drivers, but they appear at different offsets in the register map. Thanks for the patch. Some minor comments below. Signed-off-by: Hans Verkuil --- drivers/gpu/drm/bridge/adv7511/Kconfig | 8 + drivers/gpu/drm/bridge/adv7511/Makefile | 1 + drivers/gpu/drm/bridge/adv7511/adv7511.h | 45 +++- drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 314 +++ drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 152 +++-- drivers/gpu/drm/bridge/adv7511/adv7533.c | 30 +-- 6 files changed, 500 insertions(+), 50 deletions(-) create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_cec.c diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig index 2fed567f9943..592b9d2ec034 100644 --- a/drivers/gpu/drm/bridge/adv7511/Kconfig +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig @@ -21,3 +21,11 @@ config DRM_I2C_ADV7533 default y help Support for the Analog Devices ADV7533 DSI to HDMI encoder. + +config DRM_I2C_ADV7511_CEC + bool "ADV7511/33 HDMI CEC driver" + depends on DRM_I2C_ADV7511 + select CEC_CORE + default y + help + When selected the HDMI transmitter will support the CEC feature. diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile b/drivers/gpu/drm/bridge/adv7511/Makefile index 5ba675534f6e..5bb384938a71 100644 --- a/drivers/gpu/drm/bridge/adv7511/Makefile +++ b/drivers/gpu/drm/bridge/adv7511/Makefile @@ -1,4 +1,5 @@ adv7511-y := adv7511_drv.o adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += adv7511_audio.o +adv7511-$(CONFIG_DRM_I2C_ADV7511_CEC) += adv7511_cec.o adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index fe18a5d2d84b..4fd7b14f619b 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -195,6 +195,25 @@ #define ADV7511_PACKET_GM(x) ADV7511_PACKET(5, x) #define ADV7511_PACKET_SPARE(x) ADV7511_PACKET(6, x) +#define ADV7511_REG_CEC_TX_FRAME_HDR 0x00 +#define ADV7511_REG_CEC_TX_FRAME_DATA0 0x01 +#define ADV7511_REG_CEC_TX_FRAME_LEN 0x10 +#define ADV7511_REG_CEC_TX_ENABLE 0x11 +#define ADV7511_REG_CEC_TX_RETRY 0x12 +#define ADV7511_REG_CEC_TX_LOW_DRV_CNT 0x14 +#define ADV7511_REG_CEC_RX_FRAME_HDR 0x15 +#define ADV7511_REG_CEC_RX_FRAME_DATA0 0x16 +#define ADV7511_REG_CEC_RX_FRAME_LEN 0x25 +#define ADV7511_REG_CEC_RX_ENABLE 0x26 +#define ADV7511_REG_CEC_RX_BUFFERS 0x4a +#define ADV7511_REG_CEC_LOG_ADDR_MASK 0x4b +#define ADV7511_REG_CEC_LOG_ADDR_0_1 0x4c +#define ADV7511_REG_CEC_LOG_ADDR_2 0x4d +#define ADV7511_REG_CEC_CLK_DIV0x4e +#define ADV7511_REG_CEC_SOFT_RESET 0x50 + +#define ADV7533_REG_CEC_OFFSET 0x70 + enum adv7511_input_clock { ADV7511_INPUT_CLOCK_1X, ADV7511_INPUT_CLOCK_2X, @@ -297,6 +316,8 @@ enum adv7511_type { ADV7533, }; +#define ADV7511_MAX_ADDRS 3 + struct adv7511 { struct i2c_client *i2c_main; struct i2c_client *i2c_edid; @@ -343,15 +364,29 @@ struct adv7511 { enum adv7511_type type; struct platform_device *audio_pdev; + + struct cec_adapter *cec_adap; + u8 cec_addr[ADV7511_MAX_ADDRS]; + u8 cec_valid_addrs; + bool cec_enabled_adap; + struct clk *cec_clk; + u32 cec_clk_freq; }; +#ifdef CONFIG_DRM_I2C_ADV7511_CEC +extern const struct cec_adap_ops adv7511_cec_adap_ops; + +void adv7511_cec_init(struct adv7511 *adv7511, unsigned int offset); +int adv7511_cec_parse_dt(struct device *dev, struct adv7511 *adv7511); +void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1); +#endif + #ifdef CONFIG_DRM_I2C_ADV7533 void adv7533_dsi_power_on(struct adv7511 *adv); void adv7533_dsi_power_off(struct adv7511 *adv); void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode); int adv7533_patch_registers(struct adv7511 *adv); -void adv7533_uninit_cec(struct adv7511 *adv); -int adv7533_init_cec(struct adv7511 *adv); +int adv7533_patch_cec_registers(struct adv7511 *adv); int adv7533_attach_dsi(struct adv7511 *adv); void adv7533_detach_dsi(struct adv7511 *adv); int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv); @@ -374,11 +409,7 @@ static inline int adv7533_patch_registers(struct adv7511 *adv) return -ENODEV; } -static inline void adv7533_uninit_cec(struct adv7511 *adv) -{ -} - -static inline int adv7533_init_cec(struct adv7511 *adv) +static inline int adv7533_patch_cec_registers(struct adv7511 *adv) { return -ENODEV; } diff --git
Re: [PATCH 0/4] drm/bridge/adv7511: add CEC support
On 07/30/2017 06:37 PM, Hans Verkuil wrote: From: Hans VerkuilThis patch series adds CEC support to the drm adv7511/adv7533 drivers. I have tested this with the Qualcomm Dragonboard C410 (adv7533 based) and the Renesas R-Car Koelsch board (adv7511 based). Note: the Dragonboard needs this patch: https://patchwork.kernel.org/patch/9824773/ Archit, can you confirm that this patch will go to kernel 4.14? Yes, it's been queued to clk-next. Thanks, Archit And the Koelsch board needs this 4.13 fix: https://patchwork.kernel.org/patch/9836865/ I only have the Koelsch board to test with, but it looks like other R-Car boards use the same adv7511. It would be nice if someone can add CEC support to the other R-Car boards as well. The main thing to check is if they all use the same 12 MHz fixed CEC clock source. Anyone who wants to test this will need the CEC utilities that are part of the v4l-utils git repository: git clone git://linuxtv.org/v4l-utils.git cd v4l-utils ./bootstrap.sh ./configure make sudo make install Now configure the CEC adapter as a Playback device: cec-ctl --playback Discover other CEC devices: cec-ctl -S Regards, Hans Hans Verkuil (4): dt-bindings: adi,adv7511.txt: document cec clock arm: dts: qcom: add cec clock for apq8016 board arm: dts: renesas: add cec clock for Koelsch board drm: adv7511/33: add HDMI CEC support .../bindings/display/bridge/adi,adv7511.txt| 4 + arch/arm/boot/dts/r8a7791-koelsch.dts | 8 + arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 2 + drivers/gpu/drm/bridge/adv7511/Kconfig | 8 + drivers/gpu/drm/bridge/adv7511/Makefile| 1 + drivers/gpu/drm/bridge/adv7511/adv7511.h | 45 ++- drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 314 + drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 152 +- drivers/gpu/drm/bridge/adv7511/adv7533.c | 30 +- 9 files changed, 514 insertions(+), 50 deletions(-) create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_cec.c -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCHv3 4/4] drm/bridge: dw-hdmi: remove CEC engine register definitions
On 08/03/2017 12:11 AM, Hans Verkuil wrote: From: Russell KingWe don't need the CEC engine register definitions, so let's remove them. Queued to drm-misc-next. Thanks, Archit Signed-off-by: Russell King Acked-by: Hans Verkuil Tested-by: Hans Verkuil Tested-by: Laurent Pinchart --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 45 --- 1 file changed, 45 deletions(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index 69644c83a0f8..9d90eb9c46e5 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h @@ -478,51 +478,6 @@ #define HDMI_A_PRESETUP 0x501A #define HDMI_A_SRM_BASE 0x5020 -/* CEC Engine Registers */ -#define HDMI_CEC_CTRL 0x7D00 -#define HDMI_CEC_STAT 0x7D01 -#define HDMI_CEC_MASK 0x7D02 -#define HDMI_CEC_POLARITY 0x7D03 -#define HDMI_CEC_INT0x7D04 -#define HDMI_CEC_ADDR_L 0x7D05 -#define HDMI_CEC_ADDR_H 0x7D06 -#define HDMI_CEC_TX_CNT 0x7D07 -#define HDMI_CEC_RX_CNT 0x7D08 -#define HDMI_CEC_TX_DATA0 0x7D10 -#define HDMI_CEC_TX_DATA1 0x7D11 -#define HDMI_CEC_TX_DATA2 0x7D12 -#define HDMI_CEC_TX_DATA3 0x7D13 -#define HDMI_CEC_TX_DATA4 0x7D14 -#define HDMI_CEC_TX_DATA5 0x7D15 -#define HDMI_CEC_TX_DATA6 0x7D16 -#define HDMI_CEC_TX_DATA7 0x7D17 -#define HDMI_CEC_TX_DATA8 0x7D18 -#define HDMI_CEC_TX_DATA9 0x7D19 -#define HDMI_CEC_TX_DATA10 0x7D1a -#define HDMI_CEC_TX_DATA11 0x7D1b -#define HDMI_CEC_TX_DATA12 0x7D1c -#define HDMI_CEC_TX_DATA13 0x7D1d -#define HDMI_CEC_TX_DATA14 0x7D1e -#define HDMI_CEC_TX_DATA15 0x7D1f -#define HDMI_CEC_RX_DATA0 0x7D20 -#define HDMI_CEC_RX_DATA1 0x7D21 -#define HDMI_CEC_RX_DATA2 0x7D22 -#define HDMI_CEC_RX_DATA3 0x7D23 -#define HDMI_CEC_RX_DATA4 0x7D24 -#define HDMI_CEC_RX_DATA5 0x7D25 -#define HDMI_CEC_RX_DATA6 0x7D26 -#define HDMI_CEC_RX_DATA7 0x7D27 -#define HDMI_CEC_RX_DATA8 0x7D28 -#define HDMI_CEC_RX_DATA9 0x7D29 -#define HDMI_CEC_RX_DATA10 0x7D2a -#define HDMI_CEC_RX_DATA11 0x7D2b -#define HDMI_CEC_RX_DATA12 0x7D2c -#define HDMI_CEC_RX_DATA13 0x7D2d -#define HDMI_CEC_RX_DATA14 0x7D2e -#define HDMI_CEC_RX_DATA15 0x7D2f -#define HDMI_CEC_LOCK 0x7D30 -#define HDMI_CEC_WKUPCTRL 0x7D31 - /* I2C Master Registers (E-DDC) */ #define HDMI_I2CM_SLAVE 0x7E00 #define HDMI_I2CM_ADDRESS 0x7E01 -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCHv3 3/4] drm/bridge: dw-hdmi: add cec driver
On 08/03/2017 12:11 AM, Hans Verkuil wrote: From: Russell KingAdd a CEC driver for the dw-hdmi hardware. Queued to drm-misc-next after fixing up some minor Makefile/Kconfig conflicts. Thanks, Archit Reviewed-by: Neil Armstrong Signed-off-by: Russell King [hans.verkuil: unsigned -> unsigned int] [hans.verkuil: cec_transmit_done -> cec_transmit_attempt_done] [hans.verkuil: add missing CEC_CAP_PASSTHROUGH] Acked-by: Hans Verkuil Tested-by: Hans Verkuil Tested-by: Laurent Pinchart --- drivers/gpu/drm/bridge/synopsys/Kconfig | 9 + drivers/gpu/drm/bridge/synopsys/Makefile | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 327 ++ drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h | 19 ++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 42 +++- drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 1 + 6 files changed, 398 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig index 351db000599a..d34c3dc04ba9 100644 --- a/drivers/gpu/drm/bridge/synopsys/Kconfig +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig @@ -23,3 +23,12 @@ config DRM_DW_HDMI_I2S_AUDIO help Support the I2S Audio interface which is part of the Synopsys Designware HDMI block. + +config DRM_DW_HDMI_CEC + tristate "Synopsis Designware CEC interface" + depends on DRM_DW_HDMI + select CEC_CORE + select CEC_NOTIFIER + help + Support the CE interface which is part of the Synopsys + Designware HDMI block. diff --git a/drivers/gpu/drm/bridge/synopsys/Makefile b/drivers/gpu/drm/bridge/synopsys/Makefile index 17aa7a65b57e..6fe415903668 100644 --- a/drivers/gpu/drm/bridge/synopsys/Makefile +++ b/drivers/gpu/drm/bridge/synopsys/Makefile @@ -3,3 +3,4 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o obj-$(CONFIG_DRM_DW_HDMI_I2S_AUDIO) += dw-hdmi-i2s-audio.o +obj-$(CONFIG_DRM_DW_HDMI_CEC) += dw-hdmi-cec.o diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c new file mode 100644 index ..6c323510f128 --- /dev/null +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c @@ -0,0 +1,327 @@ +/* + * Designware HDMI CEC driver + * + * Copyright (C) 2015-2017 Russell King. + * + * 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. + */ +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +#include "dw-hdmi-cec.h" + +enum { + HDMI_IH_CEC_STAT0 = 0x0106, + HDMI_IH_MUTE_CEC_STAT0 = 0x0186, + + HDMI_CEC_CTRL = 0x7d00, + CEC_CTRL_START = BIT(0), + CEC_CTRL_FRAME_TYP = 3 << 1, + CEC_CTRL_RETRY = 0 << 1, + CEC_CTRL_NORMAL = 1 << 1, + CEC_CTRL_IMMED = 2 << 1, + + HDMI_CEC_STAT = 0x7d01, + CEC_STAT_DONE = BIT(0), + CEC_STAT_EOM= BIT(1), + CEC_STAT_NACK = BIT(2), + CEC_STAT_ARBLOST= BIT(3), + CEC_STAT_ERROR_INIT = BIT(4), + CEC_STAT_ERROR_FOLL = BIT(5), + CEC_STAT_WAKEUP = BIT(6), + + HDMI_CEC_MASK = 0x7d02, + HDMI_CEC_POLARITY = 0x7d03, + HDMI_CEC_INT= 0x7d04, + HDMI_CEC_ADDR_L = 0x7d05, + HDMI_CEC_ADDR_H = 0x7d06, + HDMI_CEC_TX_CNT = 0x7d07, + HDMI_CEC_RX_CNT = 0x7d08, + HDMI_CEC_TX_DATA0 = 0x7d10, + HDMI_CEC_RX_DATA0 = 0x7d20, + HDMI_CEC_LOCK = 0x7d30, + HDMI_CEC_WKUPCTRL = 0x7d31, +}; + +struct dw_hdmi_cec { + struct dw_hdmi *hdmi; + const struct dw_hdmi_cec_ops *ops; + u32 addresses; + struct cec_adapter *adap; + struct cec_msg rx_msg; + unsigned int tx_status; + bool tx_done; + bool rx_done; + struct cec_notifier *notify; + int irq; +}; + +static void dw_hdmi_write(struct dw_hdmi_cec *cec, u8 val, int offset) +{ + cec->ops->write(cec->hdmi, val, offset); +} + +static u8 dw_hdmi_read(struct dw_hdmi_cec *cec, int offset) +{ + return cec->ops->read(cec->hdmi, offset); +} + +static int dw_hdmi_cec_log_addr(struct cec_adapter *adap, u8 logical_addr) +{ + struct dw_hdmi_cec *cec = cec_get_drvdata(adap); + + if (logical_addr == CEC_LOG_ADDR_INVALID) + cec->addresses = 0; + else + cec->addresses |=
Re: [PATCHv3 07/22] staging: android: ion: Remove page faulting support
Hi, On 04/04/2017 12:27 AM, Laura Abbott wrote: The new method of syncing with dma_map means that the page faulting sync implementation is no longer applicable. Remove it. Signed-off-by: Laura Abbott--- drivers/staging/android/ion/ion.c | 117 -- 1 file changed, 117 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 6aac935..226ea1f 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -42,37 +42,11 @@ #include "ion_priv.h" #include "compat_ion.h" -bool ion_buffer_fault_user_mappings(struct ion_buffer *buffer) -{ - return (buffer->flags & ION_FLAG_CACHED) && - !(buffer->flags & ION_FLAG_CACHED_NEEDS_SYNC); -} - bool ion_buffer_cached(struct ion_buffer *buffer) { return !!(buffer->flags & ION_FLAG_CACHED); } -static inline struct page *ion_buffer_page(struct page *page) -{ - return (struct page *)((unsigned long)page & ~(1UL)); -} - -static inline bool ion_buffer_page_is_dirty(struct page *page) -{ - return !!((unsigned long)page & 1UL); -} - -static inline void ion_buffer_page_dirty(struct page **page) -{ - *page = (struct page *)((unsigned long)(*page) | 1UL); -} - -static inline void ion_buffer_page_clean(struct page **page) -{ - *page = (struct page *)((unsigned long)(*page) & ~(1UL)); -} - /* this function should only be called while dev->lock is held */ static void ion_buffer_add(struct ion_device *dev, struct ion_buffer *buffer) @@ -140,25 +114,6 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap *heap, buffer->dev = dev; buffer->size = len; - if (ion_buffer_fault_user_mappings(buffer)) { - int num_pages = PAGE_ALIGN(buffer->size) / PAGE_SIZE; - struct scatterlist *sg; - int i, j, k = 0; - - buffer->pages = vmalloc(sizeof(struct page *) * num_pages); We should also remove the vfree(buffer->pages) call in ion_buffer_destroy. In fact, we could removes 'pages' member from ion_buffer altogether. Thanks, Archit - if (!buffer->pages) { - ret = -ENOMEM; - goto err1; - } - - for_each_sg(table->sgl, sg, table->nents, i) { - struct page *page = sg_page(sg); - - for (j = 0; j < sg->length / PAGE_SIZE; j++) - buffer->pages[k++] = page++; - } - } - buffer->dev = dev; buffer->size = len; INIT_LIST_HEAD(>vmas); @@ -924,69 +879,6 @@ void ion_pages_sync_for_device(struct device *dev, struct page *page, dma_sync_sg_for_device(dev, , 1, dir); } -struct ion_vma_list { - struct list_head list; - struct vm_area_struct *vma; -}; - -static int ion_vm_fault(struct vm_fault *vmf) -{ - struct ion_buffer *buffer = vmf->vma->vm_private_data; - unsigned long pfn; - int ret; - - mutex_lock(>lock); - ion_buffer_page_dirty(buffer->pages + vmf->pgoff); - BUG_ON(!buffer->pages || !buffer->pages[vmf->pgoff]); - - pfn = page_to_pfn(ion_buffer_page(buffer->pages[vmf->pgoff])); - ret = vm_insert_pfn(vmf->vma, vmf->address, pfn); - mutex_unlock(>lock); - if (ret) - return VM_FAULT_ERROR; - - return VM_FAULT_NOPAGE; -} - -static void ion_vm_open(struct vm_area_struct *vma) -{ - struct ion_buffer *buffer = vma->vm_private_data; - struct ion_vma_list *vma_list; - - vma_list = kmalloc(sizeof(*vma_list), GFP_KERNEL); - if (!vma_list) - return; - vma_list->vma = vma; - mutex_lock(>lock); - list_add(_list->list, >vmas); - mutex_unlock(>lock); - pr_debug("%s: adding %p\n", __func__, vma); -} - -static void ion_vm_close(struct vm_area_struct *vma) -{ - struct ion_buffer *buffer = vma->vm_private_data; - struct ion_vma_list *vma_list, *tmp; - - pr_debug("%s\n", __func__); - mutex_lock(>lock); - list_for_each_entry_safe(vma_list, tmp, >vmas, list) { - if (vma_list->vma != vma) - continue; - list_del(_list->list); - kfree(vma_list); - pr_debug("%s: deleting %p\n", __func__, vma); - break; - } - mutex_unlock(>lock); -} - -static const struct vm_operations_struct ion_vma_ops = { - .open = ion_vm_open, - .close = ion_vm_close, - .fault = ion_vm_fault, -}; - static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) { struct ion_buffer *buffer = dmabuf->priv; @@ -998,15 +890,6 @@ static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) return -EINVAL; } - if (ion_buffer_fault_user_mappings(buffer)) { - vma->vm_flags |=
Re: [PATCHv3 08/22] staging: android: ion: Remove crufty cache support
Hi, On 04/04/2017 12:27 AM, Laura Abbott wrote: Now that we call dma_map in the dma_buf API callbacks there is no need to use the existing cache APIs. Remove the sync ioctl and the existing bad dma_sync calls. Explicit caching can be handled with the dma_buf sync API. We could get rid of the ION_FLAG_CACHED_NEEDS_SYNC flag in this patch too. Thanks, Archit Signed-off-by: Laura Abbott--- drivers/staging/android/ion/compat_ion.c| 1 - drivers/staging/android/ion/ion-ioctl.c | 6 drivers/staging/android/ion/ion.c | 40 - drivers/staging/android/ion/ion_carveout_heap.c | 6 drivers/staging/android/ion/ion_chunk_heap.c| 6 drivers/staging/android/ion/ion_page_pool.c | 3 -- drivers/staging/android/ion/ion_priv.h | 13 drivers/staging/android/ion/ion_system_heap.c | 5 drivers/staging/android/uapi/ion.h | 10 --- 9 files changed, 90 deletions(-) diff --git a/drivers/staging/android/ion/compat_ion.c b/drivers/staging/android/ion/compat_ion.c index 9a978d2..b892d3a 100644 --- a/drivers/staging/android/ion/compat_ion.c +++ b/drivers/staging/android/ion/compat_ion.c @@ -186,7 +186,6 @@ long compat_ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) case ION_IOC_SHARE: case ION_IOC_MAP: case ION_IOC_IMPORT: - case ION_IOC_SYNC: return filp->f_op->unlocked_ioctl(filp, cmd, (unsigned long)compat_ptr(arg)); default: diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c index 5b2e93f..e096bcd 100644 --- a/drivers/staging/android/ion/ion-ioctl.c +++ b/drivers/staging/android/ion/ion-ioctl.c @@ -51,7 +51,6 @@ static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg) static unsigned int ion_ioctl_dir(unsigned int cmd) { switch (cmd) { - case ION_IOC_SYNC: case ION_IOC_FREE: case ION_IOC_CUSTOM: return _IOC_WRITE; @@ -146,11 +145,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) data.handle.handle = handle->id; break; } - case ION_IOC_SYNC: - { - ret = ion_sync_for_device(client, data.fd.fd); - break; - } case ION_IOC_CUSTOM: { if (!dev->custom_ioctl) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 226ea1f..8757164 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -863,22 +863,6 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment, dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction); } -void ion_pages_sync_for_device(struct device *dev, struct page *page, - size_t size, enum dma_data_direction dir) -{ - struct scatterlist sg; - - sg_init_table(, 1); - sg_set_page(, page, size, 0); - /* -* This is not correct - sg_dma_address needs a dma_addr_t that is valid -* for the targeted device, but this works on the currently targeted -* hardware. -*/ - sg_dma_address() = page_to_phys(page); - dma_sync_sg_for_device(dev, , 1, dir); -} - static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) { struct ion_buffer *buffer = dmabuf->priv; @@ -1097,30 +1081,6 @@ struct ion_handle *ion_import_dma_buf_fd(struct ion_client *client, int fd) } EXPORT_SYMBOL(ion_import_dma_buf_fd); -int ion_sync_for_device(struct ion_client *client, int fd) -{ - struct dma_buf *dmabuf; - struct ion_buffer *buffer; - - dmabuf = dma_buf_get(fd); - if (IS_ERR(dmabuf)) - return PTR_ERR(dmabuf); - - /* if this memory came from ion */ - if (dmabuf->ops != _buf_ops) { - pr_err("%s: can not sync dmabuf from another exporter\n", - __func__); - dma_buf_put(dmabuf); - return -EINVAL; - } - buffer = dmabuf->priv; - - dma_sync_sg_for_device(NULL, buffer->sg_table->sgl, - buffer->sg_table->nents, DMA_BIDIRECTIONAL); - dma_buf_put(dmabuf); - return 0; -} - int ion_query_heaps(struct ion_client *client, struct ion_heap_query *query) { struct ion_device *dev = client->dev; diff --git a/drivers/staging/android/ion/ion_carveout_heap.c b/drivers/staging/android/ion/ion_carveout_heap.c index 9bf8e98..e0e360f 100644 --- a/drivers/staging/android/ion/ion_carveout_heap.c +++ b/drivers/staging/android/ion/ion_carveout_heap.c @@ -100,10 +100,6 @@ static void ion_carveout_heap_free(struct ion_buffer *buffer) ion_heap_buffer_zero(buffer); - if (ion_buffer_cached(buffer)) - dma_sync_sg_for_device(NULL,
Re: [PATCH v5.1 0/6] drm: bridge: dw-hdmi: Add support for Custom PHYs
Hi, On 03/31/2017 07:55 PM, Neil Armstrong wrote: The Amlogic GX SoCs implements a Synopsys DesignWare HDMI TX Controller in combination with a very custom PHY. Thanks to Laurent Pinchart's changes, the HW report the following : Detected HDMI TX controller v2.01a with HDCP (meson_dw_hdmi_phy) The following differs from common PHY integration as managed in the current driver : - Amlogic PHY is not configured through the internal I2C link - Amlogic PHY do not use the ENTMDS, SVSRET, PDDQ, ... signals from the controller - Amlogic PHY do not export HPD ands RxSense signals to the controller And finally, concerning the controller integration : - the Controller registers are not flat memory-mapped, and uses an addr+read/write register pair to write all registers. - Inputs only YUV444 pixel data Most of these uses case are implemented in Laurent Pinchart v5.1 patchset merged in drm-misc-next branch. This is why the following patchset implements : - Configure the Input format from the plat_data - Add PHY callback to handle HPD and RxSense out of the dw-hdmi driver To implement the input format handling, the Synopsys HDMIT TX Controller input V4L bus formats are used and missing formats + documentation are added. This patchset makes the Amlogic GX SoCs HDMI output successfully work, and is also tested on the RK3288 ACT8846 EVB Board. Please feel free to add my Reviewed-by for all the patches. Did we get an Ack from the v4l maintainers to take the media changes via the drm-misc branch? If so, I guess we could go ahead and push the series to drm-misc-next. Thanks, Archit Changes since v5 at [6] : - Small addition in V4L YUV bus formats documentation Changes since v4 at [5] : - Rebased on drm-misc-next at bd283d2f66c2 - Fix 4:2:0 bus formats naming - Renamed function fd_registered to i2c_init in dw-hdmi.c Changes since v3 at [4] : - Fix 4:2:0 bus formats naming - Add separate 36bit and 48bit tables for bus formats documentation - Added 4:2:0 bus config in hdmi_video_sample - Moved dw_hdmi documentation in a "bridge" subdir - Rebase on drm-misc-next at 62c58af32c93 Changes since v2 at [3] : - Rebase on laurent patch "Extract PHY interrupt setup to a function" - Reduce phy operations - Switch the V4L bus formats and encodings instead of custom enum Changes since v1 at [2] : - Drop patches submitted by laurent Changes since RFC at [1] : - Regmap fixup for 4bytes register access, tested on RK3288 SoC - Move phy callbacks to phy_ops and move Synopsys PHY calls into default ops - Move HDMI link data into shared header - Move Pixel Encoding enum to shared header [1] http://lkml.kernel.org/r/1484656294-6140-1-git-send-email-narmstr...@baylibre.com [2] http://lkml.kernel.org/r/1485774318-21916-1-git-send-email-narmstr...@baylibre.com [3] http://lkml.kernel.org/r/1488468572-31971-1-git-send-email-narmstr...@baylibre.com [4] http://lkml.kernel.org/r/1488904944-14285-1-git-send-email-narmstr...@baylibre.com [5] http://lkml.kernel.org/r/1490109161-20529-1-git-send-email-narmstr...@baylibre.com [6] http://lkml.kernel.org/r/1490864675-17336-1-git-send-email-narmstr...@baylibre.com Laurent Pinchart (1): drm: bridge: dw-hdmi: Extract PHY interrupt setup to a function Neil Armstrong (5): media: uapi: Add RGB and YUV bus formats for Synopsys HDMI TX Controller documentation: media: Add documentation for new RGB and YUV bus formats drm: bridge: dw-hdmi: Switch to V4L bus format and encodings drm: bridge: dw-hdmi: Add Documentation on supported input formats drm: bridge: dw-hdmi: Move HPD handling to PHY operations Documentation/gpu/bridge/dw-hdmi.rst| 15 + Documentation/gpu/index.rst | 1 + Documentation/media/uapi/v4l/subdev-formats.rst | 874 +++- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 470 - include/drm/bridge/dw_hdmi.h| 68 ++ include/uapi/linux/media-bus-format.h | 13 +- 6 files changed, 1268 insertions(+), 173 deletions(-) create mode 100644 Documentation/gpu/bridge/dw-hdmi.rst -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v3 3/6] documentation: media: Add documentation for new RGB and YUV bus formats
On 3/7/2017 10:12 PM, Neil Armstrong wrote: Add documentation for added Bus Formats to describe RGB and YUS formats used s/YUS/YUV as input to the Synopsys DesignWare HDMI TX Controller. Signed-off-by: Neil Armstrong--- Documentation/media/uapi/v4l/subdev-formats.rst | 4992 ++- 1 file changed, 3963 insertions(+), 1029 deletions(-) Do we know if there is a better way to add more columns without adding so many lines? If not, one option could be to create a separate tables for 48 bit RGB formats, 48 bit YUV formats etc. Thanks, Archit -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH v3 2/6] media: uapi: Add RGB and YUV bus formats for Synopsys HDMI TX Controller
On 3/7/2017 10:12 PM, Neil Armstrong wrote: In order to describe the RGB and YUB bus formats used to feed the s/YUB/YUV Synopsys DesignWare HDMI TX Controller, add missing formats to the list of Bus Formats. Documentation for these formats is added in a separate patch. Reviewed-by: Archit Taneja <arch...@codeaurora.org> Signed-off-by: Neil Armstrong <narmstr...@baylibre.com> --- include/uapi/linux/media-bus-format.h | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h index 2168759..7cc820b 100644 --- a/include/uapi/linux/media-bus-format.h +++ b/include/uapi/linux/media-bus-format.h @@ -33,7 +33,7 @@ #define MEDIA_BUS_FMT_FIXED0x0001 -/* RGB - next is 0x1018 */ +/* RGB - next is 0x101b */ #define MEDIA_BUS_FMT_RGB444_1X12 0x1016 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE 0x1001 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE 0x1002 @@ -57,8 +57,11 @@ #define MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA 0x1012 #define MEDIA_BUS_FMT_ARGB_1X320x100d #define MEDIA_BUS_FMT_RGB888_1X32_PADHI0x100f +#define MEDIA_BUS_FMT_RGB101010_1X30 0x1018 +#define MEDIA_BUS_FMT_RGB121212_1X36 0x1019 +#define MEDIA_BUS_FMT_RGB161616_1X48 0x101a -/* YUV (including grey) - next is 0x2026 */ +/* YUV (including grey) - next is 0x202c */ #define MEDIA_BUS_FMT_Y8_1X8 0x2001 #define MEDIA_BUS_FMT_UV8_1X8 0x2015 #define MEDIA_BUS_FMT_UYVY8_1_5X8 0x2002 @@ -90,12 +93,18 @@ #define MEDIA_BUS_FMT_YVYU10_1X20 0x200e #define MEDIA_BUS_FMT_VUY8_1X240x2024 #define MEDIA_BUS_FMT_YUV8_1X240x2025 +#define MEDIA_BUS_FMT_UYVY8_1_1X24 0x2026 #define MEDIA_BUS_FMT_UYVY12_1X24 0x2020 #define MEDIA_BUS_FMT_VYUY12_1X24 0x2021 #define MEDIA_BUS_FMT_YUYV12_1X24 0x2022 #define MEDIA_BUS_FMT_YVYU12_1X24 0x2023 #define MEDIA_BUS_FMT_YUV10_1X30 0x2016 +#define MEDIA_BUS_FMT_UYVY10_1_1X300x2027 #define MEDIA_BUS_FMT_AYUV8_1X32 0x2017 +#define MEDIA_BUS_FMT_UYVY12_1_1X360x2028 +#define MEDIA_BUS_FMT_YUV12_1X36 0x2029 +#define MEDIA_BUS_FMT_YUV16_1X48 0x202a +#define MEDIA_BUS_FMT_UYVY16_1_1X480x202b /* Bayer - next is 0x3021 */ #define MEDIA_BUS_FMT_SBGGR8_1X8 0x3001 -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH v3 5/6] drm: bridge: dw-hdmi: Add Documentation on supported input formats
On 3/7/2017 10:12 PM, Neil Armstrong wrote: This patch adds a new DRM documentation entry and links to the input format table added in the dw_hdmi header. Signed-off-by: Neil Armstrong--- Documentation/gpu/dw-hdmi.rst | 15 +++ Documentation/gpu/index.rst | 1 + Maybe we create a sub-directory for bridges here? Maybe a hierarchy similar to tinydrm's? Looks good otherwise. Archit 2 files changed, 16 insertions(+) create mode 100644 Documentation/gpu/dw-hdmi.rst diff --git a/Documentation/gpu/dw-hdmi.rst b/Documentation/gpu/dw-hdmi.rst new file mode 100644 index 000..486faad --- /dev/null +++ b/Documentation/gpu/dw-hdmi.rst @@ -0,0 +1,15 @@ +=== + drm/bridge/dw-hdmi Synopsys DesignWare HDMI Controller +=== + +Synopsys DesignWare HDMI Controller +=== + +This section covers everything related to the Synopsys DesignWare HDMI +Controller implemented as a DRM bridge. + +Supported Input Formats and Encodings +- + +.. kernel-doc:: include/drm/bridge/dw_hdmi.h + :doc: Supported input formats and encodings diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst index e998ee0..0725449 100644 --- a/Documentation/gpu/index.rst +++ b/Documentation/gpu/index.rst @@ -10,6 +10,7 @@ Linux GPU Driver Developer's Guide drm-kms drm-kms-helpers drm-uapi + dw-hdmi i915 tinydrm vc4 -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
Re: [RFC PATCH 00/11] Introduce writeback connectors
Hi Brian, On 10/11/2016 08:23 PM, Brian Starkey wrote: Hi, This RFC series introduces a new connector type: DRM_MODE_CONNECTOR_WRITEBACK It is a follow-on from a previous discussion: [1] Writeback connectors are used to expose the memory writeback engines found in some display controllers, which can write a CRTC's composition result to a memory buffer. This is useful e.g. for testing, screen-recording, screenshots, wireless display, display cloning, memory-to-memory composition. Patches 1-7 include the core framework changes required, and patches 8-11 implement a writeback connector for the Mali-DP writeback engine. The Mali-DP patches depend on this other series: [2]. The connector is given the FB_ID property for the output framebuffer, and two new read-only properties: PIXEL_FORMATS and PIXEL_FORMATS_SIZE, which expose the supported framebuffer pixel formats of the engine. The EDID property is not exposed for writeback connectors. Writeback connector usage: -- Due to connector routing changes being treated as "full modeset" operations, any client which wishes to use a writeback connector should include the connector in every modeset. The writeback will not actually become active until a framebuffer is attached. The writeback itself is enabled by attaching a framebuffer to the FB_ID property of the connector. The driver must then ensure that the CRTC content of that atomic commit is written into the framebuffer. The writeback works in a one-shot mode with each atomic commit. This prevents the same content from being written multiple times. In some cases (front-buffer rendering) there might be a desire for continuous operation - I think a property could be added later for this kind of control. Writeback can be disabled by setting FB_ID to zero. Known issues: - * I'm not sure what "DPMS" should mean for writeback connectors. It could be used to disable writeback (even when a framebuffer is attached), or it could be hidden entirely (which would break the legacy DPMS call for writeback connectors). * With Daniel's recent re-iteration of the userspace API rules, I fully expect to provide some userspace code to support this. The question is what, and where? We want to use writeback for testing, so perhaps some tests in igt is suitable. * Documentation. Probably some portion of this cover letter needs to make it into Documentation/ * Synchronisation. Our hardware will finish the writeback by the next vsync. I've not implemented fence support here, but it would be an obvious addition. See Also: - [1] https://lists.freedesktop.org/archives/dri-devel/2016-July/113197.html [2] https://lists.freedesktop.org/archives/dri-devel/2016-October/120486.html I welcome any comments, especially if this approach does/doesn't fit well with anyone else's hardware. Thanks for working on this! Some points below. - Writeback hardware generally allows us to specify the region within the framebuffer we want to write to. It's analogous to the SRC_X/Y/W/H plane properties. We could have similar props for the writeback connectors, and maybe set them to the FB_ID dimensions if they aren't configured by userspace. - Besides the above property, writeback hardware can have provisions for scaling, color space conversion and rotation. This would mean that we'd eventually add more writeback specific props/params in drm_connector/drm_connector_state. Would we be okay adding more such props for connectors? Thanks, Archit Thanks, -Brian --- Brian Starkey (10): drm: add writeback connector type drm/fb-helper: skip writeback connectors drm: extract CRTC/plane disable from drm_framebuffer_remove drm: add __drm_framebuffer_remove_atomic drm: add fb to connector state drm: expose fb_id property for writeback connectors drm: add writeback-connector pixel format properties drm: mali-dp: rename malidp_input_format drm: mali-dp: add RGB writeback formats for DP550/DP650 drm: mali-dp: add writeback connector Liviu Dudau (1): drm: mali-dp: Add support for writeback on DP550/DP650 drivers/gpu/drm/arm/Makefile|1 + drivers/gpu/drm/arm/malidp_crtc.c | 10 ++ drivers/gpu/drm/arm/malidp_drv.c| 25 +++- drivers/gpu/drm/arm/malidp_drv.h|5 + drivers/gpu/drm/arm/malidp_hw.c | 104 ++ drivers/gpu/drm/arm/malidp_hw.h | 27 +++- drivers/gpu/drm/arm/malidp_mw.c | 268 +++ drivers/gpu/drm/arm/malidp_planes.c |8 +- drivers/gpu/drm/arm/malidp_regs.h | 15 ++ drivers/gpu/drm/drm_atomic.c| 40 ++ drivers/gpu/drm/drm_atomic_helper.c |4 + drivers/gpu/drm/drm_connector.c | 79 ++- drivers/gpu/drm/drm_crtc.c | 14 +- drivers/gpu/drm/drm_fb_helper.c |4 + drivers/gpu/drm/drm_framebuffer.c | 249 drivers/gpu/drm/drm_ioctl.c |7 + include/drm/drmP.h
Re: [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE
On Friday 11 April 2014 01:41 PM, Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com This is not allowed by the spec and does in fact not make any sense. Return -EINVAL if this is the case. Signed-off-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Pawel Osciak pa...@osciak.com Acked-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/v4l2-core/videobuf2-core.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 6e05495..f8c0247 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -1511,6 +1511,19 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b) dprintk(1, plane parameters verification failed: %d\n, ret); return ret; } + if (b-field == V4L2_FIELD_ALTERNATE V4L2_TYPE_IS_OUTPUT(q-type)) { + /* +* If the format's field is ALTERNATE, then the buffer's field +* should be either TOP or BOTTOM, not ALTERNATE since that +* makes no sense. The driver has to know whether the +* buffer represents a top or a bottom field in order to +* program any DMA correctly. Using ALTERNATE is wrong, since +* that just says that it is either a top or a bottom field, +* but not which of the two it is. +*/ + dprintk(1, the field is incorrectly set to ALTERNATE for an output buffer\n); + return -EINVAL; + } If vb2_queue had a field parameter, which tells the format's field type. We could have returned an error if the field format was ALTERNATE, and the buffer field was not TOP or BOTTOM. I don't know whether having a field parameter in vb2_queue is a good idea or not. Thanks, Archit -- 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: [REVIEWv3 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE
On Friday 11 April 2014 02:28 PM, Hans Verkuil wrote: On 04/11/2014 10:42 AM, Archit Taneja wrote: On Friday 11 April 2014 01:41 PM, Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com This is not allowed by the spec and does in fact not make any sense. Return -EINVAL if this is the case. Signed-off-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Pawel Osciak pa...@osciak.com Acked-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/v4l2-core/videobuf2-core.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 6e05495..f8c0247 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -1511,6 +1511,19 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b) dprintk(1, plane parameters verification failed: %d\n, ret); return ret; } + if (b-field == V4L2_FIELD_ALTERNATE V4L2_TYPE_IS_OUTPUT(q-type)) { + /* +* If the format's field is ALTERNATE, then the buffer's field +* should be either TOP or BOTTOM, not ALTERNATE since that +* makes no sense. The driver has to know whether the +* buffer represents a top or a bottom field in order to +* program any DMA correctly. Using ALTERNATE is wrong, since +* that just says that it is either a top or a bottom field, +* but not which of the two it is. +*/ + dprintk(1, the field is incorrectly set to ALTERNATE for an output buffer\n); + return -EINVAL; + } If vb2_queue had a field parameter, which tells the format's field type. We could have returned an error if the field format was ALTERNATE, and the buffer field was not TOP or BOTTOM. I don't know whether having a field parameter in vb2_queue is a good idea or not. The predecessor of vb2, videobuf, had that actually. I am not sure myself if it is a good idea or not to do the same for vb2. For now I think we should leave it as is. There are very few drivers that support FIELD_ALTERNATE although this should become more common for drivers supporting interlaced HDTV formats. When we see more drivers that support this, then we can see if it makes sense to move part of the handling to vb2. Sure, queue_setup vb op might be a good place to populate it. But as you said, there aren't many drivers that use FIELD_ALTERNATE, and there doesn't seem to be any other advantage for having 'field' in vb2_queue. So, it can be left for later. Archit -- 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
[GIT PULL for 3.15 fixes] VPE fixes
Hi Kamil, Since the VPE m2m patch set couldn't make it on time, I've separated out the fixes from the series so that they can be taken in one of the 3.15-rc series. Thanks, Archit The following changes since commit a83b93a7480441a47856dc9104bea970e84cda87: [media] em28xx-dvb: fix PCTV 461e tuner I2C binding (2014-03-31 08:02:16 -0300) are available in the git repository at: git://github.com/boddob/linux.git vpe-fixes-315-rc for you to fetch changes up to 3c7d629f0aa98ed587306913831e5a8968504f7a: v4l: ti-vpe: retain v4l2_buffer flags for captured buffers (2014-04-07 12:56:47 +0530) Archit Taneja (9): v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs v4l: ti-vpe: Use video_device_release_empty v4l: ti-vpe: Allow usage of smaller images v4l: ti-vpe: report correct capabilities in querycap v4l: ti-vpe: Use correct bus_info name for the device in querycap v4l: ti-vpe: Fix initial configuration queue data v4l: ti-vpe: zero out reserved fields in try_fmt v4l: ti-vpe: Set correct field parameter for output and capture buffers v4l: ti-vpe: retain v4l2_buffer flags for captured buffers drivers/media/platform/ti-vpe/vpe.c | 45 ++--- 1 file changed, 32 insertions(+), 13 deletions(-) -- 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] v4l: ti-vpe: fix devm_ioremap_resource() return value checking
Hi, On Tuesday 18 March 2014 04:11 PM, Bartlomiej Zolnierkiewicz wrote: devm_ioremap_resource() returns a pointer to the remapped memory or an ERR_PTR() encoded error code on failure. Fix the checks inside csc_create() and sc_create() accordingly. Cc: Archit Taneja arc...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com --- Compile tested only. Thanks for the patch. It looks like this wasn't patch created using git-format-patch. I can convert it if needed. Tested-by: Archit Tanejaarc...@ti.com Thanks, Archit drivers/media/platform/ti-vpe/csc.c |4 ++-- drivers/media/platform/ti-vpe/sc.c |4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) Index: b/drivers/media/platform/ti-vpe/csc.c === --- a/drivers/media/platform/ti-vpe/csc.c 2014-03-14 16:45:25.848724010 +0100 +++ b/drivers/media/platform/ti-vpe/csc.c 2014-03-18 11:01:36.595182833 +0100 @@ -187,9 +187,9 @@ struct csc_data *csc_create(struct platf } csc-base = devm_ioremap_resource(pdev-dev, csc-res); - if (!csc-base) { + if (IS_ERR(csc-base)) { dev_err(pdev-dev, failed to ioremap\n); - return ERR_PTR(-ENOMEM); + return csc-base; } return csc; Index: b/drivers/media/platform/ti-vpe/sc.c === --- a/drivers/media/platform/ti-vpe/sc.c2014-03-14 16:45:25.848724010 +0100 +++ b/drivers/media/platform/ti-vpe/sc.c2014-03-18 11:02:09.555182273 +0100 @@ -302,9 +302,9 @@ struct sc_data *sc_create(struct platfor } sc-base = devm_ioremap_resource(pdev-dev, sc-res); - if (!sc-base) { + if (IS_ERR(sc-base)) { dev_err(pdev-dev, failed to ioremap\n); - return ERR_PTR(-ENOMEM); + return sc-base; } return sc; -- 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 08/14] v4l: ti-vpe: Rename csc memory resource name
Hi Kamil, On Thursday 13 March 2014 08:14 PM, Kamil Debski wrote: Hi, From: Archit Taneja [mailto:arc...@ti.com] Sent: Thursday, March 13, 2014 12:44 PM Rename the memory block resource vpe_csc to csc since it also exists within the VIP IP block. This would make the name more generic, and both VPE and VIP DT nodes in the future can use it. I understand that this is not yet used in any public dts files. Right? Best wishes, Yes, a VPE DT node doesn't exist in any public dts files yet. So it's safe to change the name. It should eventually come in dra7.dtsi. There is a dependency on a crossbar IP module, which provides us with an IRQ line for VPE going to the GIC. Once that is merged, I can add the VPE DT node. Thanks, Archit -- 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 02/14] v4l: ti-vpe: register video device only when firmware is loaded
Hi, On Thursday 13 March 2014 07:59 PM, Kamil Debski wrote: Hi, From: Archit Taneja [mailto:arc...@ti.com] Sent: Thursday, March 13, 2014 1:09 PM Hi Kamil, On Thursday 13 March 2014 05:18 PM, Kamil Debski wrote: Hi Archit, From: Archit Taneja [mailto:arc...@ti.com] Sent: Tuesday, March 11, 2014 9:34 AM vpe fops(vpe_open in particular) should be called only when VPDMA firmware is loaded. File operations on the video device are possible the moment it is registered. Currently, we register the video device for VPE at driver probe, after calling a vpdma helper to initialize VPDMA and load firmware. This function is non-blocking(it calls request_firmware_nowait()), and doesn't ensure that the firmware is actually loaded when it returns. We remove the device registration from vpe probe, and move it to a callback provided by the vpe driver to the vpdma library, through vpdma_create(). The ready field in vpdma_data is no longer needed since we always have firmware loaded before the device is registered. A minor problem with this approach is that if the video_register_device fails(which doesn't really happen), the vpe platform device would be registered. however, there won't be any v4l2 device corresponding to it. Could you explain to me one thing. request_firmware cannot be used in probe, thus you are using request_firmware_nowait. Why cannot the firmware be loaded on open with a regular request_firmware that is waiting? I totally agree with you here. Placing the firmware in open() would probably make more sense. The reason I didn't place it in open() is because I didn't want to release firmware in a corresponding close(), and be in a situation where the firmware is loaded multiple times in the driver's lifetime. Would it be possible to load firmware in open and release it in remove? I know that this would disturb the symmetry between open-release and probe-remove. But this could work. That might work. Currently, the driver doesn't do any clock management, it just enables the clocks in probe, and disables them in remove. I need to check how the firmware is dependent on clocks. We could make a better decision about where to release the firmware with that information. Archit -- 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 00/14] v4l: ti-vpe: Some VPE fixes and enhancements
This patch set mainly consists of minor fixes for the VPE driver. These fixes ensure the following: - The VPE module can be inserted and removed successively. - Make sure that smaller resolutions like qcif work correctly. - Prevent race condition between firmware loading and an open call to the v4l2 device. - Prevent the possibility of output m2m queue not having sufficient 'ready' buffers. - Some VPDMA data descriptor fields weren't understood correctly before. They are now used correctly. The rest of the patches add some minor features like DMA buf support and cropping/composing. Reference branch: g...@github.com:boddob/linux.git vpe_for_315 Changes in v4: - More legible code for selection API - Stricter checking for target type vs crop type in g_selection - Minor fix in patch 13/14, there was a logical bug in buf_prepare when checking validity of top and bottom fields. Changes in v3: - improvements in selection API patch. - querycap fixes for v4l2 compliance. - v4l2_buffer 'field' and flags' fixes for compliance. - fixes in try_fmt vpe_open for compliance. - rename a IOMEM resource for better DT compatibility. Changes in v2: - selection API used instead of older cropping API. - Typo fix. - Some changes in patch 6/7 to support composing on the capture side of VPE. Archit Taneja (14): v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs v4l: ti-vpe: register video device only when firmware is loaded v4l: ti-vpe: Use video_device_release_empty v4l: ti-vpe: Allow DMABUF buffer type support v4l: ti-vpe: Allow usage of smaller images v4l: ti-vpe: Fix some params in VPE data descriptors v4l: ti-vpe: Add selection API in VPE driver v4l: ti-vpe: Rename csc memory resource name v4l: ti-vpe: report correct capabilities in querycap v4l: ti-vpe: Use correct bus_info name for the device in querycap v4l: ti-vpe: Fix initial configuration queue data v4l: ti-vpe: zero out reserved fields in try_fmt v4l: ti-vpe: Set correct field parameter for output and capture buffers v4l: ti-vpe: retain v4l2_buffer flags for captured buffers drivers/media/platform/ti-vpe/csc.c | 2 +- drivers/media/platform/ti-vpe/vpdma.c | 68 ++--- drivers/media/platform/ti-vpe/vpdma.h | 17 ++- drivers/media/platform/ti-vpe/vpe.c | 272 -- 4 files changed, 290 insertions(+), 69 deletions(-) -- 1.8.3.2 -- 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 02/14] v4l: ti-vpe: register video device only when firmware is loaded
vpe fops(vpe_open in particular) should be called only when VPDMA firmware is loaded. File operations on the video device are possible the moment it is registered. Currently, we register the video device for VPE at driver probe, after calling a vpdma helper to initialize VPDMA and load firmware. This function is non-blocking(it calls request_firmware_nowait()), and doesn't ensure that the firmware is actually loaded when it returns. We remove the device registration from vpe probe, and move it to a callback provided by the vpe driver to the vpdma library, through vpdma_create(). The ready field in vpdma_data is no longer needed since we always have firmware loaded before the device is registered. A minor problem with this approach is that if the video_register_device fails(which doesn't really happen), the vpe platform device would be registered. however, there won't be any v4l2 device corresponding to it. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 8 +++-- drivers/media/platform/ti-vpe/vpdma.h | 7 +++-- drivers/media/platform/ti-vpe/vpe.c | 55 --- 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index e8175e7..73dd38e 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -781,7 +781,7 @@ static void vpdma_firmware_cb(const struct firmware *f, void *context) /* already initialized */ if (read_field_reg(vpdma, VPDMA_LIST_ATTR, VPDMA_LIST_RDY_MASK, VPDMA_LIST_RDY_SHFT)) { - vpdma-ready = true; + vpdma-cb(vpdma-pdev); return; } @@ -811,7 +811,7 @@ static void vpdma_firmware_cb(const struct firmware *f, void *context) goto free_buf; } - vpdma-ready = true; + vpdma-cb(vpdma-pdev); free_buf: vpdma_unmap_desc_buf(vpdma, fw_dma_buf); @@ -839,7 +839,8 @@ static int vpdma_load_firmware(struct vpdma_data *vpdma) return 0; } -struct vpdma_data *vpdma_create(struct platform_device *pdev) +struct vpdma_data *vpdma_create(struct platform_device *pdev, + void (*cb)(struct platform_device *pdev)) { struct resource *res; struct vpdma_data *vpdma; @@ -854,6 +855,7 @@ struct vpdma_data *vpdma_create(struct platform_device *pdev) } vpdma-pdev = pdev; + vpdma-cb = cb; res = platform_get_resource_byname(pdev, IORESOURCE_MEM, vpdma); if (res == NULL) { diff --git a/drivers/media/platform/ti-vpe/vpdma.h b/drivers/media/platform/ti-vpe/vpdma.h index cf40f11..bf5f8bb 100644 --- a/drivers/media/platform/ti-vpe/vpdma.h +++ b/drivers/media/platform/ti-vpe/vpdma.h @@ -35,8 +35,8 @@ struct vpdma_data { struct platform_device *pdev; - /* tells whether vpdma firmware is loaded or not */ - bool ready; + /* callback to VPE driver when the firmware is loaded */ + void (*cb)(struct platform_device *pdev); }; enum vpdma_data_format_type { @@ -208,6 +208,7 @@ void vpdma_set_frame_start_event(struct vpdma_data *vpdma, void vpdma_dump_regs(struct vpdma_data *vpdma); /* initialize vpdma, passed with VPE's platform device pointer */ -struct vpdma_data *vpdma_create(struct platform_device *pdev); +struct vpdma_data *vpdma_create(struct platform_device *pdev, + void (*cb)(struct platform_device *pdev)); #endif diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index f3143ac..f1eae67 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1817,11 +1817,6 @@ static int vpe_open(struct file *file) vpe_dbg(dev, vpe_open\n); - if (!dev-vpdma-ready) { - vpe_err(dev, vpdma firmware not loaded\n); - return -ENODEV; - } - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) return -ENOMEM; @@ -2039,10 +2034,40 @@ static void vpe_runtime_put(struct platform_device *pdev) WARN_ON(r 0 r != -ENOSYS); } +static void vpe_fw_cb(struct platform_device *pdev) +{ + struct vpe_dev *dev = platform_get_drvdata(pdev); + struct video_device *vfd; + int ret; + + vfd = dev-vfd; + *vfd = vpe_videodev; + vfd-lock = dev-dev_mutex; + vfd-v4l2_dev = dev-v4l2_dev; + + ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0); + if (ret) { + vpe_err(dev, Failed to register video device\n); + + vpe_set_clock_enable(dev, 0); + vpe_runtime_put(pdev); + pm_runtime_disable(pdev-dev); + v4l2_m2m_release(dev-m2m_dev); + vb2_dma_contig_cleanup_ctx(dev-alloc_ctx); + v4l2_device_unregister(dev-v4l2_dev); + + return; + } + + video_set_drvdata
[PATCH v4 03/14] v4l: ti-vpe: Use video_device_release_empty
The video_device struct is currently embedded in the driver data struct vpe_dev. A vpe_dev instance is allocated by the driver, and the memory for the vfd is a part of this struct. The v4l2 core, however, manages the removal of the vfd region, through the video_device's .release() op, which currently is the helper video_device_release. This causes memory corruption, and leads to issues when we try to re-insert the vpe module. Use the video_device_release_empty helper function instead. Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index f1eae67..0363df6 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -2000,7 +2000,7 @@ static struct video_device vpe_videodev = { .fops = vpe_fops, .ioctl_ops = vpe_ioctl_ops, .minor = -1, - .release= video_device_release, + .release= video_device_release_empty, .vfl_dir= VFL_DIR_M2M, }; -- 1.8.3.2 -- 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 09/14] v4l: ti-vpe: report correct capabilities in querycap
querycap currently returns V4L2_CAP_VIDEO_M2M as a capability, this should be V4L2_CAP_VIDEO_M2M_MPLANE instead, as the driver supports multiplanar formats. Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index f3f1c10..c066eb8 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1347,7 +1347,7 @@ static int vpe_querycap(struct file *file, void *priv, strncpy(cap-driver, VPE_MODULE_NAME, sizeof(cap-driver) - 1); strncpy(cap-card, VPE_MODULE_NAME, sizeof(cap-card) - 1); strlcpy(cap-bus_info, VPE_MODULE_NAME, sizeof(cap-bus_info)); - cap-device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING; + cap-device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; cap-capabilities = cap-device_caps | V4L2_CAP_DEVICE_CAPS; return 0; } -- 1.8.3.2 -- 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 13/14] v4l: ti-vpe: Set correct field parameter for output and capture buffers
The vpe driver wasn't setting the correct field parameter for dequed CAPTURE type buffers for the case where the captured output is progressive. Set the field to V4L2_FIELD_NONE for the completed destination buffers when the captured output is progressive. For OUTPUT type buffers, a queued buffer's field is forced to V4L2_FIELD_NONE if the pixel format(configured through s_fmt for the buffer type V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE specifies) the field type isn't interlaced. If the pixel format specified was V4L2_FIELD_ALTERNATE, and the queued buffer's field isn't V4L2_FIELD_TOP or V4L2_FIELD_BOTTOM, the vb2 buf_prepare op returns an error. This ensures compliance, and that the dequeued output and captured buffers contain the field type that the driver used internally. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index c0ae847..362d5be 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1296,10 +1296,10 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) d_buf-timecode = s_buf-timecode; } d_buf-sequence = ctx-sequence; - d_buf-field = ctx-field; d_q_data = ctx-q_data[Q_DATA_DST]; if (d_q_data-flags Q_DATA_INTERLACED) { + d_buf-field = ctx-field; if (ctx-field == V4L2_FIELD_BOTTOM) { ctx-sequence++; ctx-field = V4L2_FIELD_TOP; @@ -1308,6 +1308,7 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) ctx-field = V4L2_FIELD_BOTTOM; } } else { + d_buf-field = V4L2_FIELD_NONE; ctx-sequence++; } @@ -1880,6 +1881,16 @@ static int vpe_buf_prepare(struct vb2_buffer *vb) q_data = get_q_data(ctx, vb-vb2_queue-type); num_planes = q_data-fmt-coplanar ? 2 : 1; + if (vb-vb2_queue-type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { + if (!(q_data-flags Q_DATA_INTERLACED)) { + vb-v4l2_buf.field = V4L2_FIELD_NONE; + } else { + if (vb-v4l2_buf.field != V4L2_FIELD_TOP + vb-v4l2_buf.field != V4L2_FIELD_BOTTOM) + return -EINVAL; + } + } + for (i = 0; i num_planes; i++) { if (vb2_plane_size(vb, i) q_data-sizeimage[i]) { vpe_err(ctx-dev, -- 1.8.3.2 -- 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 04/14] v4l: ti-vpe: Allow DMABUF buffer type support
For OMAP and DRA7x, we generally allocate video and graphics buffers through omapdrm since the corresponding omap-gem driver provides DMM-Tiler backed contiguous buffers. omapdrm is a dma-buf exporter. These buffers are used by other drivers in the video pipeline. Add VB2_DMABUF flag to the io_modes of the vb2 output and capture queues. This allows the driver to import dma shared buffers. Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 0363df6..0e7573a 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1770,7 +1770,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(src_vq, 0, sizeof(*src_vq)); src_vq-type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; - src_vq-io_modes = VB2_MMAP; + src_vq-io_modes = VB2_MMAP | VB2_DMABUF; src_vq-drv_priv = ctx; src_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); src_vq-ops = vpe_qops; @@ -1783,7 +1783,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(dst_vq, 0, sizeof(*dst_vq)); dst_vq-type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; - dst_vq-io_modes = VB2_MMAP; + dst_vq-io_modes = VB2_MMAP | VB2_DMABUF; dst_vq-drv_priv = ctx; dst_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); dst_vq-ops = vpe_qops; -- 1.8.3.2 -- 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 07/14] v4l: ti-vpe: Add selection API in VPE driver
Add selection ioctl ops. For VPE, cropping makes sense only for the input to VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers). For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP results in selecting a rectangle region within the source buffer. Setting the crop/compose rectangles should successfully result in re-configuration of registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 150 1 file changed, 150 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index ece9b96..f3f1c10 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx, { switch (type) { case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: + case V4L2_BUF_TYPE_VIDEO_OUTPUT: return ctx-q_data[Q_DATA_SRC]; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: + case V4L2_BUF_TYPE_VIDEO_CAPTURE: return ctx-q_data[Q_DATA_DST]; default: BUG(); @@ -1587,6 +1589,151 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f) return set_srcdst_params(ctx); } +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s) +{ + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + case V4L2_SEL_TGT_COMPOSE: + /* +* COMPOSE target is only valid for capture buffer type, return +* error for output buffer type +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) + return -EINVAL; + break; + case V4L2_SEL_TGT_CROP: + /* +* CROP target is only valid for output buffer type, return +* error for capture buffer type +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) + return -EINVAL; + break; + /* +* bound and default crop/compose targets are invalid targets to +* try/set +*/ + default: + return -EINVAL; + } + + if (s-r.top 0 || s-r.left 0) { + vpe_err(ctx-dev, negative values for top and left\n); + s-r.top = s-r.left = 0; + } + + v4l_bound_align_image(s-r.width, MIN_W, q_data-width, 1, + s-r.height, MIN_H, q_data-height, H_ALIGN, S_ALIGN); + + /* adjust left/top if cropping rectangle is out of bounds */ + if (s-r.left + s-r.width q_data-width) + s-r.left = q_data-width - s-r.width; + if (s-r.top + s-r.height q_data-height) + s-r.top = q_data-height - s-r.height; + + return 0; +} + +static int vpe_g_selection(struct file *file, void *fh, + struct v4l2_selection *s) +{ + struct vpe_ctx *ctx = file2ctx(file); + struct vpe_q_data *q_data; + bool use_c_rect = false; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + case V4L2_SEL_TGT_COMPOSE_DEFAULT: + case V4L2_SEL_TGT_COMPOSE_BOUNDS: + if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) + return -EINVAL; + break; + case V4L2_SEL_TGT_CROP_BOUNDS: + case V4L2_SEL_TGT_CROP_DEFAULT: + if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) + return -EINVAL; + break; + case V4L2_SEL_TGT_COMPOSE: + if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) + return -EINVAL; + use_c_rect = true; + break; + case V4L2_SEL_TGT_CROP: + if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) + return -EINVAL; + use_c_rect = true; + break; + default: + return -EINVAL; + } + + if (use_c_rect) { + /* +* for CROP/COMPOSE target type, return c_rect params from the +* respective buffer type +*/ + s-r = q_data-c_rect; + } else
[PATCH v4 12/14] v4l: ti-vpe: zero out reserved fields in try_fmt
Zero out the reserved formats in v4l2_pix_format_mplane and v4l2_plane_pix_format members of the returned v4l2_format pointer when passed through TRY_FMT ioctl. This ensures that the user doesn't interpret the non-zero fields as some data passed by the driver, and ensures compliance. Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index d7befb9..c0ae847 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1488,6 +1488,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, } } + memset(pix-reserved, 0, sizeof(pix-reserved)); for (i = 0; i pix-num_planes; i++) { plane_fmt = pix-plane_fmt[i]; depth = fmt-vpdma_fmt[i]-depth; @@ -1499,6 +1500,8 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, plane_fmt-sizeimage = (pix-height * pix-width * depth) 3; + + memset(plane_fmt-reserved, 0, sizeof(plane_fmt-reserved)); } return 0; -- 1.8.3.2 -- 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 14/14] v4l: ti-vpe: retain v4l2_buffer flags for captured buffers
The dequed CAPTURE_MPLANE type buffers don't contain the flags that the originally queued OUTPUT_MPLANE type buffers have. This breaks compliance. Copy the source v4l2_buffer flags to the destination v4l2_buffer flags before they are dequed. Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 362d5be..972f43f 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1288,13 +1288,12 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) s_buf = s_vb-v4l2_buf; d_buf = d_vb-v4l2_buf; + d_buf-flags = s_buf-flags; + d_buf-timestamp = s_buf-timestamp; - d_buf-flags = ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK; - d_buf-flags |= s_buf-flags V4L2_BUF_FLAG_TSTAMP_SRC_MASK; - if (s_buf-flags V4L2_BUF_FLAG_TIMECODE) { - d_buf-flags |= V4L2_BUF_FLAG_TIMECODE; + if (s_buf-flags V4L2_BUF_FLAG_TIMECODE) d_buf-timecode = s_buf-timecode; - } + d_buf-sequence = ctx-sequence; d_q_data = ctx-q_data[Q_DATA_DST]; -- 1.8.3.2 -- 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 08/14] v4l: ti-vpe: Rename csc memory resource name
Rename the memory block resource vpe_csc to csc since it also exists within the VIP IP block. This would make the name more generic, and both VPE and VIP DT nodes in the future can use it. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/csc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/csc.c b/drivers/media/platform/ti-vpe/csc.c index acfea50..039 100644 --- a/drivers/media/platform/ti-vpe/csc.c +++ b/drivers/media/platform/ti-vpe/csc.c @@ -180,7 +180,7 @@ struct csc_data *csc_create(struct platform_device *pdev) csc-pdev = pdev; csc-res = platform_get_resource_byname(pdev, IORESOURCE_MEM, - vpe_csc); + csc); if (csc-res == NULL) { dev_err(pdev-dev, missing platform resources data\n); return ERR_PTR(-ENODEV); -- 1.8.3.2 -- 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 05/14] v4l: ti-vpe: Allow usage of smaller images
The minimum width and height for VPE input/output was kept as 128 pixels. VPE doesn't have a constraint on the image height, it requires the image width to be at least 16 bytes. Change the minimum supported dimensions to 32x32. This allows us to de-interlace qcif content. A smaller image size than 32x32 didn't make much sense, so stopped at this. Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 0e7573a..dbdc338 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -49,8 +49,8 @@ #define VPE_MODULE_NAME vpe /* minimum and maximum frame sizes */ -#define MIN_W 128 -#define MIN_H 128 +#define MIN_W 32 +#define MIN_H 32 #define MAX_W 1920 #define MAX_H 1080 -- 1.8.3.2 -- 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 06/14] v4l: ti-vpe: Fix some params in VPE data descriptors
Some parameters of the VPE descriptors were understood incorrectly. They are now fixed. The fixes are explained as follows: - When adding an inbound data descriptor to the VPDMA descriptor list, we intend to use c_rect as the cropped region fetched by VPDMA. Therefore, c_rect-width shouldn't be used to calculate the line stride, the original image width should be used for that. We add a 'width' argument which gives the buffer width in memory. - frame_width and frame_height describe the complete width and height of the client to which the channel is connected. If there are multiple channels fetching data and providing to the same client, the above 2 arguments should be the width and height of the region covered by all the channels. In the case where there is only one channel providing pixel data to the client (like in VPE), frame_width and frame_height should be the cropped width and cropped height respectively. The calculation of these params is done in the vpe driver now. - start_h and start_v is also used in the case of multiple channels to describe where each channel should start filling pixel data. We don't use this in VPE, and pass 0s to the vpdma_add_in_dtd() helper. - Some minor changes are made to the vpdma_add_out_dtd() helper. The c_rect param is used for specifying the 'composition' target, and 'width' is added to calculate the line stride. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 60 +++ drivers/media/platform/ti-vpe/vpdma.h | 10 +++--- drivers/media/platform/ti-vpe/vpe.c | 18 +++ 3 files changed, 64 insertions(+), 24 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index 73dd38e..a51a013 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -614,8 +614,17 @@ static void dump_dtd(struct vpdma_dtd *dtd) /* * append an outbound data transfer descriptor to the given descriptor list, * this sets up a 'client to memory' VPDMA transfer for the given VPDMA channel + * + * @list: vpdma desc list to which we add this decriptor + * @width: width of the image in pixels in memory + * @c_rect: compose params of output image + * @fmt: vpdma data format of the buffer + * dma_addr: dma address as seen by VPDMA + * chan: VPDMA channel + * flags: VPDMA flags to configure some descriptor fileds */ -void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, +void vpdma_add_out_dtd(struct vpdma_desc_list *list, int width, + const struct v4l2_rect *c_rect, const struct vpdma_data_format *fmt, dma_addr_t dma_addr, enum vpdma_channel chan, u32 flags) { @@ -623,6 +632,7 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, int field = 0; int notify = 1; int channel, next_chan; + struct v4l2_rect rect = *c_rect; int depth = fmt-depth; int stride; struct vpdma_dtd *dtd; @@ -630,11 +640,15 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, channel = next_chan = chan_info[chan].num; if (fmt-type == VPDMA_DATA_FMT_TYPE_YUV - fmt-data_type == DATA_TYPE_C420) + fmt-data_type == DATA_TYPE_C420) { + rect.height = 1; + rect.top = 1; depth = 8; + } - stride = ALIGN((depth * c_rect-width) 3, VPDMA_STRIDE_ALIGN); - dma_addr += (c_rect-left * depth) 3; + stride = ALIGN((depth * width) 3, VPDMA_STRIDE_ALIGN); + + dma_addr += rect.top * stride + (rect.left * depth 3); dtd = list-next; WARN_ON((void *)(dtd + 1) (list-buf.addr + list-buf.size)); @@ -664,31 +678,48 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, /* * append an inbound data transfer descriptor to the given descriptor list, * this sets up a 'memory to client' VPDMA transfer for the given VPDMA channel + * + * @list: vpdma desc list to which we add this decriptor + * @width: width of the image in pixels in memory(not the cropped width) + * @c_rect: crop params of input image + * @fmt: vpdma data format of the buffer + * dma_addr: dma address as seen by VPDMA + * chan: VPDMA channel + * field: top or bottom field info of the input image + * flags: VPDMA flags to configure some descriptor fileds + * frame_width/height: the complete width/height of the image presented to the + * client (this makes sense when multiple channels are + * connected to the same client, forming a larger frame) + * start_h, start_v: position where the given channel starts providing pixel + * data to the client (makes sense when multiple channels + * contribute to the client) */ -void vpdma_add_in_dtd(struct
[PATCH v4 11/14] v4l: ti-vpe: Fix initial configuration queue data
The vpe output and capture queues are initially configured to default values in vpe_open(). A G_FMT before any S_FMTs will result in these values being populated. The colorspace and bytesperline parameter of this initial configuration are incorrect. This breaks compliance when as we get 'TRY_FMT(G_FMT) != G_FMT'. Fix the initial queue configuration such that it wouldn't need to be fixed by try_fmt. Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 3a312ea..d7befb9 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -2021,9 +2021,11 @@ static int vpe_open(struct file *file) s_q_data-fmt = vpe_formats[2]; s_q_data-width = 1920; s_q_data-height = 1080; - s_q_data-sizeimage[VPE_LUMA] = (s_q_data-width * s_q_data-height * + s_q_data-bytesperline[VPE_LUMA] = (s_q_data-width * s_q_data-fmt-vpdma_fmt[VPE_LUMA]-depth) 3; - s_q_data-colorspace = V4L2_COLORSPACE_SMPTE170M; + s_q_data-sizeimage[VPE_LUMA] = (s_q_data-bytesperline[VPE_LUMA] * + s_q_data-height); + s_q_data-colorspace = V4L2_COLORSPACE_REC709; s_q_data-field = V4L2_FIELD_NONE; s_q_data-c_rect.left = 0; s_q_data-c_rect.top = 0; -- 1.8.3.2 -- 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 10/14] v4l: ti-vpe: Use correct bus_info name for the device in querycap
The bus_info parameter in v4l2_capabilities expects a 'platform_' prefix. This wasn't done in the driver and hence was breaking compliance. Update the bus_info parameter accordingly. Reviewed-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index c066eb8..3a312ea 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1346,7 +1346,8 @@ static int vpe_querycap(struct file *file, void *priv, { strncpy(cap-driver, VPE_MODULE_NAME, sizeof(cap-driver) - 1); strncpy(cap-card, VPE_MODULE_NAME, sizeof(cap-card) - 1); - strlcpy(cap-bus_info, VPE_MODULE_NAME, sizeof(cap-bus_info)); + snprintf(cap-bus_info, sizeof(cap-bus_info), platform:%s, + VPE_MODULE_NAME); cap-device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; cap-capabilities = cap-device_caps | V4L2_CAP_DEVICE_CAPS; return 0; -- 1.8.3.2 -- 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 02/14] v4l: ti-vpe: register video device only when firmware is loaded
Hi Kamil, On Thursday 13 March 2014 05:18 PM, Kamil Debski wrote: Hi Archit, From: Archit Taneja [mailto:arc...@ti.com] Sent: Tuesday, March 11, 2014 9:34 AM vpe fops(vpe_open in particular) should be called only when VPDMA firmware is loaded. File operations on the video device are possible the moment it is registered. Currently, we register the video device for VPE at driver probe, after calling a vpdma helper to initialize VPDMA and load firmware. This function is non-blocking(it calls request_firmware_nowait()), and doesn't ensure that the firmware is actually loaded when it returns. We remove the device registration from vpe probe, and move it to a callback provided by the vpe driver to the vpdma library, through vpdma_create(). The ready field in vpdma_data is no longer needed since we always have firmware loaded before the device is registered. A minor problem with this approach is that if the video_register_device fails(which doesn't really happen), the vpe platform device would be registered. however, there won't be any v4l2 device corresponding to it. Could you explain to me one thing. request_firmware cannot be used in probe, thus you are using request_firmware_nowait. Why cannot the firmware be loaded on open with a regular request_firmware that is waiting? I totally agree with you here. Placing the firmware in open() would probably make more sense. The reason I didn't place it in open() is because I didn't want to release firmware in a corresponding close(), and be in a situation where the firmware is loaded multiple times in the driver's lifetime. There are some reasons for doing this. First, it will require more testing with respect to whether the firmware is loaded correctly successive times :), the second is that loading firmware might probably take a bit of time, and we don't want to make applications too slow(I haven't measured the time taken, so I don't have a strong case for this either) This patch seems to swap one problem for another. The possibility that open fails (because firmware is not yet loaded) is swapped for a vague possibility that video_register_device. The driver will work fine in most cases even without this patch(apart from the possibility mentioned as above). We could discard this patch from this series, and I can work on a patch which moves firmware loading to the vpe_open() call, and hence solving the issue in the right manner. Does that sound fine? Thanks, Archit -- 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 v3 00/14] v4l: ti-vpe: Some VPE fixes and enhancements
This patch set mainly consists of minor fixes for the VPE driver. These fixes ensure the following: - The VPE module can be inserted and removed successively. - Make sure that smaller resolutions like qcif work correctly. - Prevent race condition between firmware loading and an open call to the v4l2 device. - Prevent the possibility of output m2m queue not having sufficient 'ready' buffers. - Some VPDMA data descriptor fields weren't understood correctly before. They are now used correctly. The rest of the patches add some minor features like DMA buf support and cropping/composing. Reference branch: g...@github.com:boddob/linux.git vpe_for_315 Changes in v3: - improvements in selection API patch. - querycap fixes for v4l2 compliance. - v4l2_buffer 'field' and flags' fixes for compliance. - fixes in try_fmt vpe_open for compliance. - rename a IOMEM resource for better DT compatibility. Changes in v2: - selection API used instead of older cropping API. - Typo fix. - Some changes in patch 6/7 to support composing on the capture side of VPE. Archit Taneja (14): v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs v4l: ti-vpe: register video device only when firmware is loaded v4l: ti-vpe: Use video_device_release_empty v4l: ti-vpe: Allow DMABUF buffer type support v4l: ti-vpe: Allow usage of smaller images v4l: ti-vpe: Fix some params in VPE data descriptors v4l: ti-vpe: Add selection API in VPE driver v4l: ti-vpe: Rename csc memory resource name v4l: ti-vpe: report correct capabilities in querycap v4l: ti-vpe: Use correct bus_info name for the device in querycap v4l: ti-vpe: Fix initial configuration queue data v4l: ti-vpe: zero out reserved fields in try_fmt v4l: ti-vpe: Set correct field parameter for output and capture buffers v4l: ti-vpe: retain v4l2_buffer flags for captured buffers drivers/media/platform/ti-vpe/csc.c | 2 +- drivers/media/platform/ti-vpe/vpdma.c | 68 ++--- drivers/media/platform/ti-vpe/vpdma.h | 17 ++- drivers/media/platform/ti-vpe/vpe.c | 263 -- 4 files changed, 281 insertions(+), 69 deletions(-) -- 1.8.3.2 -- 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 v3 03/14] v4l: ti-vpe: Use video_device_release_empty
The video_device struct is currently embedded in the driver data struct vpe_dev. A vpe_dev instance is allocated by the driver, and the memory for the vfd is a part of this struct. The v4l2 core, however, manages the removal of the vfd region, through the video_device's .release() op, which currently is the helper video_device_release. This causes memory corruption, and leads to issues when we try to re-insert the vpe module. Use the video_device_release_empty helper function instead Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index f1eae67..0363df6 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -2000,7 +2000,7 @@ static struct video_device vpe_videodev = { .fops = vpe_fops, .ioctl_ops = vpe_ioctl_ops, .minor = -1, - .release= video_device_release, + .release= video_device_release_empty, .vfl_dir= VFL_DIR_M2M, }; -- 1.8.3.2 -- 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 v3 01/14] v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs
VPE has a ctrl parameter which decides how many mem to mem transactions the active job from the job queue can perform. The driver's job_ready() made sure that the number of ready source buffers are sufficient for the job to execute successfully. But it didn't make sure if there are sufficient ready destination buffers in the capture queue for the VPE output. If the time taken by VPE to process a single frame is really slow, then it's possible that we don't need to imply such a restriction on the dst queue, but really fast transactions(small resolution, no de-interlacing) may cause us to hit the condition where we don't have any free buffers for the VPE to write on. Add the extra check in job_ready() to make sure we have the sufficient amount of destination buffers. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 7a77a5b..f3143ac 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -887,6 +887,9 @@ static int job_ready(void *priv) if (v4l2_m2m_num_src_bufs_ready(ctx-m2m_ctx) needed) return 0; + if (v4l2_m2m_num_dst_bufs_ready(ctx-m2m_ctx) needed) + return 0; + return 1; } -- 1.8.3.2 -- 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 v3 11/14] v4l: ti-vpe: Fix initial configuration queue data
The vpe output and capture queues are initially configured to default values in vpe_open(). A G_FMT before any S_FMTs will result in these values being populated. The colorspace and bytesperline parameter of this initial configuration are incorrect. This breaks compliance when as we get 'TRY_FMT(G_FMT) != G_FMT'. Fix the initial queue configuration such that it wouldn't need to be fixed by try_fmt. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 5591d04..85d1122 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -2012,9 +2012,11 @@ static int vpe_open(struct file *file) s_q_data-fmt = vpe_formats[2]; s_q_data-width = 1920; s_q_data-height = 1080; - s_q_data-sizeimage[VPE_LUMA] = (s_q_data-width * s_q_data-height * + s_q_data-bytesperline[VPE_LUMA] = (s_q_data-width * s_q_data-fmt-vpdma_fmt[VPE_LUMA]-depth) 3; - s_q_data-colorspace = V4L2_COLORSPACE_SMPTE170M; + s_q_data-sizeimage[VPE_LUMA] = (s_q_data-bytesperline[VPE_LUMA] * + s_q_data-height); + s_q_data-colorspace = V4L2_COLORSPACE_REC709; s_q_data-field = V4L2_FIELD_NONE; s_q_data-c_rect.left = 0; s_q_data-c_rect.top = 0; -- 1.8.3.2 -- 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 v3 13/14] v4l: ti-vpe: Set correct field parameter for output and capture buffers
The vpe driver wasn't setting the correct field parameter for dequed CAPTURE type buffers for the case where the captured output is progressive. Set the field to V4L2_FIELD_NONE for the completed destination buffers when the captured output is progressive. For OUTPUT type buffers, a queued buffer's field is forced to V4L2_FIELD_NONE if the pixel format(configured through s_fmt for the buffer type V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE specifies) the field type isn't interlaced. If the pixel format specified was V4L2_FIELD_ALTERNATE, and the queued buffer's field isn't V4L2_FIELD_TOP or V4L2_FIELD_BOTTOM, the vb2 buf_prepare op returns an error. This ensures compliance, and that the dequeued output and captured buffers contain the field type that the driver used internally. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 970408a..c884910 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1296,10 +1296,10 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) d_buf-timecode = s_buf-timecode; } d_buf-sequence = ctx-sequence; - d_buf-field = ctx-field; d_q_data = ctx-q_data[Q_DATA_DST]; if (d_q_data-flags Q_DATA_INTERLACED) { + d_buf-field = ctx-field; if (ctx-field == V4L2_FIELD_BOTTOM) { ctx-sequence++; ctx-field = V4L2_FIELD_TOP; @@ -1308,6 +1308,7 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) ctx-field = V4L2_FIELD_BOTTOM; } } else { + d_buf-field = V4L2_FIELD_NONE; ctx-sequence++; } @@ -1871,6 +1872,16 @@ static int vpe_buf_prepare(struct vb2_buffer *vb) q_data = get_q_data(ctx, vb-vb2_queue-type); num_planes = q_data-fmt-coplanar ? 2 : 1; + if (vb-vb2_queue-type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { + if (!(q_data-flags Q_DATA_INTERLACED)) { + vb-v4l2_buf.field = V4L2_FIELD_NONE; + } else { + if (vb-v4l2_buf.field != V4L2_FIELD_TOP || + vb-v4l2_buf.field != V4L2_FIELD_BOTTOM) + return -EINVAL; + } + } + for (i = 0; i num_planes; i++) { if (vb2_plane_size(vb, i) q_data-sizeimage[i]) { vpe_err(ctx-dev, -- 1.8.3.2 -- 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 v3 07/14] v4l: ti-vpe: Add selection API in VPE driver
Add selection ioctl ops. For VPE, cropping makes sense only for the input to VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers). For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP results in selecting a rectangle region within the source buffer. Setting the crop/compose rectangles should successfully result in re-configuration of registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 141 1 file changed, 141 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index ece9b96..4abb85c 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx, { switch (type) { case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: + case V4L2_BUF_TYPE_VIDEO_OUTPUT: return ctx-q_data[Q_DATA_SRC]; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: + case V4L2_BUF_TYPE_VIDEO_CAPTURE: return ctx-q_data[Q_DATA_DST]; default: BUG(); @@ -1587,6 +1589,142 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f) return set_srcdst_params(ctx); } +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s) +{ + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + case V4L2_SEL_TGT_COMPOSE: + /* +* COMPOSE target is only valid for capture buffer type, for +* output buffer type, assign existing crop parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) + break; + + s-r = q_data-c_rect; + return 0; + + case V4L2_SEL_TGT_CROP: + /* +* CROP target is only valid for output buffer type, for capture +* buffer type, assign existing compose parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) + break; + + s-r = q_data-c_rect; + return 0; + + /* +* bound and default crop/compose targets are invalid targets to +* try/set +*/ + default: + return -EINVAL; + } + + if (s-r.top 0 || s-r.left 0) { + vpe_err(ctx-dev, negative values for top and left\n); + s-r.top = s-r.left = 0; + } + + v4l_bound_align_image(s-r.width, MIN_W, q_data-width, 1, + s-r.height, MIN_H, q_data-height, H_ALIGN, S_ALIGN); + + /* adjust left/top if cropping rectangle is out of bounds */ + if (s-r.left + s-r.width q_data-width) + s-r.left = q_data-width - s-r.width; + if (s-r.top + s-r.height q_data-height) + s-r.top = q_data-height - s-r.height; + + return 0; +} + +static int vpe_g_selection(struct file *file, void *fh, + struct v4l2_selection *s) +{ + struct vpe_ctx *ctx = file2ctx(file); + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + /* return width and height from S_FMT of the respective buffer type */ + case V4L2_SEL_TGT_COMPOSE_DEFAULT: + case V4L2_SEL_TGT_COMPOSE_BOUNDS: + case V4L2_SEL_TGT_CROP_BOUNDS: + case V4L2_SEL_TGT_CROP_DEFAULT: + s-r.left = 0; + s-r.top = 0; + s-r.width = q_data-width; + s-r.height = q_data-height; + return 0; + + /* +* CROP target holds for the output buffer type, and COMPOSE target +* holds for the capture buffer type. We still return the c_rect params +* for both the target types. +*/ + case V4L2_SEL_TGT_COMPOSE: + case V4L2_SEL_TGT_CROP: + s-r.left = q_data-c_rect.left; + s-r.top = q_data-c_rect.top; + s-r.width = q_data-c_rect.width; + s-r.height = q_data-c_rect.height; + return 0; + } + + return
[PATCH v3 05/14] v4l: ti-vpe: Allow usage of smaller images
The minimum width and height for VPE input/output was kept as 128 pixels. VPE doesn't have a constraint on the image height, it requires the image width to be at least 16 bytes. Change the minimum supported dimensions to 32x32. This allows us to de-interlace qcif content. A smaller image size than 32x32 didn't make much sense, so stopped at this. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 0e7573a..dbdc338 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -49,8 +49,8 @@ #define VPE_MODULE_NAME vpe /* minimum and maximum frame sizes */ -#define MIN_W 128 -#define MIN_H 128 +#define MIN_W 32 +#define MIN_H 32 #define MAX_W 1920 #define MAX_H 1080 -- 1.8.3.2 -- 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 v3 04/14] v4l: ti-vpe: Allow DMABUF buffer type support
For OMAP and DRA7x, we generally allocate video and graphics buffers through omapdrm since the corresponding omap-gem driver provides DMM-Tiler backed contiguous buffers. omapdrm is a dma-buf exporter. These buffers are used by other drivers in the video pipeline. Add VB2_DMABUF flag to the io_modes of the vb2 output and capture queues. This allows the driver to import dma shared buffers. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 0363df6..0e7573a 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1770,7 +1770,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(src_vq, 0, sizeof(*src_vq)); src_vq-type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; - src_vq-io_modes = VB2_MMAP; + src_vq-io_modes = VB2_MMAP | VB2_DMABUF; src_vq-drv_priv = ctx; src_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); src_vq-ops = vpe_qops; @@ -1783,7 +1783,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(dst_vq, 0, sizeof(*dst_vq)); dst_vq-type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; - dst_vq-io_modes = VB2_MMAP; + dst_vq-io_modes = VB2_MMAP | VB2_DMABUF; dst_vq-drv_priv = ctx; dst_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); dst_vq-ops = vpe_qops; -- 1.8.3.2 -- 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 v3 10/14] v4l: ti-vpe: Use correct bus_info name for the device in querycap
The bus_info parameter in v4l2_capabilities expects a 'platform_' prefix. This wasn't done in the driver and hence was breaking compliance. Update the bus_info parameter accordingly. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 46b9d44..5591d04 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1346,7 +1346,8 @@ static int vpe_querycap(struct file *file, void *priv, { strncpy(cap-driver, VPE_MODULE_NAME, sizeof(cap-driver) - 1); strncpy(cap-card, VPE_MODULE_NAME, sizeof(cap-card) - 1); - strlcpy(cap-bus_info, VPE_MODULE_NAME, sizeof(cap-bus_info)); + snprintf(cap-bus_info, sizeof(cap-bus_info), platform:%s, + VPE_MODULE_NAME); cap-device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; cap-capabilities = cap-device_caps | V4L2_CAP_DEVICE_CAPS; return 0; -- 1.8.3.2 -- 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 v3 12/14] v4l: ti-vpe: zero out reserved fields in try_fmt
Zero out the reserved formats in v4l2_pix_format_mplane and v4l2_plane_pix_format members of the returned v4l2_format pointer when passed through TRY_FMT ioctl. This ensures that the user doesn't interpret the non-zero fields as some data passed by the driver, and ensures compliance. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 85d1122..970408a 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1488,6 +1488,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, } } + memset(pix-reserved, 0, sizeof(pix-reserved)); for (i = 0; i pix-num_planes; i++) { plane_fmt = pix-plane_fmt[i]; depth = fmt-vpdma_fmt[i]-depth; @@ -1499,6 +1500,8 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, plane_fmt-sizeimage = (pix-height * pix-width * depth) 3; + + memset(plane_fmt-reserved, 0, sizeof(plane_fmt-reserved)); } return 0; -- 1.8.3.2 -- 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 v3 09/14] v4l: ti-vpe: report correct capabilities in querycap
querycap currently returns V4L2_CAP_VIDEO_M2M as a capability, this should be V4L2_CAP_VIDEO_M2M_MPLANE instead, as the driver supports multiplanar formats. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 4abb85c..46b9d44 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1347,7 +1347,7 @@ static int vpe_querycap(struct file *file, void *priv, strncpy(cap-driver, VPE_MODULE_NAME, sizeof(cap-driver) - 1); strncpy(cap-card, VPE_MODULE_NAME, sizeof(cap-card) - 1); strlcpy(cap-bus_info, VPE_MODULE_NAME, sizeof(cap-bus_info)); - cap-device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING; + cap-device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; cap-capabilities = cap-device_caps | V4L2_CAP_DEVICE_CAPS; return 0; } -- 1.8.3.2 -- 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 v3 14/14] v4l: ti-vpe: retain v4l2_buffer flags for captured buffers
The dequed CAPTURE_MPLANE type buffers don't contain the flags that the originally queued OUTPUT_MPLANE type buffers have. This breaks compliance. Copy the source v4l2_buffer flags to the destination v4l2_buffer flags before they are dequed. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index c884910..f7759e8 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1288,13 +1288,12 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) s_buf = s_vb-v4l2_buf; d_buf = d_vb-v4l2_buf; + d_buf-flags = s_buf-flags; + d_buf-timestamp = s_buf-timestamp; - d_buf-flags = ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK; - d_buf-flags |= s_buf-flags V4L2_BUF_FLAG_TSTAMP_SRC_MASK; - if (s_buf-flags V4L2_BUF_FLAG_TIMECODE) { - d_buf-flags |= V4L2_BUF_FLAG_TIMECODE; + if (s_buf-flags V4L2_BUF_FLAG_TIMECODE) d_buf-timecode = s_buf-timecode; - } + d_buf-sequence = ctx-sequence; d_q_data = ctx-q_data[Q_DATA_DST]; -- 1.8.3.2 -- 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 v3 08/14] v4l: ti-vpe: Rename csc memory resource name
Rename the memory block resource vpe_csc to csc since it also exists within the VIP IP block. This would make the name more generic, and both VPE and VIP DT nodes in the future can use it. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/csc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/csc.c b/drivers/media/platform/ti-vpe/csc.c index acfea50..039 100644 --- a/drivers/media/platform/ti-vpe/csc.c +++ b/drivers/media/platform/ti-vpe/csc.c @@ -180,7 +180,7 @@ struct csc_data *csc_create(struct platform_device *pdev) csc-pdev = pdev; csc-res = platform_get_resource_byname(pdev, IORESOURCE_MEM, - vpe_csc); + csc); if (csc-res == NULL) { dev_err(pdev-dev, missing platform resources data\n); return ERR_PTR(-ENODEV); -- 1.8.3.2 -- 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 v3 02/14] v4l: ti-vpe: register video device only when firmware is loaded
vpe fops(vpe_open in particular) should be called only when VPDMA firmware is loaded. File operations on the video device are possible the moment it is registered. Currently, we register the video device for VPE at driver probe, after calling a vpdma helper to initialize VPDMA and load firmware. This function is non-blocking(it calls request_firmware_nowait()), and doesn't ensure that the firmware is actually loaded when it returns. We remove the device registration from vpe probe, and move it to a callback provided by the vpe driver to the vpdma library, through vpdma_create(). The ready field in vpdma_data is no longer needed since we always have firmware loaded before the device is registered. A minor problem with this approach is that if the video_register_device fails(which doesn't really happen), the vpe platform device would be registered. however, there won't be any v4l2 device corresponding to it. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 8 +++-- drivers/media/platform/ti-vpe/vpdma.h | 7 +++-- drivers/media/platform/ti-vpe/vpe.c | 55 --- 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index e8175e7..73dd38e 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -781,7 +781,7 @@ static void vpdma_firmware_cb(const struct firmware *f, void *context) /* already initialized */ if (read_field_reg(vpdma, VPDMA_LIST_ATTR, VPDMA_LIST_RDY_MASK, VPDMA_LIST_RDY_SHFT)) { - vpdma-ready = true; + vpdma-cb(vpdma-pdev); return; } @@ -811,7 +811,7 @@ static void vpdma_firmware_cb(const struct firmware *f, void *context) goto free_buf; } - vpdma-ready = true; + vpdma-cb(vpdma-pdev); free_buf: vpdma_unmap_desc_buf(vpdma, fw_dma_buf); @@ -839,7 +839,8 @@ static int vpdma_load_firmware(struct vpdma_data *vpdma) return 0; } -struct vpdma_data *vpdma_create(struct platform_device *pdev) +struct vpdma_data *vpdma_create(struct platform_device *pdev, + void (*cb)(struct platform_device *pdev)) { struct resource *res; struct vpdma_data *vpdma; @@ -854,6 +855,7 @@ struct vpdma_data *vpdma_create(struct platform_device *pdev) } vpdma-pdev = pdev; + vpdma-cb = cb; res = platform_get_resource_byname(pdev, IORESOURCE_MEM, vpdma); if (res == NULL) { diff --git a/drivers/media/platform/ti-vpe/vpdma.h b/drivers/media/platform/ti-vpe/vpdma.h index cf40f11..bf5f8bb 100644 --- a/drivers/media/platform/ti-vpe/vpdma.h +++ b/drivers/media/platform/ti-vpe/vpdma.h @@ -35,8 +35,8 @@ struct vpdma_data { struct platform_device *pdev; - /* tells whether vpdma firmware is loaded or not */ - bool ready; + /* callback to VPE driver when the firmware is loaded */ + void (*cb)(struct platform_device *pdev); }; enum vpdma_data_format_type { @@ -208,6 +208,7 @@ void vpdma_set_frame_start_event(struct vpdma_data *vpdma, void vpdma_dump_regs(struct vpdma_data *vpdma); /* initialize vpdma, passed with VPE's platform device pointer */ -struct vpdma_data *vpdma_create(struct platform_device *pdev); +struct vpdma_data *vpdma_create(struct platform_device *pdev, + void (*cb)(struct platform_device *pdev)); #endif diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index f3143ac..f1eae67 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1817,11 +1817,6 @@ static int vpe_open(struct file *file) vpe_dbg(dev, vpe_open\n); - if (!dev-vpdma-ready) { - vpe_err(dev, vpdma firmware not loaded\n); - return -ENODEV; - } - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) return -ENOMEM; @@ -2039,10 +2034,40 @@ static void vpe_runtime_put(struct platform_device *pdev) WARN_ON(r 0 r != -ENOSYS); } +static void vpe_fw_cb(struct platform_device *pdev) +{ + struct vpe_dev *dev = platform_get_drvdata(pdev); + struct video_device *vfd; + int ret; + + vfd = dev-vfd; + *vfd = vpe_videodev; + vfd-lock = dev-dev_mutex; + vfd-v4l2_dev = dev-v4l2_dev; + + ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0); + if (ret) { + vpe_err(dev, Failed to register video device\n); + + vpe_set_clock_enable(dev, 0); + vpe_runtime_put(pdev); + pm_runtime_disable(pdev-dev); + v4l2_m2m_release(dev-m2m_dev); + vb2_dma_contig_cleanup_ctx(dev-alloc_ctx); + v4l2_device_unregister(dev-v4l2_dev); + + return; + } + + video_set_drvdata
[PATCH v3 06/14] v4l: ti-vpe: Fix some params in VPE data descriptors
Some parameters of the VPE descriptors were understood incorrectly. They are now fixed. The fixes are explained as follows: - When adding an inbound data descriptor to the VPDMA descriptor list, we intend to use c_rect as the cropped region fetched by VPDMA. Therefore, c_rect-width shouldn't be used to calculate the line stride, the original image width should be used for that. We add a 'width' argument which gives the buffer width in memory. - frame_width and frame_height describe the complete width and height of the client to which the channel is connected. If there are multiple channels fetching data and providing to the same client, the above 2 arguments should be the width and height of the region covered by all the channels. In the case where there is only one channel providing pixel data to the client (like in VPE), frame_width and frame_height should be the cropped width and cropped height respectively. The calculation of these params is done in the vpe driver now. - start_h and start_v is also used in the case of multiple channels to describe where each channel should start filling pixel data. We don't use this in VPE, and pass 0s to the vpdma_add_in_dtd() helper. - Some minor changes are made to the vpdma_add_out_dtd() helper. The c_rect param is used for specifying the 'composition' target, and 'width' is added to calculate the line stride. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 60 +++ drivers/media/platform/ti-vpe/vpdma.h | 10 +++--- drivers/media/platform/ti-vpe/vpe.c | 18 +++ 3 files changed, 64 insertions(+), 24 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index 73dd38e..a51a013 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -614,8 +614,17 @@ static void dump_dtd(struct vpdma_dtd *dtd) /* * append an outbound data transfer descriptor to the given descriptor list, * this sets up a 'client to memory' VPDMA transfer for the given VPDMA channel + * + * @list: vpdma desc list to which we add this decriptor + * @width: width of the image in pixels in memory + * @c_rect: compose params of output image + * @fmt: vpdma data format of the buffer + * dma_addr: dma address as seen by VPDMA + * chan: VPDMA channel + * flags: VPDMA flags to configure some descriptor fileds */ -void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, +void vpdma_add_out_dtd(struct vpdma_desc_list *list, int width, + const struct v4l2_rect *c_rect, const struct vpdma_data_format *fmt, dma_addr_t dma_addr, enum vpdma_channel chan, u32 flags) { @@ -623,6 +632,7 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, int field = 0; int notify = 1; int channel, next_chan; + struct v4l2_rect rect = *c_rect; int depth = fmt-depth; int stride; struct vpdma_dtd *dtd; @@ -630,11 +640,15 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, channel = next_chan = chan_info[chan].num; if (fmt-type == VPDMA_DATA_FMT_TYPE_YUV - fmt-data_type == DATA_TYPE_C420) + fmt-data_type == DATA_TYPE_C420) { + rect.height = 1; + rect.top = 1; depth = 8; + } - stride = ALIGN((depth * c_rect-width) 3, VPDMA_STRIDE_ALIGN); - dma_addr += (c_rect-left * depth) 3; + stride = ALIGN((depth * width) 3, VPDMA_STRIDE_ALIGN); + + dma_addr += rect.top * stride + (rect.left * depth 3); dtd = list-next; WARN_ON((void *)(dtd + 1) (list-buf.addr + list-buf.size)); @@ -664,31 +678,48 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, /* * append an inbound data transfer descriptor to the given descriptor list, * this sets up a 'memory to client' VPDMA transfer for the given VPDMA channel + * + * @list: vpdma desc list to which we add this decriptor + * @width: width of the image in pixels in memory(not the cropped width) + * @c_rect: crop params of input image + * @fmt: vpdma data format of the buffer + * dma_addr: dma address as seen by VPDMA + * chan: VPDMA channel + * field: top or bottom field info of the input image + * flags: VPDMA flags to configure some descriptor fileds + * frame_width/height: the complete width/height of the image presented to the + * client (this makes sense when multiple channels are + * connected to the same client, forming a larger frame) + * start_h, start_v: position where the given channel starts providing pixel + * data to the client (makes sense when multiple channels + * contribute to the client) */ -void vpdma_add_in_dtd(struct
Re: [PATCH v3 07/14] v4l: ti-vpe: Add selection API in VPE driver
On Tuesday 11 March 2014 05:51 PM, Hans Verkuil wrote: Hi Archit, A few small comments below... On 03/11/14 09:33, Archit Taneja wrote: Add selection ioctl ops. For VPE, cropping makes sense only for the input to VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers). For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP results in selecting a rectangle region within the source buffer. Setting the crop/compose rectangles should successfully result in re-configuration of registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 141 1 file changed, 141 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index ece9b96..4abb85c 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx, { switch (type) { case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: + case V4L2_BUF_TYPE_VIDEO_OUTPUT: return ctx-q_data[Q_DATA_SRC]; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: + case V4L2_BUF_TYPE_VIDEO_CAPTURE: return ctx-q_data[Q_DATA_DST]; default: BUG(); @@ -1587,6 +1589,142 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f) return set_srcdst_params(ctx); } +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s) +{ + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + case V4L2_SEL_TGT_COMPOSE: + /* +* COMPOSE target is only valid for capture buffer type, for +* output buffer type, assign existing crop parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) + break; Shouldn't this return -EINVAL? compose only makes sense for CAPTURE. So, it breaks and performs the calculations after the switch statement. + + s-r = q_data-c_rect; + return 0; The above 2 lines are called for if we try to set compose for OUTPUT. I don't return an error here, just keep the size as the original rect size, and return 0. I'll replace these 2 lines with 'return -EINVAL;' + + case V4L2_SEL_TGT_CROP: + /* +* CROP target is only valid for output buffer type, for capture +* buffer type, assign existing compose parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) + break; Ditto. + + s-r = q_data-c_rect; + return 0; + + /* +* bound and default crop/compose targets are invalid targets to +* try/set +*/ + default: + return -EINVAL; + } + + if (s-r.top 0 || s-r.left 0) { + vpe_err(ctx-dev, negative values for top and left\n); + s-r.top = s-r.left = 0; + } + + v4l_bound_align_image(s-r.width, MIN_W, q_data-width, 1, + s-r.height, MIN_H, q_data-height, H_ALIGN, S_ALIGN); + + /* adjust left/top if cropping rectangle is out of bounds */ + if (s-r.left + s-r.width q_data-width) + s-r.left = q_data-width - s-r.width; + if (s-r.top + s-r.height q_data-height) + s-r.top = q_data-height - s-r.height; + + return 0; +} + +static int vpe_g_selection(struct file *file, void *fh, + struct v4l2_selection *s) +{ + struct vpe_ctx *ctx = file2ctx(file); + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + /* return width and height from S_FMT of the respective buffer type */ + case V4L2_SEL_TGT_COMPOSE_DEFAULT: + case V4L2_SEL_TGT_COMPOSE_BOUNDS: + case V4L2_SEL_TGT_CROP_BOUNDS: + case V4L2_SEL_TGT_CROP_DEFAULT: + s-r.left = 0; + s-r.top = 0; + s-r.width = q_data-width; + s-r.height = q_data-height; The crop targets only make sense
Re: [PATCH v3 07/14] v4l: ti-vpe: Add selection API in VPE driver
On Tuesday 11 March 2014 06:19 PM, Hans Verkuil wrote: On 03/11/14 13:46, Archit Taneja wrote: On Tuesday 11 March 2014 05:51 PM, Hans Verkuil wrote: Hi Archit, A few small comments below... On 03/11/14 09:33, Archit Taneja wrote: snip Yes. If for no other reason that I plan on adding crop/compose/scaling support to v4l2-compliance soon, and this will probably be one of the tests. Thanks, will update. Thanks for reviewing of the series. Archit Thanks, Archit -- 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 -- 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 v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
Hi Hans, On Tuesday 04 March 2014 05:05 PM, Hans Verkuil wrote: On 03/04/14 12:25, Archit Taneja wrote: I had a minor question about the selection API: Are the V4L2_SET_TGT_CROP/COMPOSE_DEFAULT and the corresponding 'BOUNDS' targets supposed to be used with VIDIOC_S_SELECTION? If so, what's the expect behaviour? No, those are read only in practice. So only used with G_SELECTION, never with S_SELECTION. snip I tried the v4l2-compliance thing. It's awesome! And a bit annoying too when it comes to fixing little things needed for compliance :). But it's required, and I hope to fix these eventually. After a few small fixes in the driver, I get the results as below. I am debugging the cause of try_fmt and s_fmt failures. I'm not sure why the streaming test fails with MMAP, the logs of my driver show that a successful mem2mem transaction happened. I tried this on the 'vb2-part1' branch as you suggested. Do you think I can go ahead with posting the v3 patch set for 3.15, and work on fixing the compliance issue for the -rc fixes? Thanks, Archit # ./utils/v4l2-compliance/v4l2-compliance -v --streaming=10 root@localhost:~/source_trees/v4l-utils# Driver Info: Driver name : vpe Card type : vpe Bus info : platform:vpe Driver version: 3.14.0 Capabilities : 0x84004000 Video Memory-to-Memory Multiplanar Streaming Device Capabilities Device Caps : 0x04004000 Video Memory-to-Memory Multiplanar Streaming 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 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: 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 Control ioctls: info: checking v4l2_queryctrl of control 'User Controls' (0x00980001) info: checking v4l2_queryctrl of control 'Buffers Per Transaction' (0x00981950) info: checking v4l2_queryctrl of control 'Buffers Per Transaction' (0x0800) test VIDIOC_QUERYCTRL/MENU: OK info: checking control 'User Controls' (0x00980001) info: checking control 'Buffers Per Transaction' (0x00981950) test VIDIOC_G/S_CTRL: OK info: checking extended control 'User Controls' (0x00980001) info: checking extended control 'Buffers Per Transaction' (0x00981950) test VIDIOC_G/S/TRY_EXT_CTRLS: OK info: checking control event 'User Controls' (0x00980001) info: checking control event 'Buffers Per Transaction' (0x00981950) test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 1 Private Controls: 1 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) Format ioctls: info: found 8 formats for buftype 9 info: found 4 formats for buftype 10 test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK fail: v4l2-test-formats.cpp(614): Video Capture Multiplanar: TRY_FMT(G_FMT) != G_FMT test VIDIOC_TRY_FMT: FAIL warn: v4l2-test-formats.cpp(834): S_FMT cannot handle an invalid pixelformat. warn: v4l2-test-formats.cpp(835): This may or may not be a problem. For more information see: warn: v4l2-test-formats.cpp(836): http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html fail: v4l2-test-formats.cpp(420): pix_mp.reserved not zeroed fail: v4l2-test-formats.cpp(851): Video Capture Multiplanar is valid, but no S_FMT was implemented test VIDIOC_S_FMT: FAIL test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_
Re: [PATCH v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
Hi, On Friday 07 March 2014 06:29 PM, Hans Verkuil wrote: Do you think I can go ahead with posting the v3 patch set for 3.15, and work on fixing the compliance issue for the -rc fixes? It's fine to upstream this in staging, but while not all compliance errors are fixed it can't go to drivers/media. I'm tightening the screws on that since v4l2-compliance is getting to be such a powerful tool for ensuring the driver complies. But the vpe driver is already in drivers/media. How do I push these patches if the vpe drivers is not in staging? snip Multiplanar: TRY_FMT(G_FMT) != G_FMT test VIDIOC_TRY_FMT: FAIL warn: v4l2-test-formats.cpp(834): S_FMT cannot handle an invalid pixelformat. warn: v4l2-test-formats.cpp(835): This may or may not be a problem. For more information see: warn: v4l2-test-formats.cpp(836): http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html fail: v4l2-test-formats.cpp(420): pix_mp.reserved not zeroed This is easy enough to fix. fail: v4l2-test-formats.cpp(851): Video Capture Multiplanar is valid, but no S_FMT was implemented For the FMT things: run with -T: that gives nice traces. You can also set the debug flag: echo 2 /sys/class/video4linux/video0/debug to see all ioctls in more detail. Thanks for the tip, will try this. test VIDIOC_S_FMT: FAIL 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: info: test buftype Video Capture Multiplanar warn: v4l2-test-buffers.cpp(403): VIDIOC_CREATE_BUFS not supported info: test buftype Video Output Multiplanar warn: v4l2-test-buffers.cpp(403): VIDIOC_CREATE_BUFS not supported test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK (Not Supported) test read/write: OK (Not Supported) Video Capture Multiplanar (polling): Buffer: 0 Sequence: 0 Field: Top Timestamp: 113.178208s fail: v4l2-test-buffers.cpp(222): buf.field != cur_fmt.fmt.pix.field Definitely needs to be fixed, you probably just don't set the field at all. The VPE output is always progressive. But yes, I should still set the field parameter to something. Thanks, Archit -- 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 v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
On Friday 07 March 2014 07:02 PM, Hans Verkuil wrote: On 03/07/2014 02:22 PM, Archit Taneja wrote: Hi, On Friday 07 March 2014 06:29 PM, Hans Verkuil wrote: Do you think I can go ahead with posting the v3 patch set for 3.15, and work on fixing the compliance issue for the -rc fixes? It's fine to upstream this in staging, but while not all compliance errors are fixed it can't go to drivers/media. I'm tightening the screws on that since v4l2-compliance is getting to be such a powerful tool for ensuring the driver complies. But the vpe driver is already in drivers/media. How do I push these patches if the vpe drivers is not in staging? Oops, sorry. I got confused with Benoit's AM437x ti-vpfe patch :-) Disregard what I said, it's OK to upstream it. But if you could just spend some hours fixing the problems, that would really be best. Sure, I'll try to fix these issues and then post a v3. snip Multiplanar: TRY_FMT(G_FMT) != G_FMT test VIDIOC_TRY_FMT: FAIL warn: v4l2-test-formats.cpp(834): S_FMT cannot handle an invalid pixelformat. warn: v4l2-test-formats.cpp(835): This may or may not be a problem. For more information see: warn: v4l2-test-formats.cpp(836): http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html fail: v4l2-test-formats.cpp(420): pix_mp.reserved not zeroed This is easy enough to fix. fail: v4l2-test-formats.cpp(851): Video Capture Multiplanar is valid, but no S_FMT was implemented For the FMT things: run with -T: that gives nice traces. You can also set the debug flag: echo 2 /sys/class/video4linux/video0/debug to see all ioctls in more detail. Thanks for the tip, will try this. test VIDIOC_S_FMT: FAIL 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: info: test buftype Video Capture Multiplanar warn: v4l2-test-buffers.cpp(403): VIDIOC_CREATE_BUFS not supported info: test buftype Video Output Multiplanar warn: v4l2-test-buffers.cpp(403): VIDIOC_CREATE_BUFS not supported test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK (Not Supported) test read/write: OK (Not Supported) Video Capture Multiplanar (polling): Buffer: 0 Sequence: 0 Field: Top Timestamp: 113.178208s fail: v4l2-test-buffers.cpp(222): buf.field != cur_fmt.fmt.pix.field Definitely needs to be fixed, you probably just don't set the field at all. The VPE output is always progressive. But yes, I should still set the field parameter to something. V4L2_FIELD_NONE is the correct field setting for that. I checked the driver, it isn't setting it to V4L2_FIELD_NONE. Will fix this. Archit -- 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 7/7] v4l: ti-vpe: Add crop support in VPE driver
Hi, On Tuesday 04 March 2014 01:13 PM, Hans Verkuil wrote: On 03/04/2014 08:38 AM, Archit Taneja wrote: Hi Hans, On Monday 03 March 2014 01:20 PM, Hans Verkuil wrote: Hi Archit! On 03/03/2014 08:33 AM, Archit Taneja wrote: Add crop ioctl ops. For VPE, cropping only makes sense with the input to VPE, or the V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE buffer type. For the CAPTURE type, a S_CROP ioctl results in setting the crop region as the whole image itself, hence making crop dimensions same as the pix dimensions. Setting the crop successfully should result in re-configuration of those registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Some standard crop parameter checks are done in __vpe_try_crop(). Please use the selection ops instead: if you implement cropping with those then you'll support both the selection API and the old cropping API will be implemented by the v4l2 core using the selection ops. Two for the price of one... When using selection API, I was finding issues using the older cropping API. The v4l_s_crop() ioctl func assumes that crop means compose for output devices. However, for a m2m device. It probably makes sense to provide the following configuration: for V4L2_BUF_TYPE_VIDEO_OUTPUT (input to the mem to mem HW), use CROP target(to crop the input buffer) and, for V4L2_BUF_TYPE_VIDEO_CAPTURE(output of the mem to mem HW), use COMPOSE target(to place the HW output into a larger region) Don't you think forcing OUTPUT devices to 'COMPOSE' for older cropping API is a bit limiting? Yes, and that's why the selection API was created to work around that limitation :-) The old cropping API was insufficiently flexible for modern devices, so we came up with this replacement. Another reason why you have to implement the selection API: it's the only way to implement your functionality. Okay, I'll go ahead with the selection API then :) Archit -- 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 v2 0/7] v4l: ti-vpe: Some VPE fixes and enhancements
This patch set mainly consists of minor fixes for the VPE driver. These fixes ensure the following: - The VPE module can be inserted and removed successively. - Make sure that smaller resolutions like qcif work correctly. - Prevent race condition between firmware loading and an open call to the v4l2 device. - Prevent the possibility of output m2m queue not having sufficient 'ready' buffers. - Some VPDMA data descriptor fields weren't understood correctly before. They are now used correctly. The rest of the patches add some minor features like DMA buf support and cropping/composing. Reference branch: g...@github.com:boddob/linux.git vpe_for_315 Changes in v2: - selection API used instead of older cropping API. - Typo fix. - Some changes in patch 6/7 to support composing on the capture side of VPE. Archit Taneja (7): v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs v4l: ti-vpe: register video device only when firmware is loaded v4l: ti-vpe: Use video_device_release_empty v4l: ti-vpe: Allow DMABUF buffer type support v4l: ti-vpe: Allow usage of smaller images v4l: ti-vpe: Fix some params in VPE data descriptors v4l: ti-vpe: Add selection API in VPE driver drivers/media/platform/ti-vpe/vpdma.c | 68 +++--- drivers/media/platform/ti-vpe/vpdma.h | 17 +-- drivers/media/platform/ti-vpe/vpe.c | 228 +- 3 files changed, 255 insertions(+), 58 deletions(-) -- 1.8.3.2 -- 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 v2 4/7] v4l: ti-vpe: Allow DMABUF buffer type support
For OMAP and DRA7x, we generally allocate video and graphics buffers through omapdrm since the corresponding omap-gem driver provides DMM-Tiler backed contiguous buffers. omapdrm is a dma-buf exporter. These buffers are used by other drivers in the video pipeline. Add VB2_DMABUF flag to the io_modes of the vb2 output and capture queues. This allows the driver to import dma shared buffers. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index bb275f4..915029b 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1768,7 +1768,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(src_vq, 0, sizeof(*src_vq)); src_vq-type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; - src_vq-io_modes = VB2_MMAP; + src_vq-io_modes = VB2_MMAP | VB2_DMABUF; src_vq-drv_priv = ctx; src_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); src_vq-ops = vpe_qops; @@ -1781,7 +1781,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(dst_vq, 0, sizeof(*dst_vq)); dst_vq-type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; - dst_vq-io_modes = VB2_MMAP; + dst_vq-io_modes = VB2_MMAP | VB2_DMABUF; dst_vq-drv_priv = ctx; dst_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); dst_vq-ops = vpe_qops; -- 1.8.3.2 -- 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 v2 2/7] v4l: ti-vpe: register video device only when firmware is loaded
vpe fops(vpe_open in particular) should be called only when VPDMA firmware is loaded. File operations on the video device are possible the moment it is registered. Currently, we register the video device for VPE at driver probe, after calling a vpdma helper to initialize VPDMA and load firmware. This function is non-blocking(it calls request_firmware_nowait()), and doesn't ensure that the firmware is actually loaded when it returns. We remove the device registration from vpe probe, and move it to a callback provided by the vpe driver to the vpdma library, through vpdma_create(). The ready field in vpdma_data is no longer needed since we always have firmware loaded before the device is registered. A minor problem with this approach is that if the video_register_device fails(which doesn't really happen), the vpe platform device would be registered. however, there won't be any v4l2 device corresponding to it. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 8 +++-- drivers/media/platform/ti-vpe/vpdma.h | 7 +++-- drivers/media/platform/ti-vpe/vpe.c | 55 --- 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index e8175e7..73dd38e 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -781,7 +781,7 @@ static void vpdma_firmware_cb(const struct firmware *f, void *context) /* already initialized */ if (read_field_reg(vpdma, VPDMA_LIST_ATTR, VPDMA_LIST_RDY_MASK, VPDMA_LIST_RDY_SHFT)) { - vpdma-ready = true; + vpdma-cb(vpdma-pdev); return; } @@ -811,7 +811,7 @@ static void vpdma_firmware_cb(const struct firmware *f, void *context) goto free_buf; } - vpdma-ready = true; + vpdma-cb(vpdma-pdev); free_buf: vpdma_unmap_desc_buf(vpdma, fw_dma_buf); @@ -839,7 +839,8 @@ static int vpdma_load_firmware(struct vpdma_data *vpdma) return 0; } -struct vpdma_data *vpdma_create(struct platform_device *pdev) +struct vpdma_data *vpdma_create(struct platform_device *pdev, + void (*cb)(struct platform_device *pdev)) { struct resource *res; struct vpdma_data *vpdma; @@ -854,6 +855,7 @@ struct vpdma_data *vpdma_create(struct platform_device *pdev) } vpdma-pdev = pdev; + vpdma-cb = cb; res = platform_get_resource_byname(pdev, IORESOURCE_MEM, vpdma); if (res == NULL) { diff --git a/drivers/media/platform/ti-vpe/vpdma.h b/drivers/media/platform/ti-vpe/vpdma.h index cf40f11..bf5f8bb 100644 --- a/drivers/media/platform/ti-vpe/vpdma.h +++ b/drivers/media/platform/ti-vpe/vpdma.h @@ -35,8 +35,8 @@ struct vpdma_data { struct platform_device *pdev; - /* tells whether vpdma firmware is loaded or not */ - bool ready; + /* callback to VPE driver when the firmware is loaded */ + void (*cb)(struct platform_device *pdev); }; enum vpdma_data_format_type { @@ -208,6 +208,7 @@ void vpdma_set_frame_start_event(struct vpdma_data *vpdma, void vpdma_dump_regs(struct vpdma_data *vpdma); /* initialize vpdma, passed with VPE's platform device pointer */ -struct vpdma_data *vpdma_create(struct platform_device *pdev); +struct vpdma_data *vpdma_create(struct platform_device *pdev, + void (*cb)(struct platform_device *pdev)); #endif diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 623e59e..4243687 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1815,11 +1815,6 @@ static int vpe_open(struct file *file) vpe_dbg(dev, vpe_open\n); - if (!dev-vpdma-ready) { - vpe_err(dev, vpdma firmware not loaded\n); - return -ENODEV; - } - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) return -ENOMEM; @@ -2037,10 +2032,40 @@ static void vpe_runtime_put(struct platform_device *pdev) WARN_ON(r 0 r != -ENOSYS); } +static void vpe_fw_cb(struct platform_device *pdev) +{ + struct vpe_dev *dev = platform_get_drvdata(pdev); + struct video_device *vfd; + int ret; + + vfd = dev-vfd; + *vfd = vpe_videodev; + vfd-lock = dev-dev_mutex; + vfd-v4l2_dev = dev-v4l2_dev; + + ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0); + if (ret) { + vpe_err(dev, Failed to register video device\n); + + vpe_set_clock_enable(dev, 0); + vpe_runtime_put(pdev); + pm_runtime_disable(pdev-dev); + v4l2_m2m_release(dev-m2m_dev); + vb2_dma_contig_cleanup_ctx(dev-alloc_ctx); + v4l2_device_unregister(dev-v4l2_dev); + + return; + } + + video_set_drvdata
[PATCH v2 3/7] v4l: ti-vpe: Use video_device_release_empty
The video_device struct is currently embedded in the driver data struct vpe_dev. A vpe_dev instance is allocated by the driver, and the memory for the vfd is a part of this struct. The v4l2 core, however, manages the removal of the vfd region, through the video_device's .release() op, which currently is the helper video_device_release. This causes memory corruption, and leads to issues when we try to re-insert the vpe module. Use the video_device_release_empty helper function instead Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 4243687..bb275f4 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1998,7 +1998,7 @@ static struct video_device vpe_videodev = { .fops = vpe_fops, .ioctl_ops = vpe_ioctl_ops, .minor = -1, - .release= video_device_release, + .release= video_device_release_empty, .vfl_dir= VFL_DIR_M2M, }; -- 1.8.3.2 -- 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 v2 1/7] v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs
VPE has a ctrl parameter which decides how many mem to mem transactions the active job from the job queue can perform. The driver's job_ready() made sure that the number of ready source buffers are sufficient for the job to execute successfully. But it didn't make sure if there are sufficient ready destination buffers in the capture queue for the VPE output. If the time taken by VPE to process a single frame is really slow, then it's possible that we don't need to imply such a restriction on the dst queue, but really fast transactions(small resolution, no de-interlacing) may cause us to hit the condition where we don't have any free buffers for the VPE to write on. Add the extra check in job_ready() to make sure we have the sufficient amount of destination buffers. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 1296c53..623e59e 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -887,6 +887,9 @@ static int job_ready(void *priv) if (v4l2_m2m_num_src_bufs_ready(ctx-m2m_ctx) needed) return 0; + if (v4l2_m2m_num_dst_bufs_ready(ctx-m2m_ctx) needed) + return 0; + return 1; } -- 1.8.3.2 -- 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 v2 5/7] v4l: ti-vpe: Allow usage of smaller images
The minimum width and height for VPE input/output was kept as 128 pixels. VPE doesn't have a constraint on the image height, it requires the image width to be at least 16 bytes. Change the minimum supported dimensions to 32x32. This allows us to de-interlace qcif content. A smaller image size than 32x32 didn't make much sense, so stopped at this. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 915029b..3a610a6 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -49,8 +49,8 @@ #define VPE_MODULE_NAME vpe /* minimum and maximum frame sizes */ -#define MIN_W 128 -#define MIN_H 128 +#define MIN_W 32 +#define MIN_H 32 #define MAX_W 1920 #define MAX_H 1080 -- 1.8.3.2 -- 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 v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
Add selection ioctl ops. For VPE, cropping makes sense only for the input to VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers). For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP results in selecting a rectangle region within the source buffer. Setting the crop/compose rectangles should successfully result in re-configuration of registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 142 1 file changed, 142 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 03a6846..b938590 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx, { switch (type) { case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: + case V4L2_BUF_TYPE_VIDEO_OUTPUT: return ctx-q_data[Q_DATA_SRC]; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: + case V4L2_BUF_TYPE_VIDEO_CAPTURE: return ctx-q_data[Q_DATA_DST]; default: BUG(); @@ -1585,6 +1587,143 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f) return set_srcdst_params(ctx); } +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s) +{ + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + case V4L2_SEL_TGT_COMPOSE: + case V4L2_SEL_TGT_COMPOSE_DEFAULT: + case V4L2_SEL_TGT_COMPOSE_BOUNDS: + /* +* COMPOSE target is only valid for capture buffer type, for +* output buffer type, assign existing crop parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) { + break; + } else { + s-r = q_data-c_rect; + return 0; + } + + case V4L2_SEL_TGT_CROP: + case V4L2_SEL_TGT_CROP_DEFAULT: + case V4L2_SEL_TGT_CROP_BOUNDS: + /* +* CROP target is only valid for output buffer type, for capture +* buffer type, assign existing compose parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { + break; + } else { + s-r = q_data-c_rect; + return 0; + } + default: + return -EINVAL; + } + + if (s-r.top 0 || s-r.left 0) { + vpe_err(ctx-dev, negative values for top and left\n); + s-r.top = s-r.left = 0; + } + + v4l_bound_align_image(s-r.width, MIN_W, q_data-width, 1, + s-r.height, MIN_H, q_data-height, H_ALIGN, S_ALIGN); + + /* adjust left/top if cropping rectangle is out of bounds */ + if (s-r.left + s-r.width q_data-width) + s-r.left = q_data-width - s-r.width; + if (s-r.top + s-r.height q_data-height) + s-r.top = q_data-height - s-r.height; + + return 0; +} + +static int vpe_g_selection(struct file *file, void *fh, + struct v4l2_selection *s) +{ + struct vpe_ctx *ctx = file2ctx(file); + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + /* return width and height from S_FMT of the respective buffer type */ + case V4L2_SEL_TGT_COMPOSE_DEFAULT: + case V4L2_SEL_TGT_COMPOSE_BOUNDS: + case V4L2_SEL_TGT_CROP_BOUNDS: + case V4L2_SEL_TGT_CROP_DEFAULT: + s-r.left = 0; + s-r.top = 0; + s-r.width = q_data-width; + s-r.height = q_data-height; + return 0; + + /* +* CROP target holds for the output buffer type, and COMPOSE target +* holds for the capture buffer type. We still return the c_rect params +* for both the target types. +*/ + case V4L2_SEL_TGT_COMPOSE: + case V4L2_SEL_TGT_CROP: + s-r.left = q_data-c_rect.left; + s-r.top = q_data
[PATCH v2 6/7] v4l: ti-vpe: Fix some params in VPE data descriptors
Some parameters of the VPE descriptors were understood incorrectly. They are now fixed. The fixes are explained as follows: - When adding an inbound data descriptor to the VPDMA descriptor list, we intend to use c_rect as the cropped region fetched by VPDMA. Therefore, c_rect-width shouldn't be used to calculate the line stride, the original image width should be used for that. We add a 'width' argument which gives the buffer width in memory. - frame_width and frame_height describe the complete width and height of the client to which the channel is connected. If there are multiple channels fetching data and providing to the same client, the above 2 arguments should be the width and height of the region covered by all the channels. In the case where there is only one channel providing pixel data to the client (like in VPE), frame_width and frame_height should be the cropped width and cropped height respectively. The calculation of these params is done in the vpe driver now. - start_h and start_v is also used in the case of multiple channels to describe where each channel should start filling pixel data. We don't use this in VPE, and pass 0s to the vpdma_add_in_dtd() helper. - Some minor changes are made to the vpdma_add_out_dtd() helper. The c_rect param is used for specifying the 'composition' target, and 'width' is added to calculate the line stride. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 60 +++ drivers/media/platform/ti-vpe/vpdma.h | 10 +++--- drivers/media/platform/ti-vpe/vpe.c | 18 +++ 3 files changed, 64 insertions(+), 24 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index 73dd38e..a51a013 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -614,8 +614,17 @@ static void dump_dtd(struct vpdma_dtd *dtd) /* * append an outbound data transfer descriptor to the given descriptor list, * this sets up a 'client to memory' VPDMA transfer for the given VPDMA channel + * + * @list: vpdma desc list to which we add this decriptor + * @width: width of the image in pixels in memory + * @c_rect: compose params of output image + * @fmt: vpdma data format of the buffer + * dma_addr: dma address as seen by VPDMA + * chan: VPDMA channel + * flags: VPDMA flags to configure some descriptor fileds */ -void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, +void vpdma_add_out_dtd(struct vpdma_desc_list *list, int width, + const struct v4l2_rect *c_rect, const struct vpdma_data_format *fmt, dma_addr_t dma_addr, enum vpdma_channel chan, u32 flags) { @@ -623,6 +632,7 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, int field = 0; int notify = 1; int channel, next_chan; + struct v4l2_rect rect = *c_rect; int depth = fmt-depth; int stride; struct vpdma_dtd *dtd; @@ -630,11 +640,15 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, channel = next_chan = chan_info[chan].num; if (fmt-type == VPDMA_DATA_FMT_TYPE_YUV - fmt-data_type == DATA_TYPE_C420) + fmt-data_type == DATA_TYPE_C420) { + rect.height = 1; + rect.top = 1; depth = 8; + } - stride = ALIGN((depth * c_rect-width) 3, VPDMA_STRIDE_ALIGN); - dma_addr += (c_rect-left * depth) 3; + stride = ALIGN((depth * width) 3, VPDMA_STRIDE_ALIGN); + + dma_addr += rect.top * stride + (rect.left * depth 3); dtd = list-next; WARN_ON((void *)(dtd + 1) (list-buf.addr + list-buf.size)); @@ -664,31 +678,48 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, /* * append an inbound data transfer descriptor to the given descriptor list, * this sets up a 'memory to client' VPDMA transfer for the given VPDMA channel + * + * @list: vpdma desc list to which we add this decriptor + * @width: width of the image in pixels in memory(not the cropped width) + * @c_rect: crop params of input image + * @fmt: vpdma data format of the buffer + * dma_addr: dma address as seen by VPDMA + * chan: VPDMA channel + * field: top or bottom field info of the input image + * flags: VPDMA flags to configure some descriptor fileds + * frame_width/height: the complete width/height of the image presented to the + * client (this makes sense when multiple channels are + * connected to the same client, forming a larger frame) + * start_h, start_v: position where the given channel starts providing pixel + * data to the client (makes sense when multiple channels + * contribute to the client) */ -void vpdma_add_in_dtd(struct
Re: [PATCH v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
Hi, On Tuesday 04 March 2014 03:10 PM, Hans Verkuil wrote: Hi Archit, On 03/04/14 09:49, Archit Taneja wrote: Add selection ioctl ops. For VPE, cropping makes sense only for the input to VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers). For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP results in selecting a rectangle region within the source buffer. Setting the crop/compose rectangles should successfully result in re-configuration of registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 142 1 file changed, 142 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 03a6846..b938590 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx, { switch (type) { case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE: + case V4L2_BUF_TYPE_VIDEO_OUTPUT: return ctx-q_data[Q_DATA_SRC]; case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE: + case V4L2_BUF_TYPE_VIDEO_CAPTURE: I noticed that the querycap implementation is wrong. It reports V4L2_CAP_VIDEO_M2M instead of V4L2_CAP_VIDEO_M2M_MPLANE. This driver is using the multiplanar formats, so the M2M_MPLANE cap should be set. This should be a separate patch. Thanks for pointing this out, I'll make a patch for that. BTW, did you test the driver with the v4l2-compliance tool? The latest version (http://git.linuxtv.org/v4l-utils.git) has m2m support. I haven't tested it with this yet. However, if you want to test streaming (the -s option), then you will probably need to base your kernel on this tree: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/vb2-part1 I can give it a try. It'll probably take a bit more time to try this out. I'll need to port some minor DRA7x stuff. Kamil, Do you think you have some more time for the m2m pull request? That branch contains a pile of fixes for vb2 and without that v4l2-compliance -s will fail a number of tests. return ctx-q_data[Q_DATA_DST]; default: BUG(); @@ -1585,6 +1587,143 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f) return set_srcdst_params(ctx); } +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s) +{ + struct vpe_q_data *q_data; + + if ((s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT)) + return -EINVAL; + + q_data = get_q_data(ctx, s-type); + if (!q_data) + return -EINVAL; + + switch (s-target) { + case V4L2_SEL_TGT_COMPOSE: + case V4L2_SEL_TGT_COMPOSE_DEFAULT: + case V4L2_SEL_TGT_COMPOSE_BOUNDS: + /* +* COMPOSE target is only valid for capture buffer type, for +* output buffer type, assign existing crop parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_CAPTURE) { + break; + } else { No need for the 'else' keywork here. + s-r = q_data-c_rect; + return 0; + } + + case V4L2_SEL_TGT_CROP: + case V4L2_SEL_TGT_CROP_DEFAULT: + case V4L2_SEL_TGT_CROP_BOUNDS: + /* +* CROP target is only valid for output buffer type, for capture +* buffer type, assign existing compose parameters to the +* selection rectangle +*/ + if (s-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { + break; + } else { Ditto. Thanks. I'll fix these. I had a minor question about the selection API: Are the V4L2_SET_TGT_CROP/COMPOSE_DEFAULT and the corresponding 'BOUNDS' targets supposed to be used with VIDIOC_S_SELECTION? If so, what's the expect behaviour? Thanks, Archit -- 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 7/7] v4l: ti-vpe: Add crop support in VPE driver
Hi, On Monday 03 March 2014 01:20 PM, Hans Verkuil wrote: Hi Archit! On 03/03/2014 08:33 AM, Archit Taneja wrote: Add crop ioctl ops. For VPE, cropping only makes sense with the input to VPE, or the V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE buffer type. For the CAPTURE type, a S_CROP ioctl results in setting the crop region as the whole image itself, hence making crop dimensions same as the pix dimensions. Setting the crop successfully should result in re-configuration of those registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Some standard crop parameter checks are done in __vpe_try_crop(). Please use the selection ops instead: if you implement cropping with those then you'll support both the selection API and the old cropping API will be implemented by the v4l2 core using the selection ops. Two for the price of one... snip Thanks for the feedback. I'll use selection ops here. Archit -- 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 7/7] v4l: ti-vpe: Add crop support in VPE driver
Hi, On Monday 03 March 2014 05:51 PM, Kamil Debski wrote: Hi Archit, From: Archit Taneja [mailto:arc...@ti.com] Sent: Monday, March 03, 2014 9:26 AM Hi, On Monday 03 March 2014 01:20 PM, Hans Verkuil wrote: Hi Archit! On 03/03/2014 08:33 AM, Archit Taneja wrote: Add crop ioctl ops. For VPE, cropping only makes sense with the input to VPE, or the V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE buffer type. For the CAPTURE type, a S_CROP ioctl results in setting the crop region as the whole image itself, hence making crop dimensions same as the pix dimensions. Setting the crop successfully should result in re-configuration of those registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Some standard crop parameter checks are done in __vpe_try_crop(). Please use the selection ops instead: if you implement cropping with those then you'll support both the selection API and the old cropping API will be implemented by the v4l2 core using the selection ops. Two for the price of one... snip Thanks for the feedback. I'll use selection ops here. From your reply I understand that you will send a v2 of these patches, right? If so, please correct the typos I mentioned in the patch 5/7. Also, it is quite late for v3.15. I did already send a pull request to Mauro, However I have one patch pending. Could you tell me when are you planning to post v2 of these patches? I want to decide whether should I wait for your patchset or should I send the second pull request with the pending patch as soon as possible. I'll post a v2 of this set by tomorrow(India time). I have worked on a patch which converts it to selection ioctls, but I need to test it, and get some comments on whether I have implemented the selection ops correctly. Thanks, Archit -- 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 5/7] v4l: ti-vpe: Allow usage of smaller images
Hi, On Monday 03 March 2014 05:44 PM, Kamil Debski wrote: Hi Archit, From: Archit Taneja [mailto:arc...@ti.com] Sent: Monday, March 03, 2014 8:33 AM The minimum width and height for VPE input/output was kept as 128 pixels. VPE doesn't have a constraint on the image height, it requires the image width to be atleast 16 bytes. 16 bytes - shouldn't it be pixels? (also at least :) ) I can correct it when applying the patch if the above is the case. VPDMA IP requires the line stride of the input/output images to be at least 16 bytes in total. This can safely result in a minimum width of 16 pixels(NV12 has the least bpp with 1 byte per pixel). But we don't really have any need to support such small image resolutions. So I picked up 32x32 as the minimum size, and tested VPE with that. Thanks, Archit -- 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 7/7] v4l: ti-vpe: Add crop support in VPE driver
Hi Hans, On Monday 03 March 2014 01:20 PM, Hans Verkuil wrote: Hi Archit! On 03/03/2014 08:33 AM, Archit Taneja wrote: Add crop ioctl ops. For VPE, cropping only makes sense with the input to VPE, or the V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE buffer type. For the CAPTURE type, a S_CROP ioctl results in setting the crop region as the whole image itself, hence making crop dimensions same as the pix dimensions. Setting the crop successfully should result in re-configuration of those registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Some standard crop parameter checks are done in __vpe_try_crop(). Please use the selection ops instead: if you implement cropping with those then you'll support both the selection API and the old cropping API will be implemented by the v4l2 core using the selection ops. Two for the price of one... When using selection API, I was finding issues using the older cropping API. The v4l_s_crop() ioctl func assumes that crop means compose for output devices. However, for a m2m device. It probably makes sense to provide the following configuration: for V4L2_BUF_TYPE_VIDEO_OUTPUT (input to the mem to mem HW), use CROP target(to crop the input buffer) and, for V4L2_BUF_TYPE_VIDEO_CAPTURE(output of the mem to mem HW), use COMPOSE target(to place the HW output into a larger region) Don't you think forcing OUTPUT devices to 'COMPOSE' for older cropping API is a bit limiting? Thanks, Archit -- 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 3/7] v4l: ti-vpe: Use video_device_release_empty
The video_device struct is currently embedded in the driver data struct vpe_dev. A vpe_dev instance is allocated by the driver, and the memory for the vfd is a part of this struct. The v4l2 core, however, manages the removal of the vfd region, through the video_device's .release() op, which currently is the helper video_device_release. This causes memory corruption, and leads to issues when we try to re-insert the vpe module. Use the video_device_release_empty helper function instead Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 4243687..bb275f4 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1998,7 +1998,7 @@ static struct video_device vpe_videodev = { .fops = vpe_fops, .ioctl_ops = vpe_ioctl_ops, .minor = -1, - .release= video_device_release, + .release= video_device_release_empty, .vfl_dir= VFL_DIR_M2M, }; -- 1.8.3.2 -- 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 1/7] v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs
VPE has a ctrl parameter which decides how many mem to mem transactions the active job from the job queue can perform. The driver's job_ready() made sure that the number of ready source buffers are sufficient for the job to execute successfully. But it didn't make sure if there are sufficient ready destination buffers in the capture queue for the VPE output. If the time taken by VPE to process a single frame is really slow, then it's possible that we don't need to imply such a restriction on the dst queue, but really fast transactions(small resolution, no de-interlacing) may cause us to hit the condition where we don't have any free buffers for the VPE to write on. Add the extra check in job_ready() to make sure we have the sufficient amount of destination buffers. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 1296c53..623e59e 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -887,6 +887,9 @@ static int job_ready(void *priv) if (v4l2_m2m_num_src_bufs_ready(ctx-m2m_ctx) needed) return 0; + if (v4l2_m2m_num_dst_bufs_ready(ctx-m2m_ctx) needed) + return 0; + return 1; } -- 1.8.3.2 -- 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 4/7] v4l: ti-vpe: Allow DMABUF buffer type support
For OMAP and DRA7x, we generally allocate video and graphics buffers through omapdrm since the corresponding omap-gem driver provides DMM-Tiler backed contiguous buffers. omapdrm is a dma-buf exporter. These buffers are used by other drivers in the video pipeline. Add VB2_DMABUF flag to the io_modes of the vb2 output and capture queues. This allows the driver to import dma shared buffers. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index bb275f4..915029b 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1768,7 +1768,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(src_vq, 0, sizeof(*src_vq)); src_vq-type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; - src_vq-io_modes = VB2_MMAP; + src_vq-io_modes = VB2_MMAP | VB2_DMABUF; src_vq-drv_priv = ctx; src_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); src_vq-ops = vpe_qops; @@ -1781,7 +1781,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(dst_vq, 0, sizeof(*dst_vq)); dst_vq-type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; - dst_vq-io_modes = VB2_MMAP; + dst_vq-io_modes = VB2_MMAP | VB2_DMABUF; dst_vq-drv_priv = ctx; dst_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); dst_vq-ops = vpe_qops; -- 1.8.3.2 -- 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 5/7] v4l: ti-vpe: Allow usage of smaller images
The minimum width and height for VPE input/output was kept as 128 pixels. VPE doesn't have a constraint on the image height, it requires the image width to be atleast 16 bytes. Change the minimum supported dimensions to 32x32. This allows us to de-interlace qcif content. A smaller image size than 32x32 didn't make much sense, so stopped at this. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 915029b..3a610a6 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -49,8 +49,8 @@ #define VPE_MODULE_NAME vpe /* minimum and maximum frame sizes */ -#define MIN_W 128 -#define MIN_H 128 +#define MIN_W 32 +#define MIN_H 32 #define MAX_W 1920 #define MAX_H 1080 -- 1.8.3.2 -- 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 6/7] v4l: ti-vpe: Fix some params in VPE data descriptors
Some parameters of the VPE descriptors were understood incorrectly. They are now fixed. The fixes are explained as follows: - When adding an inbound data descriptor to the VPDMA descriptor list, we intend to use c_rect as the cropped region fetched by VPDMA. Therefore, c_rect-width shouldn't be used to calculate the line stride, the original image width should be used for that. We add a 'width' argument which gives the buffer width in memory. - frame_width and frame_height describe the complete width and height of the client to which the channel is connected. If there are multiple channels fetching data and providing to the same client, the above 2 arguments should be the width and height of the region covered by all the channels. In the case where there is only one channel providing pixel data to the client (like in VPE), frame_width and frame_height should be the cropped width and cropped height respectively. The calculation of these params is done in the vpe driver now. - start_h and start_v is also used in the case of multiple channels to describe where each channel should start filling pixel data. We don't use this in VPE, and pass 0s to the vpdma_add_in_dtd() helper. - Some minor changes are made to the vpdma_add_out_dtd() helper. The c_rect param is removed since cropping isn't possible for outbound data, and 'width' is added to calculate the line stride. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 50 ++- drivers/media/platform/ti-vpe/vpdma.h | 9 --- drivers/media/platform/ti-vpe/vpe.c | 16 +++ 3 files changed, 53 insertions(+), 22 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index 73dd38e..7248f16 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -614,8 +614,15 @@ static void dump_dtd(struct vpdma_dtd *dtd) /* * append an outbound data transfer descriptor to the given descriptor list, * this sets up a 'client to memory' VPDMA transfer for the given VPDMA channel + * + * @list: vpdma desc list to which we add this decriptor + * @width: width of the image in pixels in memory + * @fmt: vpdma data format of the buffer + * dma_addr: dma address as seen by VPDMA + * chan: VPDMA channel + * flags: VPDMA flags to configure some descriptor fileds */ -void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, +void vpdma_add_out_dtd(struct vpdma_desc_list *list, int width, const struct vpdma_data_format *fmt, dma_addr_t dma_addr, enum vpdma_channel chan, u32 flags) { @@ -633,8 +640,7 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, fmt-data_type == DATA_TYPE_C420) depth = 8; - stride = ALIGN((depth * c_rect-width) 3, VPDMA_STRIDE_ALIGN); - dma_addr += (c_rect-left * depth) 3; + stride = ALIGN((depth * width) 3, VPDMA_STRIDE_ALIGN); dtd = list-next; WARN_ON((void *)(dtd + 1) (list-buf.addr + list-buf.size)); @@ -664,31 +670,48 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, /* * append an inbound data transfer descriptor to the given descriptor list, * this sets up a 'memory to client' VPDMA transfer for the given VPDMA channel + * + * @list: vpdma desc list to which we add this decriptor + * @width: width of the image in pixels in memory(not the cropped width) + * @c_rect: crop params of input image + * @fmt: vpdma data format of the buffer + * dma_addr: dma address as seen by VPDMA + * chan: VPDMA channel + * field: top or bottom field info of the input image + * flags: VPDMA flags to configure some descriptor fileds + * frame_width/height: the complete width/height of the image presented to the + * client (this makes sense when multiple channels are + * connected to the same client, forming a larger frame) + * start_h, start_v: position where the given channel starts providing pixel + * data to the client (makes sense when multiple channels + * contribute to the client) */ -void vpdma_add_in_dtd(struct vpdma_desc_list *list, int frame_width, - int frame_height, struct v4l2_rect *c_rect, +void vpdma_add_in_dtd(struct vpdma_desc_list *list, int width, + const struct v4l2_rect *c_rect, const struct vpdma_data_format *fmt, dma_addr_t dma_addr, - enum vpdma_channel chan, int field, u32 flags) + enum vpdma_channel chan, int field, u32 flags, int frame_width, + int frame_height, int start_h, int start_v) { int priority = 0; int notify = 1; int depth = fmt-depth; int channel, next_chan; + struct v4l2_rect rect = *c_rect; int stride; - int height
[PATCH 2/7] v4l: ti-vpe: register video device only when firmware is loaded
vpe fops(vpe_open in particular) should be called only when VPDMA firmware is loaded. File operations on the video device are possible the moment it is registered. Currently, we register the video device for VPE at driver probe, after calling a vpdma helper to initialize VPDMA and load firmware. This function is non-blocking(it calls request_firmware_nowait()), and doesn't ensure that the firmware is actually loaded when it returns. We remove the device registration from vpe probe, and move it to a callback provided by the vpe driver to the vpdma library, through vpdma_create(). The ready field in vpdma_data is no longer needed since we always have firmware loaded before the device is registered. A minor problem with this approach is that if the video_register_device fails(which doesn't really happen), the vpe platform device would be registered. however, there won't be any v4l2 device corresponding to it. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 8 +++-- drivers/media/platform/ti-vpe/vpdma.h | 7 +++-- drivers/media/platform/ti-vpe/vpe.c | 55 --- 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index e8175e7..73dd38e 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -781,7 +781,7 @@ static void vpdma_firmware_cb(const struct firmware *f, void *context) /* already initialized */ if (read_field_reg(vpdma, VPDMA_LIST_ATTR, VPDMA_LIST_RDY_MASK, VPDMA_LIST_RDY_SHFT)) { - vpdma-ready = true; + vpdma-cb(vpdma-pdev); return; } @@ -811,7 +811,7 @@ static void vpdma_firmware_cb(const struct firmware *f, void *context) goto free_buf; } - vpdma-ready = true; + vpdma-cb(vpdma-pdev); free_buf: vpdma_unmap_desc_buf(vpdma, fw_dma_buf); @@ -839,7 +839,8 @@ static int vpdma_load_firmware(struct vpdma_data *vpdma) return 0; } -struct vpdma_data *vpdma_create(struct platform_device *pdev) +struct vpdma_data *vpdma_create(struct platform_device *pdev, + void (*cb)(struct platform_device *pdev)) { struct resource *res; struct vpdma_data *vpdma; @@ -854,6 +855,7 @@ struct vpdma_data *vpdma_create(struct platform_device *pdev) } vpdma-pdev = pdev; + vpdma-cb = cb; res = platform_get_resource_byname(pdev, IORESOURCE_MEM, vpdma); if (res == NULL) { diff --git a/drivers/media/platform/ti-vpe/vpdma.h b/drivers/media/platform/ti-vpe/vpdma.h index cf40f11..bf5f8bb 100644 --- a/drivers/media/platform/ti-vpe/vpdma.h +++ b/drivers/media/platform/ti-vpe/vpdma.h @@ -35,8 +35,8 @@ struct vpdma_data { struct platform_device *pdev; - /* tells whether vpdma firmware is loaded or not */ - bool ready; + /* callback to VPE driver when the firmware is loaded */ + void (*cb)(struct platform_device *pdev); }; enum vpdma_data_format_type { @@ -208,6 +208,7 @@ void vpdma_set_frame_start_event(struct vpdma_data *vpdma, void vpdma_dump_regs(struct vpdma_data *vpdma); /* initialize vpdma, passed with VPE's platform device pointer */ -struct vpdma_data *vpdma_create(struct platform_device *pdev); +struct vpdma_data *vpdma_create(struct platform_device *pdev, + void (*cb)(struct platform_device *pdev)); #endif diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 623e59e..4243687 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1815,11 +1815,6 @@ static int vpe_open(struct file *file) vpe_dbg(dev, vpe_open\n); - if (!dev-vpdma-ready) { - vpe_err(dev, vpdma firmware not loaded\n); - return -ENODEV; - } - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) return -ENOMEM; @@ -2037,10 +2032,40 @@ static void vpe_runtime_put(struct platform_device *pdev) WARN_ON(r 0 r != -ENOSYS); } +static void vpe_fw_cb(struct platform_device *pdev) +{ + struct vpe_dev *dev = platform_get_drvdata(pdev); + struct video_device *vfd; + int ret; + + vfd = dev-vfd; + *vfd = vpe_videodev; + vfd-lock = dev-dev_mutex; + vfd-v4l2_dev = dev-v4l2_dev; + + ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0); + if (ret) { + vpe_err(dev, Failed to register video device\n); + + vpe_set_clock_enable(dev, 0); + vpe_runtime_put(pdev); + pm_runtime_disable(pdev-dev); + v4l2_m2m_release(dev-m2m_dev); + vb2_dma_contig_cleanup_ctx(dev-alloc_ctx); + v4l2_device_unregister(dev-v4l2_dev); + + return; + } + + video_set_drvdata
[PATCH 7/7] v4l: ti-vpe: Add crop support in VPE driver
Add crop ioctl ops. For VPE, cropping only makes sense with the input to VPE, or the V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE buffer type. For the CAPTURE type, a S_CROP ioctl results in setting the crop region as the whole image itself, hence making crop dimensions same as the pix dimensions. Setting the crop successfully should result in re-configuration of those registers which are affected when either source or destination dimensions change, set_srcdst_params() is called for this purpose. Some standard crop parameter checks are done in __vpe_try_crop(). Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 96 + 1 file changed, 96 insertions(+) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 6acdcd8..c6778f4 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1585,6 +1585,98 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f) return set_srcdst_params(ctx); } +static int __vpe_try_crop(struct vpe_ctx *ctx, struct v4l2_crop *cr) +{ + struct vpe_q_data *q_data; + + q_data = get_q_data(ctx, cr-type); + if (!q_data) + return -EINVAL; + + /* we don't support crop on capture plane */ + if (cr-type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { + cr-c.top = cr-c.left = 0; + cr-c.width = q_data-width; + cr-c.height = q_data-height; + return 0; + } + + if (cr-c.top 0 || cr-c.left 0) { + vpe_err(ctx-dev, negative values for top and left\n); + cr-c.top = cr-c.left = 0; + } + + v4l_bound_align_image(cr-c.width, MIN_W, q_data-width, 1, + cr-c.height, MIN_H, q_data-height, H_ALIGN, S_ALIGN); + + /* adjust left/top if cropping rectangle is out of bounds */ + if (cr-c.left + cr-c.width q_data-width) + cr-c.left = q_data-width - cr-c.width; + if (cr-c.top + cr-c.height q_data-height) + cr-c.top = q_data-height - cr-c.height; + + return 0; +} + +static int vpe_cropcap(struct file *file, void *priv, struct v4l2_cropcap *cr) +{ + struct vpe_ctx *ctx = file2ctx(file); + struct vpe_q_data *q_data; + + q_data = get_q_data(ctx, cr-type); + if (!q_data) + return -EINVAL; + + cr-bounds.left = 0; + cr-bounds.top = 0; + cr-bounds.width = q_data-width; + cr-bounds.height = q_data-height; + cr-defrect = cr-bounds; + + return 0; +} + +static int vpe_g_crop(struct file *file, void *fh, struct v4l2_crop *cr) +{ + struct vpe_ctx *ctx = file2ctx(file); + struct vpe_q_data *q_data; + + q_data = get_q_data(ctx, cr-type); + if (!q_data) + return -EINVAL; + + cr-c = q_data-c_rect; + + return 0; +} + +static int vpe_s_crop(struct file *file, void *priv, + const struct v4l2_crop *crop) +{ + struct vpe_ctx *ctx = file2ctx(file); + struct vpe_q_data *q_data; + struct v4l2_crop cr = *crop; + int ret; + + ret = __vpe_try_crop(ctx, cr); + if (ret) + return ret; + + q_data = get_q_data(ctx, cr.type); + + if ((q_data-c_rect.left == cr.c.left) + (q_data-c_rect.top == cr.c.top) + (q_data-c_rect.width == cr.c.width) + (q_data-c_rect.height == cr.c.height)) { + vpe_dbg(ctx-dev, requested crop values are already set\n); + return 0; + } + + q_data-c_rect = cr.c; + + return set_srcdst_params(ctx); +} + static int vpe_reqbufs(struct file *file, void *priv, struct v4l2_requestbuffers *reqbufs) { @@ -1672,6 +1764,10 @@ static const struct v4l2_ioctl_ops vpe_ioctl_ops = { .vidioc_try_fmt_vid_out_mplane = vpe_try_fmt, .vidioc_s_fmt_vid_out_mplane= vpe_s_fmt, + .vidioc_cropcap = vpe_cropcap, + .vidioc_g_crop = vpe_g_crop, + .vidioc_s_crop = vpe_s_crop, + .vidioc_reqbufs = vpe_reqbufs, .vidioc_querybuf= vpe_querybuf, -- 1.8.3.2 -- 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 0/7] v4l: ti-vpe: Some VPE fixes and enhancements
This patch set mainly consists of minor fixes for the VPE driver. These fixes ensure the following: - The VPE module can be inserted and removed successively. - Make sure that smaller resolutions like qcif work correctly. - Prevent race condition between firmware loading and an open call to the v4l2 device. - Prevent the possibility of output m2m queue not having sufficient 'ready' buffers. - Some VPDMA data descriptor fields weren't understood correctly before. They are now used correctly. The rest of the patches add some minor features like DMA buf support and cropping. Reference branch: g...@github.com:boddob/linux.git vpe_for_315 Archit Taneja (7): v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs v4l: ti-vpe: register video device only when firmware is loaded v4l: ti-vpe: Use video_device_release_empty v4l: ti-vpe: Allow DMABUF buffer type support v4l: ti-vpe: Allow usage of smaller images v4l: ti-vpe: Fix some params in VPE data descriptors v4l: ti-vpe: Add crop support in VPE driver drivers/media/platform/ti-vpe/vpdma.c | 58 --- drivers/media/platform/ti-vpe/vpdma.h | 16 +-- drivers/media/platform/ti-vpe/vpe.c | 180 +++--- 3 files changed, 198 insertions(+), 56 deletions(-) -- 1.8.3.2 -- 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 0/8] v4l: ti-vpe: Add support for scaling and color conversion
Hi Hans, On Tuesday 17 December 2013 01:30 PM, Hans Verkuil wrote: On 12/12/2013 09:35 AM, Archit Taneja wrote: The VPE and VIP IPs in DRA7x contain common scaler and color conversion hardware blocks. We create libraries for these components such that the vpe driver and the vip driver(in future) can use these library funcs to configure the blocks. There are some points for which I would like comments: - For VPE, setting the format and colorspace for the source and destination queues is enough to determine how these blocks need to be configured and whether they need to be bypassed or not. So it didn't make sense to represent them as media controller entities. For VIP(driver not upstream yet), it's possible that there are multiple data paths which may or may not include these blocks. However, the current use cases don't require such flexibility. There may be a need to re-consider a media controller like setup once we work on the VIP driver. Is it a good idea in terms of user-space compatibilty if we use media controller framework in the future. As long as you don't need the mc, then there is no need to implement it. The thing is that we want to use these blocks for a future capture hardware called VIP. A VIP port can capture multiple streams from different sensors in time slices, and each of those streams might be sharing the same hardware, but probably in different configurations. I'm not completely aware of media controller details, and whether it can help us here. I was just wondering if it would be a problem if we later realise that mc might be useful for some use cases. Would implementing it then break previous user space applications which don't call mc ioctls? - These blocks will also require some custom control commands later on. For example, we may want to tell the scaler later on to perform bi-linear scaling, or perform peaking at a particular frequency. - The current series keeps the default scaler coefficients in a header file. These coefficients add a lot of lines of code in the kernel. Does it make more sense for the user application to pass the co-efficients to the kernel using an ioctl? Is there any driver which currenlty does this? I think it is good to keep it in the driver. Otherwise apps would be forced to set up the table. It's about 11 kilobyte in memory, which isn't that bad. Okay. The series is based on the branch: git://linuxtv.org/media_tree.git master Archit Taneja (8): v4l: ti-vpe: create a scaler block library v4l: ti-vpe: support loading of scaler coefficients v4l: ti-vpe: make vpe driver load scaler coefficients v4l: ti-vpe: enable basic scaler support v4l: ti-vpe: create a color space converter block library v4l: ti-vpe: Add helper to perform color conversion v4l: ti-vpe: enable CSC support for VPE v4l: ti-vpe: Add a type specifier to describe vpdma data format type drivers/media/platform/ti-vpe/Makefile |2 +- drivers/media/platform/ti-vpe/csc.c | 196 + drivers/media/platform/ti-vpe/csc.h | 68 ++ drivers/media/platform/ti-vpe/sc.c | 311 +++ drivers/media/platform/ti-vpe/sc.h | 208 + drivers/media/platform/ti-vpe/sc_coeff.h | 1342 ++ drivers/media/platform/ti-vpe/vpdma.c| 36 +- drivers/media/platform/ti-vpe/vpdma.h|7 + drivers/media/platform/ti-vpe/vpe.c | 251 -- drivers/media/platform/ti-vpe/vpe_regs.h | 187 - 10 files changed, 2335 insertions(+), 273 deletions(-) create mode 100644 drivers/media/platform/ti-vpe/csc.c create mode 100644 drivers/media/platform/ti-vpe/csc.h create mode 100644 drivers/media/platform/ti-vpe/sc.c create mode 100644 drivers/media/platform/ti-vpe/sc.h create mode 100644 drivers/media/platform/ti-vpe/sc_coeff.h For this patch series: Acked-by: Hans Verkuil hans.verk...@cisco.com Thanks for the review. Archit -- 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 0/8] v4l: ti-vpe: Add support for scaling and color conversion
Hi, On Tuesday 17 December 2013 05:19 PM, Hans Verkuil wrote: On 12/17/13 12:19, Archit Taneja wrote: Hi Hans, On Tuesday 17 December 2013 01:30 PM, Hans Verkuil wrote: On 12/12/2013 09:35 AM, Archit Taneja wrote: The VPE and VIP IPs in DRA7x contain common scaler and color conversion hardware blocks. We create libraries for these components such that the vpe driver and the vip driver(in future) can use these library funcs to configure the blocks. There are some points for which I would like comments: - For VPE, setting the format and colorspace for the source and destination queues is enough to determine how these blocks need to be configured and whether they need to be bypassed or not. So it didn't make sense to represent them as media controller entities. For VIP(driver not upstream yet), it's possible that there are multiple data paths which may or may not include these blocks. However, the current use cases don't require such flexibility. There may be a need to re-consider a media controller like setup once we work on the VIP driver. Is it a good idea in terms of user-space compatibilty if we use media controller framework in the future. As long as you don't need the mc, then there is no need to implement it. The thing is that we want to use these blocks for a future capture hardware called VIP. A VIP port can capture multiple streams from different sensors in time slices, and each of those streams might be sharing the same hardware, but probably in different configurations. I'm not completely aware of media controller details, and whether it can help us here. I was just wondering if it would be a problem if we later realise that mc might be useful for some use cases. Would implementing it then break previous user space applications which don't call mc ioctls? My understanding is that in the current vpe driver the mc won't be needed, so there is no point adding it. When implementing the vip capture driver the mc might be needed, but that should not require the vpe to add the mc API as well. It's a decision that has to be made per driver. That's right, vpe doesn't need mc. It might be needed for vip. The reason I brought it up now is because some of the blocks like SC/CSC are replicated in VIP hardware, and I created them in a 'library' sort of way in this series. If vip driver uses mc, these blocks might need to become media entities. When you start work on the vip driver it is probably a good idea to talk to Laurent and myself first to see whether the mc is needed or not. Thanks, that'll be quite useful. I'll look up some mc documentation and drivers using mc myself as well. If you have a block diagram of the video hardware that you can share, then that will be quite useful. Thanks for the clarification. I don't think the DRA7x documentation is public yet. I'll try to look for a block diagram(or create one) and share it with the list. Archit -- 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 7/8] v4l: ti-vpe: enable CSC support for VPE
Use the csc library functions to configure the CSC block in VPE. Some changes are required in try_fmt to handle the pix-colorspace parameter more correctly. Previously, we copied the source queue colorspace to the destination queue colorspace as we didn't support RGB formats. Now, we configure pix-colorspace based on the color format set(and the height of the image if it's a YUV format). Add basic RGB color formats to the list of supported vpe formats. If the destination format is RGB colorspace, we also need to use the RGB output port instead of the Luma and Chroma output ports. This requires configuring the output data descriptors differently. Also, make the default colorspace V4L2_COLORSPACE_SMPTE170M as that resembles the Standard Definition colorspace more closely. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 95 - 1 file changed, 72 insertions(+), 23 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 6c4db57..1296c53 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -267,6 +267,38 @@ static struct vpe_fmt vpe_formats[] = { .vpdma_fmt = { vpdma_yuv_fmts[VPDMA_DATA_FMT_CY422], }, }, + { + .name = RGB888 packed, + .fourcc = V4L2_PIX_FMT_RGB24, + .types = VPE_FMT_TYPE_CAPTURE, + .coplanar = 0, + .vpdma_fmt = { vpdma_rgb_fmts[VPDMA_DATA_FMT_RGB24], + }, + }, + { + .name = ARGB32, + .fourcc = V4L2_PIX_FMT_RGB32, + .types = VPE_FMT_TYPE_CAPTURE, + .coplanar = 0, + .vpdma_fmt = { vpdma_rgb_fmts[VPDMA_DATA_FMT_ARGB32], + }, + }, + { + .name = BGR888 packed, + .fourcc = V4L2_PIX_FMT_BGR24, + .types = VPE_FMT_TYPE_CAPTURE, + .coplanar = 0, + .vpdma_fmt = { vpdma_rgb_fmts[VPDMA_DATA_FMT_BGR24], + }, + }, + { + .name = ABGR32, + .fourcc = V4L2_PIX_FMT_BGR32, + .types = VPE_FMT_TYPE_CAPTURE, + .coplanar = 0, + .vpdma_fmt = { vpdma_rgb_fmts[VPDMA_DATA_FMT_ABGR32], + }, + }, }; /* @@ -689,17 +721,20 @@ static void set_src_registers(struct vpe_ctx *ctx) static void set_dst_registers(struct vpe_ctx *ctx) { struct vpe_mmr_adb *mmr_adb = ctx-mmr_adb.addr; + enum v4l2_colorspace clrspc = ctx-q_data[Q_DATA_DST].colorspace; struct vpe_fmt *fmt = ctx-q_data[Q_DATA_DST].fmt; u32 val = 0; - /* select RGB path when color space conversion is supported in future */ - if (fmt-fourcc == V4L2_PIX_FMT_RGB24) - val |= VPE_RGB_OUT_SELECT | VPE_CSC_SRC_DEI_SCALER; + if (clrspc == V4L2_COLORSPACE_SRGB) + val |= VPE_RGB_OUT_SELECT; else if (fmt-fourcc == V4L2_PIX_FMT_NV16) val |= VPE_COLOR_SEPARATE_422; - /* The source of CHR_DS is always the scaler, whether it's used or not */ - val |= VPE_DS_SRC_DEI_SCALER; + /* +* the source of CHR_DS and CSC is always the scaler, irrespective of +* whether it's used or not +*/ + val |= VPE_DS_SRC_DEI_SCALER | VPE_CSC_SRC_DEI_SCALER; if (fmt-fourcc != V4L2_PIX_FMT_NV12) val |= VPE_DS_BYPASS; @@ -813,7 +848,8 @@ static int set_srcdst_params(struct vpe_ctx *ctx) set_cfg_and_line_modes(ctx); set_dei_regs(ctx); - csc_set_coeff_bypass(ctx-dev-csc, mmr_adb-csc_regs[5]); + csc_set_coeff(ctx-dev-csc, mmr_adb-csc_regs[0], + s_q_data-colorspace, d_q_data-colorspace); sc_set_hs_coeffs(ctx-dev-sc, ctx-sc_coeff_h.addr, src_w, dst_w); sc_set_vs_coeffs(ctx-dev-sc, ctx-sc_coeff_v.addr, src_h, dst_h); @@ -1095,9 +1131,13 @@ static void device_run(void *priv) if (ctx-deinterlacing) add_out_dtd(ctx, VPE_PORT_MV_OUT); - add_out_dtd(ctx, VPE_PORT_LUMA_OUT); - if (d_q_data-fmt-coplanar) - add_out_dtd(ctx, VPE_PORT_CHROMA_OUT); + if (d_q_data-colorspace == V4L2_COLORSPACE_SRGB) { + add_out_dtd(ctx, VPE_PORT_RGB_OUT); + } else { + add_out_dtd(ctx, VPE_PORT_LUMA_OUT); + if (d_q_data-fmt-coplanar) + add_out_dtd(ctx, VPE_PORT_CHROMA_OUT); + } /* input data descriptors */ if (ctx-deinterlacing) { @@ -1133,9 +1173,16 @@ static void device_run(void *priv) } /* sync on channel control
[PATCH 1/8] v4l: ti-vpe: create a scaler block library
VPE and VIP IPs in DAR7x contain a scaler(SC) sub block. Create a library which will perform scaler block related configurations and hold SC register definitions. The functions provided by this library will be called by the vpe and vip drivers using a sc_data handle. The vpe_dev holds the sc_data handle. The handle represents an instance of the SC hardware, and the vpe driver uses it to access the scaler register offsets or helper functions to configure these registers. We move the SC register definitions to sc.h so that they aren't specific to VPE anymore. The register offsets are now relative to the sub-block, and not the VPE IP as a whole. In order for VPDMA to configure registers, it requires it's offset from the top level VPE module. A macro called GET_OFFSET_TOP is added to return the offset of the register relative to the VPE IP. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/Makefile | 2 +- drivers/media/platform/ti-vpe/sc.c | 91 drivers/media/platform/ti-vpe/sc.h | 175 +++ drivers/media/platform/ti-vpe/vpe.c | 60 --- drivers/media/platform/ti-vpe/vpe_regs.h | 149 -- 5 files changed, 288 insertions(+), 189 deletions(-) create mode 100644 drivers/media/platform/ti-vpe/sc.c create mode 100644 drivers/media/platform/ti-vpe/sc.h diff --git a/drivers/media/platform/ti-vpe/Makefile b/drivers/media/platform/ti-vpe/Makefile index cbf0a80..54c30b3 100644 --- a/drivers/media/platform/ti-vpe/Makefile +++ b/drivers/media/platform/ti-vpe/Makefile @@ -1,5 +1,5 @@ obj-$(CONFIG_VIDEO_TI_VPE) += ti-vpe.o -ti-vpe-y := vpe.o vpdma.o +ti-vpe-y := vpe.o sc.o vpdma.o ccflags-$(CONFIG_VIDEO_TI_VPE_DEBUG) += -DDEBUG diff --git a/drivers/media/platform/ti-vpe/sc.c b/drivers/media/platform/ti-vpe/sc.c new file mode 100644 index 000..f21dfbb --- /dev/null +++ b/drivers/media/platform/ti-vpe/sc.c @@ -0,0 +1,91 @@ +/* + * Scaler library + * + * Copyright (c) 2013 Texas Instruments Inc. + * + * David Griego, dagri...@biglakesoftware.com + * Dale Farnsworth, d...@farnsworth.org + * Archit Taneja, arc...@ti.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. + */ + +#include linux/err.h +#include linux/io.h +#include linux/platform_device.h +#include linux/slab.h + +#include sc.h + +void sc_set_regs_bypass(struct sc_data *sc, u32 *sc_reg0) +{ + *sc_reg0 |= CFG_SC_BYPASS; +} + +void sc_dump_regs(struct sc_data *sc) +{ + struct device *dev = sc-pdev-dev; + + u32 read_reg(struct sc_data *sc, int offset) + { + return ioread32(sc-base + offset); + } + +#define DUMPREG(r) dev_dbg(dev, %-35s %08x\n, #r, read_reg(sc, CFG_##r)) + + DUMPREG(SC0); + DUMPREG(SC1); + DUMPREG(SC2); + DUMPREG(SC3); + DUMPREG(SC4); + DUMPREG(SC5); + DUMPREG(SC6); + DUMPREG(SC8); + DUMPREG(SC9); + DUMPREG(SC10); + DUMPREG(SC11); + DUMPREG(SC12); + DUMPREG(SC13); + DUMPREG(SC17); + DUMPREG(SC18); + DUMPREG(SC19); + DUMPREG(SC20); + DUMPREG(SC21); + DUMPREG(SC22); + DUMPREG(SC23); + DUMPREG(SC24); + DUMPREG(SC25); + +#undef DUMPREG +} + +struct sc_data *sc_create(struct platform_device *pdev) +{ + struct sc_data *sc; + + dev_dbg(pdev-dev, sc_create\n); + + sc = devm_kzalloc(pdev-dev, sizeof(*sc), GFP_KERNEL); + if (!sc) { + dev_err(pdev-dev, couldn't alloc sc_data\n); + return ERR_PTR(-ENOMEM); + } + + sc-pdev = pdev; + + sc-res = platform_get_resource_byname(pdev, IORESOURCE_MEM, sc); + if (!sc-res) { + dev_err(pdev-dev, missing platform resources data\n); + return ERR_PTR(-ENODEV); + } + + sc-base = devm_ioremap_resource(pdev-dev, sc-res); + if (!sc-base) { + dev_err(pdev-dev, failed to ioremap\n); + return ERR_PTR(-ENOMEM); + } + + return sc; +} diff --git a/drivers/media/platform/ti-vpe/sc.h b/drivers/media/platform/ti-vpe/sc.h new file mode 100644 index 000..9248544 --- /dev/null +++ b/drivers/media/platform/ti-vpe/sc.h @@ -0,0 +1,175 @@ +/* + * Copyright (c) 2013 Texas Instruments Inc. + * + * David Griego, dagri...@biglakesoftware.com + * Dale Farnsworth, d...@farnsworth.org + * Archit Taneja, arc...@ti.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 TI_SC_H +#define TI_SC_H + +/* Scaler regs */ +#define CFG_SC00x0 +#define CFG_INTERLACE_O(1 0) +#define CFG_LINEAR (1 1) +#define
[PATCH 5/8] v4l: ti-vpe: create a color space converter block library
VPE and VIP IPs in DAR7x contain a color space converter(CSC) sub block. Create a library which will perform CSC related configurations and hold CSC register definitions. The functions provided by this library will be called by the vpe and vip drivers using a csc_data handle. The vpe_dev holds the csc_data handle. The handle represents an instance of the CSC hardware, and the vpe driver uses it to access the CSC register offsets or helper functions to configure these registers. The CSC register offsets are now relative to the CSC block itself, so we need to use the macro GET_OFFSET_TOP to get the CSC register offset relative to the VPE IP in the vpe driver. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/Makefile | 2 +- drivers/media/platform/ti-vpe/csc.c | 76 drivers/media/platform/ti-vpe/csc.h | 65 +++ drivers/media/platform/ti-vpe/vpe.c | 31 ++--- drivers/media/platform/ti-vpe/vpe_regs.h | 38 5 files changed, 155 insertions(+), 57 deletions(-) create mode 100644 drivers/media/platform/ti-vpe/csc.c create mode 100644 drivers/media/platform/ti-vpe/csc.h diff --git a/drivers/media/platform/ti-vpe/Makefile b/drivers/media/platform/ti-vpe/Makefile index 54c30b3..be680f8 100644 --- a/drivers/media/platform/ti-vpe/Makefile +++ b/drivers/media/platform/ti-vpe/Makefile @@ -1,5 +1,5 @@ obj-$(CONFIG_VIDEO_TI_VPE) += ti-vpe.o -ti-vpe-y := vpe.o sc.o vpdma.o +ti-vpe-y := vpe.o sc.o csc.o vpdma.o ccflags-$(CONFIG_VIDEO_TI_VPE_DEBUG) += -DDEBUG diff --git a/drivers/media/platform/ti-vpe/csc.c b/drivers/media/platform/ti-vpe/csc.c new file mode 100644 index 000..62e2fec --- /dev/null +++ b/drivers/media/platform/ti-vpe/csc.c @@ -0,0 +1,76 @@ +/* + * Color space converter library + * + * Copyright (c) 2013 Texas Instruments Inc. + * + * David Griego, dagri...@biglakesoftware.com + * Dale Farnsworth, d...@farnsworth.org + * Archit Taneja, arc...@ti.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. + */ + +#include linux/err.h +#include linux/io.h +#include linux/platform_device.h +#include linux/slab.h + +#include csc.h + +void csc_dump_regs(struct csc_data *csc) +{ + struct device *dev = csc-pdev-dev; + + u32 read_reg(struct csc_data *csc, int offset) + { + return ioread32(csc-base + offset); + } + +#define DUMPREG(r) dev_dbg(dev, %-35s %08x\n, #r, read_reg(csc, CSC_##r)) + + DUMPREG(CSC00); + DUMPREG(CSC01); + DUMPREG(CSC02); + DUMPREG(CSC03); + DUMPREG(CSC04); + DUMPREG(CSC05); + +#undef DUMPREG +} + +void csc_set_coeff_bypass(struct csc_data *csc, u32 *csc_reg5) +{ + *csc_reg5 |= CSC_BYPASS; +} + +struct csc_data *csc_create(struct platform_device *pdev) +{ + struct csc_data *csc; + + dev_dbg(pdev-dev, csc_create\n); + + csc = devm_kzalloc(pdev-dev, sizeof(*csc), GFP_KERNEL); + if (!csc) { + dev_err(pdev-dev, couldn't alloc csc_data\n); + return ERR_PTR(-ENOMEM); + } + + csc-pdev = pdev; + + csc-res = platform_get_resource_byname(pdev, IORESOURCE_MEM, + vpe_csc); + if (csc-res == NULL) { + dev_err(pdev-dev, missing platform resources data\n); + return ERR_PTR(-ENODEV); + } + + csc-base = devm_ioremap_resource(pdev-dev, csc-res); + if (!csc-base) { + dev_err(pdev-dev, failed to ioremap\n); + return ERR_PTR(-ENOMEM); + } + + return csc; +} diff --git a/drivers/media/platform/ti-vpe/csc.h b/drivers/media/platform/ti-vpe/csc.h new file mode 100644 index 000..57b5ed6 --- /dev/null +++ b/drivers/media/platform/ti-vpe/csc.h @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2013 Texas Instruments Inc. + * + * David Griego, dagri...@biglakesoftware.com + * Dale Farnsworth, d...@farnsworth.org + * Archit Taneja, arc...@ti.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 TI_CSC_H +#define TI_CSC_H + +/* VPE color space converter regs */ +#define CSC_CSC00 0x00 +#define CSC_A0_MASK0x1fff +#define CSC_A0_SHIFT 0 +#define CSC_B0_MASK0x1fff +#define CSC_B0_SHIFT 16 + +#define CSC_CSC01 0x04 +#define CSC_C0_MASK0x1fff +#define CSC_C0_SHIFT 0 +#define CSC_A1_MASK0x1fff +#define CSC_A1_SHIFT 16 + +#define CSC_CSC02 0x08 +#define CSC_B1_MASK0x1fff +#define CSC_B1_SHIFT 0 +#define CSC_C1_MASK0x1fff +#define CSC_C1_SHIFT 16 + +#define CSC_CSC03
[PATCH 8/8] v4l: ti-vpe: Add a type specifier to describe vpdma data format type
The struct vpdma_data_format holds the color format depth and the data_type value needed to be programmed in the data descriptors. However, it doesn't tell what type of color format is it, i.e, whether it is RGB, YUV or Misc. This information is needed when by vpdma library when forming descriptors. We modify the depth parameter for the chroma portion of the NV12 format. For this, we check if the data_type value is C420. This isn't sufficient as there are many YUV and RGB vpdma formats which have the same data_type value. Hence, we need to hold the type of the color format for the above case, and possibly more cases in the future. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 36 +-- drivers/media/platform/ti-vpe/vpdma.h | 7 +++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index f97253f..a268f68 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -30,38 +30,47 @@ const struct vpdma_data_format vpdma_yuv_fmts[] = { [VPDMA_DATA_FMT_Y444] = { + .type = VPDMA_DATA_FMT_TYPE_YUV, .data_type = DATA_TYPE_Y444, .depth = 8, }, [VPDMA_DATA_FMT_Y422] = { + .type = VPDMA_DATA_FMT_TYPE_YUV, .data_type = DATA_TYPE_Y422, .depth = 8, }, [VPDMA_DATA_FMT_Y420] = { + .type = VPDMA_DATA_FMT_TYPE_YUV, .data_type = DATA_TYPE_Y420, .depth = 8, }, [VPDMA_DATA_FMT_C444] = { + .type = VPDMA_DATA_FMT_TYPE_YUV, .data_type = DATA_TYPE_C444, .depth = 8, }, [VPDMA_DATA_FMT_C422] = { + .type = VPDMA_DATA_FMT_TYPE_YUV, .data_type = DATA_TYPE_C422, .depth = 8, }, [VPDMA_DATA_FMT_C420] = { + .type = VPDMA_DATA_FMT_TYPE_YUV, .data_type = DATA_TYPE_C420, .depth = 4, }, [VPDMA_DATA_FMT_YC422] = { + .type = VPDMA_DATA_FMT_TYPE_YUV, .data_type = DATA_TYPE_YC422, .depth = 16, }, [VPDMA_DATA_FMT_YC444] = { + .type = VPDMA_DATA_FMT_TYPE_YUV, .data_type = DATA_TYPE_YC444, .depth = 24, }, [VPDMA_DATA_FMT_CY422] = { + .type = VPDMA_DATA_FMT_TYPE_YUV, .data_type = DATA_TYPE_CY422, .depth = 16, }, @@ -69,82 +78,102 @@ const struct vpdma_data_format vpdma_yuv_fmts[] = { const struct vpdma_data_format vpdma_rgb_fmts[] = { [VPDMA_DATA_FMT_RGB565] = { + .type = VPDMA_DATA_FMT_TYPE_RGB, .data_type = DATA_TYPE_RGB16_565, .depth = 16, }, [VPDMA_DATA_FMT_ARGB16_1555] = { + .type = VPDMA_DATA_FMT_TYPE_RGB, .data_type = DATA_TYPE_ARGB_1555, .depth = 16, }, [VPDMA_DATA_FMT_ARGB16] = { + .type = VPDMA_DATA_FMT_TYPE_RGB, .data_type = DATA_TYPE_ARGB_, .depth = 16, }, [VPDMA_DATA_FMT_RGBA16_5551] = { + .type = VPDMA_DATA_FMT_TYPE_RGB, .data_type = DATA_TYPE_RGBA_5551, .depth = 16, }, [VPDMA_DATA_FMT_RGBA16] = { + .type = VPDMA_DATA_FMT_TYPE_RGB, .data_type = DATA_TYPE_RGBA_, .depth = 16, }, [VPDMA_DATA_FMT_ARGB24] = { + .type = VPDMA_DATA_FMT_TYPE_RGB, .data_type = DATA_TYPE_ARGB24_, .depth = 24, }, [VPDMA_DATA_FMT_RGB24] = { + .type = VPDMA_DATA_FMT_TYPE_RGB, .data_type = DATA_TYPE_RGB24_888, .depth = 24, }, [VPDMA_DATA_FMT_ARGB32] = { + .type = VPDMA_DATA_FMT_TYPE_RGB, .data_type = DATA_TYPE_ARGB32_, .depth = 32, }, [VPDMA_DATA_FMT_RGBA24] = { + .type = VPDMA_DATA_FMT_TYPE_RGB, .data_type = DATA_TYPE_RGBA24_, .depth = 24, }, [VPDMA_DATA_FMT_RGBA32] = { + .type = VPDMA_DATA_FMT_TYPE_RGB, .data_type = DATA_TYPE_RGBA32_, .depth
[PATCH 4/8] v4l: ti-vpe: enable basic scaler support
Add the required SC register configurations which lets us perform linear scaling for the supported range of horizontal and vertical scaling ratios. The horizontal scaler performs polyphase scaling using it's 8 tap 32 phase filter, decimation is performed when downscaling passes beyond 2x or 4x. The vertical scaler performs polyphase scaling using it's 5 tap 32 phase filter, it switches to a simpler form of scaling using the running average filter when the downscale ratio is more than 4x. Many of the SC features like peaking, trimming and non-linear scaling aren't implemented for now. Only the minimal register fields required for basic scaling operation are configured. The function to configure SC registers takes the sc_data handle, the source and destination widths and heights, and the scaler address data block offsets for the current context so that they can be configured. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/sc.c | 132 ++-- drivers/media/platform/ti-vpe/sc.h | 4 +- drivers/media/platform/ti-vpe/vpe.c | 24 +-- 3 files changed, 149 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/ti-vpe/sc.c b/drivers/media/platform/ti-vpe/sc.c index 417feb9..93f0af54 100644 --- a/drivers/media/platform/ti-vpe/sc.c +++ b/drivers/media/platform/ti-vpe/sc.c @@ -20,11 +20,6 @@ #include sc.h #include sc_coeff.h -void sc_set_regs_bypass(struct sc_data *sc, u32 *sc_reg0) -{ - *sc_reg0 |= CFG_SC_BYPASS; -} - void sc_dump_regs(struct sc_data *sc) { struct device *dev = sc-pdev-dev; @@ -159,6 +154,133 @@ void sc_set_vs_coeffs(struct sc_data *sc, void *addr, unsigned int src_h, sc-load_coeff_v = true; } +void sc_config_scaler(struct sc_data *sc, u32 *sc_reg0, u32 *sc_reg8, + u32 *sc_reg17, unsigned int src_w, unsigned int src_h, + unsigned int dst_w, unsigned int dst_h) +{ + struct device *dev = sc-pdev-dev; + u32 val; + int dcm_x, dcm_shift; + bool use_rav; + unsigned long lltmp; + u32 lin_acc_inc, lin_acc_inc_u; + u32 col_acc_offset; + u16 factor = 0; + int row_acc_init_rav = 0, row_acc_init_rav_b = 0; + u32 row_acc_inc = 0, row_acc_offset = 0, row_acc_offset_b = 0; + /* +* location of SC register in payload memory with respect to the first +* register in the mmr address data block +*/ + u32 *sc_reg9 = sc_reg8 + 1; + u32 *sc_reg12 = sc_reg8 + 4; + u32 *sc_reg13 = sc_reg8 + 5; + u32 *sc_reg24 = sc_reg17 + 7; + + val = sc_reg0[0]; + + /* clear all the features(they may get enabled elsewhere later) */ + val = ~(CFG_SELFGEN_FID | CFG_TRIM | CFG_ENABLE_SIN2_VER_INTP | + CFG_INTERLACE_I | CFG_DCM_4X | CFG_DCM_2X | CFG_AUTO_HS | + CFG_ENABLE_EV | CFG_USE_RAV | CFG_INVT_FID | CFG_SC_BYPASS | + CFG_INTERLACE_O | CFG_Y_PK_EN | CFG_HP_BYPASS | CFG_LINEAR); + + if (src_w == dst_w src_h == dst_h) { + val |= CFG_SC_BYPASS; + sc_reg0[0] = val; + return; + } + + /* we only support linear scaling for now */ + val |= CFG_LINEAR; + + /* configure horizontal scaler */ + + /* enable 2X or 4X decimation */ + dcm_x = src_w / dst_w; + if (dcm_x 4) { + val |= CFG_DCM_4X; + dcm_shift = 2; + } else if (dcm_x 2) { + val |= CFG_DCM_2X; + dcm_shift = 1; + } else { + dcm_shift = 0; + } + + lltmp = dst_w - 1; + lin_acc_inc = div64_u64(((u64)(src_w dcm_shift) - 1) 24, lltmp); + lin_acc_inc_u = 0; + col_acc_offset = 0; + + dev_dbg(dev, hs config: src_w = %d, dst_w = %d, decimation = %s, lin_acc_inc = %08x\n, + src_w, dst_w, dcm_shift == 2 ? 4x : + (dcm_shift == 1 ? 2x : none), lin_acc_inc); + + /* configure vertical scaler */ + + /* use RAV for vertical scaler if vertical downscaling is 4x */ + if (dst_h (src_h 2)) { + use_rav = true; + val |= CFG_USE_RAV; + } else { + use_rav = false; + } + + if (use_rav) { + /* use RAV */ + factor = (u16) ((dst_h 10) / src_h); + + row_acc_init_rav = factor + ((1 + factor) 1); + if (row_acc_init_rav = 1024) + row_acc_init_rav -= 1024; + + row_acc_init_rav_b = row_acc_init_rav + + (1 + (row_acc_init_rav 1)) - + (1024 1); + + if (row_acc_init_rav_b 0) { + row_acc_init_rav_b += row_acc_init_rav; + row_acc_init_rav *= 2; + } + + dev_dbg(dev, vs config(RAV): src_h = %d, dst_h = %d, factor = %d, acc_init = %08x, acc_init_b = %08x\n
[PATCH 0/8] v4l: ti-vpe: Add support for scaling and color conversion
The VPE and VIP IPs in DRA7x contain common scaler and color conversion hardware blocks. We create libraries for these components such that the vpe driver and the vip driver(in future) can use these library funcs to configure the blocks. There are some points for which I would like comments: - For VPE, setting the format and colorspace for the source and destination queues is enough to determine how these blocks need to be configured and whether they need to be bypassed or not. So it didn't make sense to represent them as media controller entities. For VIP(driver not upstream yet), it's possible that there are multiple data paths which may or may not include these blocks. However, the current use cases don't require such flexibility. There may be a need to re-consider a media controller like setup once we work on the VIP driver. Is it a good idea in terms of user-space compatibilty if we use media controller framework in the future. - These blocks will also require some custom control commands later on. For example, we may want to tell the scaler later on to perform bi-linear scaling, or perform peaking at a particular frequency. - The current series keeps the default scaler coefficients in a header file. These coefficients add a lot of lines of code in the kernel. Does it make more sense for the user application to pass the co-efficients to the kernel using an ioctl? Is there any driver which currenlty does this? The series is based on the branch: git://linuxtv.org/media_tree.git master Archit Taneja (8): v4l: ti-vpe: create a scaler block library v4l: ti-vpe: support loading of scaler coefficients v4l: ti-vpe: make vpe driver load scaler coefficients v4l: ti-vpe: enable basic scaler support v4l: ti-vpe: create a color space converter block library v4l: ti-vpe: Add helper to perform color conversion v4l: ti-vpe: enable CSC support for VPE v4l: ti-vpe: Add a type specifier to describe vpdma data format type drivers/media/platform/ti-vpe/Makefile |2 +- drivers/media/platform/ti-vpe/csc.c | 196 + drivers/media/platform/ti-vpe/csc.h | 68 ++ drivers/media/platform/ti-vpe/sc.c | 311 +++ drivers/media/platform/ti-vpe/sc.h | 208 + drivers/media/platform/ti-vpe/sc_coeff.h | 1342 ++ drivers/media/platform/ti-vpe/vpdma.c| 36 +- drivers/media/platform/ti-vpe/vpdma.h|7 + drivers/media/platform/ti-vpe/vpe.c | 251 -- drivers/media/platform/ti-vpe/vpe_regs.h | 187 - 10 files changed, 2335 insertions(+), 273 deletions(-) create mode 100644 drivers/media/platform/ti-vpe/csc.c create mode 100644 drivers/media/platform/ti-vpe/csc.h create mode 100644 drivers/media/platform/ti-vpe/sc.c create mode 100644 drivers/media/platform/ti-vpe/sc.h create mode 100644 drivers/media/platform/ti-vpe/sc_coeff.h -- 1.8.3.2 -- 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 3/8] v4l: ti-vpe: make vpe driver load scaler coefficients
Make the driver allocate dma buffers to store horizontal and scaler coeffs. Use the scaler library api to choose and copy scaler coefficients to a the above buffers based on the scaling ratio. Since the SC block comes after the de-interlacer, make sure that the source height is doubled if de-interlacer was used. These buffers now need to be used by VPDMA to load the coefficients into the SRAM within SC. In device_run, add configuration descriptors which have payloads pointing to the scaler coefficients in memory. Use the members in sc_data handle to prevent addition of these descriptors if there isn't a need to re-load coefficients into SC. This comes helps unnecessary re-loading of the coefficients when we switch back and forth between vpe contexts. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpe.c | 47 - 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index ecb85f9..50d6d0e 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -356,6 +356,8 @@ struct vpe_ctx { void*mv_buf[2]; /* virtual addrs of motion vector bufs */ size_t mv_buf_size;/* current motion vector buffer size */ struct vpdma_bufmmr_adb;/* shadow reg addr/data block */ + struct vpdma_bufsc_coeff_h; /* h coeff buffer */ + struct vpdma_bufsc_coeff_v; /* v coeff buffer */ struct vpdma_desc_list desc_list; /* DMA descriptor list */ booldeinterlacing; /* using de-interlacer */ @@ -765,6 +767,10 @@ static int set_srcdst_params(struct vpe_ctx *ctx) struct vpe_q_data *s_q_data = ctx-q_data[Q_DATA_SRC]; struct vpe_q_data *d_q_data = ctx-q_data[Q_DATA_DST]; struct vpe_mmr_adb *mmr_adb = ctx-mmr_adb.addr; + unsigned int src_w = s_q_data-c_rect.width; + unsigned int src_h = s_q_data-c_rect.height; + unsigned int dst_w = d_q_data-c_rect.width; + unsigned int dst_h = d_q_data-c_rect.height; size_t mv_buf_size; int ret; @@ -777,7 +783,6 @@ static int set_srcdst_params(struct vpe_ctx *ctx) const struct vpdma_data_format *mv = vpdma_misc_fmts[VPDMA_DATA_FMT_MV]; - ctx-deinterlacing = 1; /* * we make sure that the source image has a 16 byte aligned * stride, we need to do the same for the motion vector buffer @@ -788,6 +793,9 @@ static int set_srcdst_params(struct vpe_ctx *ctx) bytes_per_line = ALIGN((s_q_data-width * mv-depth) 3, VPDMA_STRIDE_ALIGN); mv_buf_size = bytes_per_line * s_q_data-height; + + ctx-deinterlacing = 1; + src_h = 1; } else { ctx-deinterlacing = 0; mv_buf_size = 0; @@ -802,6 +810,8 @@ static int set_srcdst_params(struct vpe_ctx *ctx) set_cfg_and_line_modes(ctx); set_dei_regs(ctx); set_csc_coeff_bypass(ctx); + sc_set_hs_coeffs(ctx-dev-sc, ctx-sc_coeff_h.addr, src_w, dst_w); + sc_set_vs_coeffs(ctx-dev-sc, ctx-sc_coeff_v.addr, src_h, dst_h); sc_set_regs_bypass(ctx-dev-sc, mmr_adb-sc_regs[0]); return 0; @@ -1035,6 +1045,7 @@ static void disable_irqs(struct vpe_ctx *ctx) static void device_run(void *priv) { struct vpe_ctx *ctx = priv; + struct sc_data *sc = ctx-dev-sc; struct vpe_q_data *d_q_data = ctx-q_data[Q_DATA_DST]; if (ctx-deinterlacing ctx-src_vbs[2] == NULL) { @@ -1057,6 +1068,26 @@ static void device_run(void *priv) ctx-load_mmrs = false; } + if (sc-loaded_coeff_h != ctx-sc_coeff_h.dma_addr || + sc-load_coeff_h) { + vpdma_map_desc_buf(ctx-dev-vpdma, ctx-sc_coeff_h); + vpdma_add_cfd_block(ctx-desc_list, CFD_SC_CLIENT, + ctx-sc_coeff_h, 0); + + sc-loaded_coeff_h = ctx-sc_coeff_h.dma_addr; + sc-load_coeff_h = false; + } + + if (sc-loaded_coeff_v != ctx-sc_coeff_v.dma_addr || + sc-load_coeff_v) { + vpdma_map_desc_buf(ctx-dev-vpdma, ctx-sc_coeff_v); + vpdma_add_cfd_block(ctx-desc_list, CFD_SC_CLIENT, + ctx-sc_coeff_v, SC_COEF_SRAM_SIZE 4); + + sc-loaded_coeff_v = ctx-sc_coeff_v.dma_addr; + sc-load_coeff_v = false; + } + /* output data descriptors */ if (ctx-deinterlacing) add_out_dtd(ctx, VPE_PORT_MV_OUT); @@ -1180,6 +1211,8 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data) vpdma_unmap_desc_buf(dev-vpdma, ctx-desc_list.buf
[PATCH 6/8] v4l: ti-vpe: Add helper to perform color conversion
The CSC block can be used for color space conversion between YUV and RGB formats. It is configurable via a programmable set of coefficients. Add functionality to choose the appropriate CSC coefficients and program them in the CSC registers. We take the source and destination colorspace formats as the arguments, and choose the coefficient table accordingly. YUV to RGB coefficients are provided for standard and high definition colorspaces. The coefficients can also be limited or full range. For now, only full range coefficients are chosen. We would need some sort of control ioctl for the user to specify the range needed. Not sure if there is a generic control ioctl for this already? Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/csc.c | 120 drivers/media/platform/ti-vpe/csc.h | 3 + 2 files changed, 123 insertions(+) diff --git a/drivers/media/platform/ti-vpe/csc.c b/drivers/media/platform/ti-vpe/csc.c index 62e2fec..acfea50 100644 --- a/drivers/media/platform/ti-vpe/csc.c +++ b/drivers/media/platform/ti-vpe/csc.c @@ -16,9 +16,79 @@ #include linux/io.h #include linux/platform_device.h #include linux/slab.h +#include linux/videodev2.h #include csc.h +/* + * 16 coefficients in the order: + * a0, b0, c0, a1, b1, c1, a2, b2, c2, d0, d1, d2 + * (we may need to pass non-default values from user space later on, we might + * need to make the coefficient struct more easy to populate) + */ +struct colorspace_coeffs { + u16 sd[12]; + u16 hd[12]; +}; + +/* VIDEO_RANGE: limited range, GRAPHICS_RANGE: full range */ +#defineCSC_COEFFS_VIDEO_RANGE_Y2R 0 +#defineCSC_COEFFS_GRAPHICS_RANGE_Y2R 1 +#defineCSC_COEFFS_VIDEO_RANGE_R2Y 2 +#defineCSC_COEFFS_GRAPHICS_RANGE_R2Y 3 + +/* default colorspace coefficients */ +static struct colorspace_coeffs colorspace_coeffs[4] = { + [CSC_COEFFS_VIDEO_RANGE_Y2R] = { + { + /* SDTV */ + 0x0400, 0x, 0x057D, 0x0400, 0x1EA7, 0x1D35, + 0x0400, 0x06EF, 0x1FFE, 0x0D40, 0x0210, 0x0C88, + }, + { + /* HDTV */ + 0x0400, 0x, 0x0629, 0x0400, 0x1F45, 0x1E2B, + 0x0400, 0x0742, 0x, 0x0CEC, 0x0148, 0x0C60, + }, + }, + [CSC_COEFFS_GRAPHICS_RANGE_Y2R] = { + { + /* SDTV */ + 0x04A8, 0x1FFE, 0x0662, 0x04A8, 0x1E6F, 0x1CBF, + 0x04A8, 0x0812, 0x1FFF, 0x0C84, 0x0220, 0x0BAC, + }, + { + /* HDTV */ + 0x04A8, 0x, 0x072C, 0x04A8, 0x1F26, 0x1DDE, + 0x04A8, 0x0873, 0x, 0x0C20, 0x0134, 0x0B7C, + }, + }, + [CSC_COEFFS_VIDEO_RANGE_R2Y] = { + { + /* SDTV */ + 0x0132, 0x0259, 0x0075, 0x1F50, 0x1EA5, 0x020B, + 0x020B, 0x1E4A, 0x1FAB, 0x, 0x0200, 0x0200, + }, + { + /* HDTV */ + 0x00DA, 0x02DC, 0x004A, 0x1F88, 0x1E6C, 0x020C, + 0x020C, 0x1E24, 0x1FD0, 0x, 0x0200, 0x0200, + }, + }, + [CSC_COEFFS_GRAPHICS_RANGE_R2Y] = { + { + /* SDTV */ + 0x0107, 0x0204, 0x0064, 0x1F68, 0x1ED6, 0x01C2, + 0x01C2, 0x1E87, 0x1FB7, 0x0040, 0x0200, 0x0200, + }, + { + /* HDTV */ + 0x04A8, 0x, 0x072C, 0x04A8, 0x1F26, 0x1DDE, + 0x04A8, 0x0873, 0x, 0x0C20, 0x0134, 0x0B7C, + }, + }, +}; + void csc_dump_regs(struct csc_data *csc) { struct device *dev = csc-pdev-dev; @@ -45,6 +115,56 @@ void csc_set_coeff_bypass(struct csc_data *csc, u32 *csc_reg5) *csc_reg5 |= CSC_BYPASS; } +/* + * set the color space converter coefficient shadow register values + */ +void csc_set_coeff(struct csc_data *csc, u32 *csc_reg0, + enum v4l2_colorspace src_colorspace, + enum v4l2_colorspace dst_colorspace) +{ + u32 *csc_reg5 = csc_reg0 + 5; + u32 *shadow_csc = csc_reg0; + struct colorspace_coeffs *sd_hd_coeffs; + u16 *coeff, *end_coeff; + enum v4l2_colorspace yuv_colorspace; + int sel = 0; + + /* +* support only graphics data range(full range) for now, a control ioctl +* would be nice here +*/ + /* Y2R */ + if (dst_colorspace == V4L2_COLORSPACE_SRGB + (src_colorspace == V4L2_COLORSPACE_SMPTE170M || + src_colorspace == V4L2_COLORSPACE_REC709)) { + /* Y2R */ + sel = 1; + yuv_colorspace = src_colorspace
Re: [PATCH 6/6] experimental: arm: dts: dra7xx: Add a DT node for VPE
Hi Laurent, On Friday 09 August 2013 03:41 AM, Laurent Pinchart wrote: Hi Archit, Thank you for the patch. On Friday 02 August 2013 19:33:43 Archit Taneja wrote: Add a DT node for VPE in dra7.dtsi. This is experimental because we might need to split the VPE address space a bit more, and also because the IRQ line described is accessible the IRQ crossbar driver is added for DRA7XX. Cc: Rajendra Nayak rna...@ti.com Cc: Sricharan R r.sricha...@ti.com Signed-off-by: Archit Taneja arc...@ti.com --- arch/arm/boot/dts/dra7.dtsi | 11 +++ Documentation is missing :-) As this is an experimental patch you can probably document the bindings later. Sorry for the late reply, I missed out on reading this message somehow. I'm blocked on adding the DT nodes because of some dependencies on dra7x clocks, and the crossbar framework. I'll make sure to add documentation :) 1 file changed, 11 insertions(+) diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi index ce9a0f0..3237972 100644 --- a/arch/arm/boot/dts/dra7.dtsi +++ b/arch/arm/boot/dts/dra7.dtsi @@ -484,6 +484,17 @@ dmas = sdma 70, sdma 71; dma-names = tx0, rx0; }; + + vpe { + compatible = ti,vpe; + ti,hwmods = vpe; + reg = 0x489d 0xd000, 0x489dd000 0x400; + reg-names = vpe, vpdma; + interrupts = 0 159 0x4; + #address-cells = 1; + #size-cells = 0; Are #address-cells and #size-cells really needed ? They aren't needed. The vpe node inherits these params from the parent ocp node. Thanks, Archit -- 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 2/2] v4l: ti-vpe: make sure VPDMA line stride constraints are met
When VPDMA fetches or writes to an image buffer, the line stride must be a multiple of 16 bytes. If it isn't, VPDMA HW will write/fetch until the next 16 byte boundry. This causes VPE to work incorrectly for source or destination widths which don't satisfy the above alignment requirement. In order to prevent this, we now make sure that when we set pix format for the input and output buffers, the VPE source and destination image line strides are 16 byte aligned. Also, the motion vector buffers for the de-interlacer are allocated in such a way that it ensures the same alignment. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma.c | 4 +-- drivers/media/platform/ti-vpe/vpdma.h | 5 +++- drivers/media/platform/ti-vpe/vpe.c | 53 ++- 3 files changed, 46 insertions(+), 16 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c index af0a5ff..f97253f 100644 --- a/drivers/media/platform/ti-vpe/vpdma.c +++ b/drivers/media/platform/ti-vpe/vpdma.c @@ -602,7 +602,7 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect, if (fmt-data_type == DATA_TYPE_C420) depth = 8; - stride = (depth * c_rect-width) 3; + stride = ALIGN((depth * c_rect-width) 3, VPDMA_STRIDE_ALIGN); dma_addr += (c_rect-left * depth) 3; dtd = list-next; @@ -655,7 +655,7 @@ void vpdma_add_in_dtd(struct vpdma_desc_list *list, int frame_width, depth = 8; } - stride = (depth * c_rect-width) 3; + stride = ALIGN((depth * c_rect-width) 3, VPDMA_STRIDE_ALIGN); dma_addr += (c_rect-left * depth) 3; dtd = list-next; diff --git a/drivers/media/platform/ti-vpe/vpdma.h b/drivers/media/platform/ti-vpe/vpdma.h index eaa2a71..62dd143 100644 --- a/drivers/media/platform/ti-vpe/vpdma.h +++ b/drivers/media/platform/ti-vpe/vpdma.h @@ -45,7 +45,10 @@ struct vpdma_data_format { }; #define VPDMA_DESC_ALIGN 16 /* 16-byte descriptor alignment */ - +#define VPDMA_STRIDE_ALIGN 16 /* +* line stride of source and dest +* buffers should be 16 byte aligned +*/ #define VPDMA_DTD_DESC_SIZE32 /* 8 words */ #define VPDMA_CFD_CTD_DESC_SIZE16 /* 4 words */ diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 4e58069..a5f7a35 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -30,6 +30,7 @@ #include linux/sched.h #include linux/slab.h #include linux/videodev2.h +#include linux/log2.h #include media/v4l2-common.h #include media/v4l2-ctrls.h @@ -54,10 +55,6 @@ /* required alignments */ #define S_ALIGN0 /* multiple of 1 */ #define H_ALIGN1 /* multiple of 2 */ -#define W_ALIGN1 /* multiple of 2 */ - -/* multiple of 128 bits, line stride, 16 bytes */ -#define L_ALIGN4 /* flags that indicate a format can be used for capture/output */ #define VPE_FMT_TYPE_CAPTURE (1 0) @@ -780,12 +777,21 @@ static int set_srcdst_params(struct vpe_ctx *ctx) if ((s_q_data-flags Q_DATA_INTERLACED) !(d_q_data-flags Q_DATA_INTERLACED)) { + int bytes_per_line; const struct vpdma_data_format *mv = vpdma_misc_fmts[VPDMA_DATA_FMT_MV]; ctx-deinterlacing = 1; - mv_buf_size = - (s_q_data-width * s_q_data-height * mv-depth) 3; + /* +* we make sure that the source image has a 16 byte aligned +* stride, we need to do the same for the motion vector buffer +* by aligning it's stride to the next 16 byte boundry. this +* extra space will not be used by the de-interlacer, but will +* ensure that vpdma operates correctly +*/ + bytes_per_line = ALIGN((s_q_data-width * mv-depth) 3, + VPDMA_STRIDE_ALIGN); + mv_buf_size = bytes_per_line * s_q_data-height; } else { ctx-deinterlacing = 0; mv_buf_size = 0; @@ -1352,7 +1358,8 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, { struct v4l2_pix_format_mplane *pix = f-fmt.pix_mp; struct v4l2_plane_pix_format *plane_fmt; - int i; + unsigned int w_align; + int i, depth, depth_bytes; if (!fmt || !(fmt-types type)) { vpe_err(ctx-dev, Fourcc format (0x%08x) invalid.\n, @@ -1363,7 +1370,31 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f, if (pix-field
[PATCH 0/2] v4l: ti-vpe: Some VPE fixes
This series fixes 2 issues in the VPE driver. The first fix allows us to use UYVY color format for source and destination buffers. The second fix makes sure we don't set pixel format widths which the VPDMA HW can't support. None of these fixes are fatal, so they don't necessarily need to go in for the 3.13-rc fixes. Archit Taneja (2): v4l: ti-vpe: Fix the data_type value for UYVY VPDMA format v4l: ti-vpe: make sure VPDMA line stride constraints are met drivers/media/platform/ti-vpe/vpdma.c | 4 +-- drivers/media/platform/ti-vpe/vpdma.h | 5 ++- drivers/media/platform/ti-vpe/vpdma_priv.h | 2 +- drivers/media/platform/ti-vpe/vpe.c| 53 ++ 4 files changed, 47 insertions(+), 17 deletions(-) -- 1.8.3.2 -- 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 1/2] v4l: ti-vpe: Fix the data_type value for UYVY VPDMA format
The data_type value to be programmed in the data descriptors to fetch/write a UYVY buffer was not mentioned correctly in the older DRA7x documentation. This caused VPE to fail with UYVY color formats. Update the data_type value to fix functionality when UYVY format is used. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/platform/ti-vpe/vpdma_priv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/ti-vpe/vpdma_priv.h b/drivers/media/platform/ti-vpe/vpdma_priv.h index f0e9a80..c1a6ce1 100644 --- a/drivers/media/platform/ti-vpe/vpdma_priv.h +++ b/drivers/media/platform/ti-vpe/vpdma_priv.h @@ -78,7 +78,7 @@ #define DATA_TYPE_C420 0x6 #define DATA_TYPE_YC4220x7 #define DATA_TYPE_YC4440x8 -#define DATA_TYPE_CY4220x23 +#define DATA_TYPE_CY4220x27 #define DATA_TYPE_RGB16_5650x0 #define DATA_TYPE_ARGB_15550x1 -- 1.8.3.2 -- 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] v4l: ti-vpe: checking for IS_ERR() instead of NULL
Hi Dan, On Friday 08 November 2013 03:31 PM, Dan Carpenter wrote: devm_ioremap() returns NULL on error, it doesn't return an ERR_PTR. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 4e58069..e163466 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1962,8 +1962,8 @@ static int vpe_probe(struct platform_device *pdev) * registers based on the sub block base addresses */ dev-base = devm_ioremap(pdev-dev, res-start, SZ_32K); - if (IS_ERR(dev-base)) { - ret = PTR_ERR(dev-base); + if (!dev-base) { + ret = -ENOMEM; goto v4l2_dev_unreg; } Thanks for the patch, this was addressed in Wei's patch though: v4l: ti-vpe: fix return value check in vpe_probe() Thanks, Archit -- 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 -next] [media] v4l: ti-vpe: fix return value check in vpe_probe()
On Wednesday 30 October 2013 08:45 AM, Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn In case of error, the function devm_kzalloc() and devm_ioremap() returns NULL pointer not ERR_PTR(). The IS_ERR() test in the return value check should be replaced with NULL test. Reviewed-by: Archit Taneja arc...@ti.com Thanks, Archit Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn --- drivers/media/platform/ti-vpe/vpe.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 4e58069..90cf369 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1942,8 +1942,8 @@ static int vpe_probe(struct platform_device *pdev) int ret, irq, func; dev = devm_kzalloc(pdev-dev, sizeof(*dev), GFP_KERNEL); - if (IS_ERR(dev)) - return PTR_ERR(dev); + if (!dev) + return -ENOMEM; spin_lock_init(dev-lock); @@ -1962,8 +1962,8 @@ static int vpe_probe(struct platform_device *pdev) * registers based on the sub block base addresses */ dev-base = devm_ioremap(pdev-dev, res-start, SZ_32K); - if (IS_ERR(dev-base)) { - ret = PTR_ERR(dev-base); + if (!dev-base) { + ret = -ENOMEM; goto v4l2_dev_unreg; } -- 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 -next] [media] v4l: ti-vpe: fix return value check in vpe_probe()
On Wednesday 30 October 2013 08:45 AM, Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn In case of error, the function devm_kzalloc() and devm_ioremap() returns NULL pointer not ERR_PTR(). The IS_ERR() test in the return value check should be replaced with NULL test. Reviewed-by: Archit Taneja arc...@ti.com Thanks, Archit Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn --- drivers/media/platform/ti-vpe/vpe.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c index 4e58069..90cf369 100644 --- a/drivers/media/platform/ti-vpe/vpe.c +++ b/drivers/media/platform/ti-vpe/vpe.c @@ -1942,8 +1942,8 @@ static int vpe_probe(struct platform_device *pdev) int ret, irq, func; dev = devm_kzalloc(pdev-dev, sizeof(*dev), GFP_KERNEL); - if (IS_ERR(dev)) - return PTR_ERR(dev); + if (!dev) + return -ENOMEM; spin_lock_init(dev-lock); @@ -1962,8 +1962,8 @@ static int vpe_probe(struct platform_device *pdev) * registers based on the sub block base addresses */ dev-base = devm_ioremap(pdev-dev, res-start, SZ_32K); - if (IS_ERR(dev-base)) { - ret = PTR_ERR(dev-base); + if (!dev-base) { + ret = -ENOMEM; goto v4l2_dev_unreg; } -- 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