Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
On 11/10/17 21:08, Dmitry Osipenko wrote:
> Add a device node for the video decoder engine found on Tegra20.
>
> Signed-off-by: Dmitry Osipenko
> ---
> arch/arm/boot/dts/tegra20.dtsi | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 7c85f97f72ea..1b5d54b6c0cb 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -249,6 +249,23 @@
> */
> };
>
> + vde@6001a000 {
> + compatible = "nvidia,tegra20-vde";
> + reg = <0x6001a000 0x3D00/* VDE registers */
> +0x4400 0x3FC00>; /* IRAM region */
> + reg-names = "regs", "iram";
> + interrupts = , /* UCQ error
> interrupt */
> + , /* Sync token
> interrupt */
> + , /* BSE-V
> interrupt */
> + , /* BSE-A
> interrupt */
> + ; /* SXE interrupt
> */
> + interrupt-names = "ucq-error", "sync-token", "bsev", "bsea",
> "sxe";
> + clocks = <&tegra_car TEGRA20_CLK_VDE>;
> + clock-names = "vde";
> + resets = <&tegra_car 61>;
> + reset-names = "vde";
> + };
> +
I don't see any binding documentation for this node. We need to make
sure we add this.
Jon
--
nvpublic
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
On 12/10/17 11:51, Dmitry Osipenko wrote:
> On 12.10.2017 11:49, Jon Hunter wrote:
>>
>> On 11/10/17 21:08, Dmitry Osipenko wrote:
>>> Add a device node for the video decoder engine found on Tegra20.
>>>
>>> Signed-off-by: Dmitry Osipenko
>>> ---
>>> arch/arm/boot/dts/tegra20.dtsi | 17 +
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>> @@ -249,6 +249,23 @@
>>> */
>>> };
>>>
>>> + vde@6001a000 {
>>> + compatible = "nvidia,tegra20-vde";
>>> + reg = <0x6001a000 0x3D00/* VDE registers */
>>> + 0x4400 0x3FC00>; /* IRAM region */
>>> + reg-names = "regs", "iram";
>>> + interrupts = , /* UCQ error
>>> interrupt */
>>> +, /* Sync token
>>> interrupt */
>>> +, /* BSE-V
>>> interrupt */
>>> +, /* BSE-A
>>> interrupt */
>>> +; /* SXE interrupt
>>> */
>>> + interrupt-names = "ucq-error", "sync-token", "bsev", "bsea",
>>> "sxe";
>>> + clocks = <&tegra_car TEGRA20_CLK_VDE>;
>>> + clock-names = "vde";
>>> + resets = <&tegra_car 61>;
>>> + reset-names = "vde";
>>> + };
>>> +
>>
>> I don't see any binding documentation for this node. We need to make
>> sure we add this.
>>
>
> It's in the first patch.
>
> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
>
Ah yes indeed, then that needs to be a separate patch.
Cheers
Jon
--
nvpublic
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
On 12/10/17 14:25, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Thu, Oct 12, 2017 at 03:06:17PM +0300, Dmitry Osipenko wrote:
>> Hello Vladimir,
>>
>> On 12.10.2017 10:43, Vladimir Zapolskiy wrote:
>>> Hello Dmitry,
>>>
>>> On 10/11/2017 11:08 PM, Dmitry Osipenko wrote:
Add a device node for the video decoder engine found on Tegra20.
Signed-off-by: Dmitry Osipenko
---
arch/arm/boot/dts/tegra20.dtsi | 17 +
1 file changed, 17 insertions(+)
diff --git a/arch/arm/boot/dts/tegra20.dtsi
b/arch/arm/boot/dts/tegra20.dtsi
index 7c85f97f72ea..1b5d54b6c0cb 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -249,6 +249,23 @@
*/
};
+ vde@6001a000 {
+ compatible = "nvidia,tegra20-vde";
+ reg = <0x6001a000 0x3D00/* VDE registers */
+ 0x4400 0x3FC00>; /* IRAM region */
>>>
>>> this notation of a used region in IRAM is non-standard and potentially it
>>> may lead to conflicts for IRAM resource between users.
>>>
>>> My proposal is to add a valid device tree node to describe an IRAM region
>>> firstly, then reserve a subregion in it by using a new "iram" property.
>>>
>>
>> The defined in DT IRAM region used by VDE isn't exactly correct, actually it
>> should be much smaller. I don't know exactly what parts of IRAM VDE uses, for
>> now it is just safer to assign the rest of the IRAM region to VDE.
>>
>> I'm not sure whether it really worthy to use a dynamic allocator for a single
>> static allocation, but maybe it would come handy later.. Stephen / Jon /
>> Thierry, what do you think?
>
> This sounds like a good idea. I agree that this currently doesn't seem
> to be warranted, but consider what would happen if at some point we have
> more devices requiring access to the IRAM. Spreading individual reg
> properties all across the DT will make it very difficult to ensure they
> don't overlap.
>
> Presumably the mmio-sram driver will check that pool don't overlap. Or
> even if it doesn't it will make it a lot easier to verify because it's
> all in the same DT node and then consumers only reference it.
>
> I like Vladimir's proposal. I also suspect that Rob may want us to stick
> to a standardized way referencing such external memory.
FWIW I agree. Seems like a nice approach and describes the h/w accurately.
Cheers
Jon
--
nvpublic
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: staging: tegra-vde: add missing pm_runtime_put_autosuspend
On 02/06/2020 06:48, Navid Emamdoost wrote:
> Call to pm_runtime_get_sync increments counter even in case of
> failure leading to incorrect ref count.
> Call pm_runtime_put_autosuspend if pm_runtime_get_sync fails.
>
> Signed-off-by: Navid Emamdoost
> ---
> drivers/staging/media/tegra-vde/vde.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/tegra-vde/vde.c
> b/drivers/staging/media/tegra-vde/vde.c
> index d3e63512a765..52cdd4a91e93 100644
> --- a/drivers/staging/media/tegra-vde/vde.c
> +++ b/drivers/staging/media/tegra-vde/vde.c
> @@ -776,8 +776,10 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde
> *vde,
> goto release_dpb_frames;
>
> ret = pm_runtime_get_sync(dev);
> - if (ret < 0)
> + if (ret < 0) {
> + pm_runtime_put_autosuspend(dev);
> goto unlock;
> + }
>
> /*
>* We rely on the VDE registers reset value, otherwise VDE
Please use the put in the error path.
Jon
--
nvpublic
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
