[PATCH v2 2/4] drm/tegra: Add VIC support

[Thierry Reding]

On Mon, Jul 20, 2015 at 10:56:00AM +0100, Jon Hunter wrote:
> 
> On 20/07/15 09:51, Mikko Perttunen wrote:
> > On 07/20/2015 11:28 AM, Jon Hunter wrote:
> >> Hi Mikko,
> > 
> > Hi!
> > 
> >> ...
> >>> +static int vic_runtime_resume(struct device *dev)
> >>> +{
> >>> + struct vic *vic = dev_get_drvdata(dev);
> >>> + int err;
> >>> +
> >>> + err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
> >>> + vic->clk, vic->rst);
> >>
> >> I would like to deprecate use of the above function [1]. I have been
> >> trying to migrate driver to use a new API instead of the above.
> > 
> > Yes, we will need to coordinate the API change here. I kept the old way
> > here to not depend on your series for now.
> > 
> >>
> >>> + if (err < 0)
> >>> + dev_err(dev, "failed to power up device\n");
> >>> +
> >>> + return err;
> >>> +}
> >>> +
> >>> ...
> >>> +
> >>> + pm_runtime_enable(>dev);
> >>> + if (!pm_runtime_enabled(>dev)) {
> >>> + err = vic_runtime_resume(>dev);
> >>> + if (err < 0)
> >>> + goto unregister_client;
> >>> + }
> >>
> >> I don't see why pm_runtime_enable() should ever fail to enable
> >> pm_runtime here. Hence, the if-statement appears to be redundant. If you
> >> are trying to determine whether rpm is supported for the power-domains
> >> then you should simply check to see if the DT node for the device has
> >> the "power-domains" property. See my series [1].
> > 
> > The intention is that if runtime PM is not enabled, the power domain is
> > still brought up. On a cursory look, it seems like with your patch, this
> > should indeed work fine without this check if CONFIG_PM is enabled. Now,
> > that might always be enabled? If so, then everything would be fine; if
> > this lands after your series, we could even just make the power-domains
> > prop mandatory and not implement the legacy mode at all. If not, then
> > AFAIU we must still keep this path.
> 
> Ok, I see you just wish to test it is enabled in the kernel config. Then
> yes that would make sense and the above is fine.

Do we really need to bother about this? If runtime PM isn't enabled we
don't care very much about power management anyway. So in my opinion if
a driver supports runtime PM it should support it all the way and not
pretend to. Having this mix of runtime PM vs. no runtime PM is beside
the point of using something like runtime PM in the first place.

So if this is about supporting legacy vs. runtime PM, then I think it
should be based on some more explicit check, like if (!dev->pm_domain),
but otherwise we should assume that pm_runtime_enable() succeeds.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 

[PATCH v2 2/4] drm/tegra: Add VIC support

[Mikko Perttunen]

On 07/20/2015 11:28 AM, Jon Hunter wrote:
> Hi Mikko,

Hi!

> ...
>> +static int vic_runtime_resume(struct device *dev)
>> +{
>> +struct vic *vic = dev_get_drvdata(dev);
>> +int err;
>> +
>> +err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
>> +vic->clk, vic->rst);
> 
> I would like to deprecate use of the above function [1]. I have been
> trying to migrate driver to use a new API instead of the above.

Yes, we will need to coordinate the API change here. I kept the old way
here to not depend on your series for now.

> 
>> +if (err < 0)
>> +dev_err(dev, "failed to power up device\n");
>> +
>> +return err;
>> +}
>> +
>> ...
>> +
>> +pm_runtime_enable(>dev);
>> +if (!pm_runtime_enabled(>dev)) {
>> +err = vic_runtime_resume(>dev);
>> +if (err < 0)
>> +goto unregister_client;
>> +}
> 
> I don't see why pm_runtime_enable() should ever fail to enable
> pm_runtime here. Hence, the if-statement appears to be redundant. If you
> are trying to determine whether rpm is supported for the power-domains
> then you should simply check to see if the DT node for the device has
> the "power-domains" property. See my series [1].

The intention is that if runtime PM is not enabled, the power domain is
still brought up. On a cursory look, it seems like with your patch, this
should indeed work fine without this check if CONFIG_PM is enabled. Now,
that might always be enabled? If so, then everything would be fine; if
this lands after your series, we could even just make the power-domains
prop mandatory and not implement the legacy mode at all. If not, then
AFAIU we must still keep this path.

> 
> Cheers
> Jon
> 
> [1] http://www.spinics.net/lists/arm-kernel/msg431051.html
> 

Cheers,
Mikko.

[PATCH v2 2/4] drm/tegra: Add VIC support

[Thierry Reding]

On Mon, Jul 20, 2015 at 09:28:36AM +0100, Jon Hunter wrote:
> Hi Mikko,
> 
> On 20/07/15 08:54, Mikko Perttunen wrote:
> > From: Arto Merilainen 
> > 
> > This patch adds support for Video Image Compositor engine which
> > can be used for 2d operations.
> > 
> > Signed-off-by: Andrew Chew 
> > Signed-off-by: Arto Merilainen 
> > Signed-off-by: Mikko Perttunen 
> > ---
> >  drivers/gpu/drm/tegra/Makefile |   3 +-
> >  drivers/gpu/drm/tegra/drm.c|   7 +
> >  drivers/gpu/drm/tegra/drm.h|   1 +
> >  drivers/gpu/drm/tegra/vic.c| 456 
> > +
> >  drivers/gpu/drm/tegra/vic.h|  35 
> >  include/linux/host1x.h |   1 +
> >  6 files changed, 502 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/tegra/vic.c
> >  create mode 100644 drivers/gpu/drm/tegra/vic.h
> > 
> > diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
> > index 93e9a4a..6af3a9a 100644
> > --- a/drivers/gpu/drm/tegra/Makefile
> > +++ b/drivers/gpu/drm/tegra/Makefile
> > @@ -14,6 +14,7 @@ tegra-drm-y := \
> > dpaux.o \
> > gr2d.o \
> > gr3d.o \
> > -   falcon.o
> > +   falcon.o \
> > +   vic.o
> >  
> >  obj-$(CONFIG_DRM_TEGRA) += tegra-drm.o
> > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> > index af4ff86..bc8cc2a 100644
> > --- a/drivers/gpu/drm/tegra/drm.c
> > +++ b/drivers/gpu/drm/tegra/drm.c
> > @@ -1145,6 +1145,7 @@ static const struct of_device_id host1x_drm_subdevs[] 
> > = {
> > { .compatible = "nvidia,tegra124-dc", },
> > { .compatible = "nvidia,tegra124-sor", },
> > { .compatible = "nvidia,tegra124-hdmi", },
> > +   { .compatible = "nvidia,tegra124-vic", },
> > { /* sentinel */ }
> >  };
> >  
> > @@ -1194,8 +1195,14 @@ static int __init host1x_drm_init(void)
> > if (err < 0)
> > goto unregister_gr2d;
> >  
> > +   err = platform_driver_register(_vic_driver);
> > +   if (err < 0)
> > +   goto unregister_gr3d;
> > +
> > return 0;
> >  
> > +unregister_gr3d:
> > +   platform_driver_unregister(_gr3d_driver);
> >  unregister_gr2d:
> > platform_driver_unregister(_gr2d_driver);
> >  unregister_dpaux:
> > diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> > index 58c83b11..2fc7e42 100644
> > --- a/drivers/gpu/drm/tegra/drm.h
> > +++ b/drivers/gpu/drm/tegra/drm.h
> > @@ -284,5 +284,6 @@ extern struct platform_driver tegra_hdmi_driver;
> >  extern struct platform_driver tegra_dpaux_driver;
> >  extern struct platform_driver tegra_gr2d_driver;
> >  extern struct platform_driver tegra_gr3d_driver;
> > +extern struct platform_driver tegra_vic_driver;
> >  
> >  #endif /* HOST1X_DRM_H */
> > diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> > new file mode 100644
> > index 000..fce7c04
> > --- /dev/null
> > +++ b/drivers/gpu/drm/tegra/vic.c
> > @@ -0,0 +1,456 @@
> > +/*
> > + * Copyright (c) 2015, NVIDIA Corporation.
> > + *
> > + * 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 
> > +
> > +#include 
> > +
> > +#include "drm.h"
> > +#include "falcon.h"
> > +#include "vic.h"
> > +
> > +struct vic_config {
> > +   /* Firmware name */
> > +   const char *ucode_name;
> > +};
> > +
> > +struct vic {
> > +   struct falcon falcon;
> > +   bool booted;
> > +
> > +   void __iomem *regs;
> > +   struct tegra_drm_client client;
> > +   struct host1x_channel *channel;
> > +   struct iommu_domain *domain;
> > +   struct device *dev;
> > +   struct clk *clk;
> > +   struct reset_control *rst;
> > +
> > +   /* Platform configuration */
> > +   const struct vic_config *config;
> > +
> > +   /* for firewall - this determines if method 1 should be regarded
> > +* as an address register */
> > +   bool method_data_is_addr_reg;
> > +};
> > +
> > +static inline struct vic *to_vic(struct tegra_drm_client *client)
> > +{
> > +   return container_of(client, struct vic, client);
> > +}
> > +
> > +static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
> > +{
> > +   writel(value, vic->regs + offset);
> > +}
> > +
> > +static int vic_runtime_resume(struct device *dev)
> > +{
> > +   struct vic *vic = dev_get_drvdata(dev);
> > +   int err;
> > +
> > +   err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
> > +   vic->clk, vic->rst);
> 
> I would like to deprecate use of the above function [1]. I have been
> trying to migrate driver to use a new API instead of the above.
> 
> > +   if (err < 0)
> > +   dev_err(dev, "failed to power up device\n");
> > +
> > +   return err;
> > +}
> > +
> > +static int vic_runtime_suspend(struct device *dev)
> > +{
> > +   

[PATCH v2 2/4] drm/tegra: Add VIC support

[Jon Hunter]

On 20/07/15 09:51, Mikko Perttunen wrote:
> On 07/20/2015 11:28 AM, Jon Hunter wrote:
>> Hi Mikko,
> 
> Hi!
> 
>> ...
>>> +static int vic_runtime_resume(struct device *dev)
>>> +{
>>> +   struct vic *vic = dev_get_drvdata(dev);
>>> +   int err;
>>> +
>>> +   err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
>>> +   vic->clk, vic->rst);
>>
>> I would like to deprecate use of the above function [1]. I have been
>> trying to migrate driver to use a new API instead of the above.
> 
> Yes, we will need to coordinate the API change here. I kept the old way
> here to not depend on your series for now.
> 
>>
>>> +   if (err < 0)
>>> +   dev_err(dev, "failed to power up device\n");
>>> +
>>> +   return err;
>>> +}
>>> +
>>> ...
>>> +
>>> +   pm_runtime_enable(>dev);
>>> +   if (!pm_runtime_enabled(>dev)) {
>>> +   err = vic_runtime_resume(>dev);
>>> +   if (err < 0)
>>> +   goto unregister_client;
>>> +   }
>>
>> I don't see why pm_runtime_enable() should ever fail to enable
>> pm_runtime here. Hence, the if-statement appears to be redundant. If you
>> are trying to determine whether rpm is supported for the power-domains
>> then you should simply check to see if the DT node for the device has
>> the "power-domains" property. See my series [1].
> 
> The intention is that if runtime PM is not enabled, the power domain is
> still brought up. On a cursory look, it seems like with your patch, this
> should indeed work fine without this check if CONFIG_PM is enabled. Now,
> that might always be enabled? If so, then everything would be fine; if
> this lands after your series, we could even just make the power-domains
> prop mandatory and not implement the legacy mode at all. If not, then
> AFAIU we must still keep this path.

Ok, I see you just wish to test it is enabled in the kernel config. Then
yes that would make sense and the above is fine.

Cheers
Jon

[PATCH v2 2/4] drm/tegra: Add VIC support

[Mikko Perttunen]

From: Arto Merilainen 

This patch adds support for Video Image Compositor engine which
can be used for 2d operations.

Signed-off-by: Andrew Chew 
Signed-off-by: Arto Merilainen 
Signed-off-by: Mikko Perttunen 
---
 drivers/gpu/drm/tegra/Makefile |   3 +-
 drivers/gpu/drm/tegra/drm.c|   7 +
 drivers/gpu/drm/tegra/drm.h|   1 +
 drivers/gpu/drm/tegra/vic.c| 456 +
 drivers/gpu/drm/tegra/vic.h|  35 
 include/linux/host1x.h |   1 +
 6 files changed, 502 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/tegra/vic.c
 create mode 100644 drivers/gpu/drm/tegra/vic.h

diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
index 93e9a4a..6af3a9a 100644
--- a/drivers/gpu/drm/tegra/Makefile
+++ b/drivers/gpu/drm/tegra/Makefile
@@ -14,6 +14,7 @@ tegra-drm-y := \
dpaux.o \
gr2d.o \
gr3d.o \
-   falcon.o
+   falcon.o \
+   vic.o

 obj-$(CONFIG_DRM_TEGRA) += tegra-drm.o
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index af4ff86..bc8cc2a 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1145,6 +1145,7 @@ static const struct of_device_id host1x_drm_subdevs[] = {
{ .compatible = "nvidia,tegra124-dc", },
{ .compatible = "nvidia,tegra124-sor", },
{ .compatible = "nvidia,tegra124-hdmi", },
+   { .compatible = "nvidia,tegra124-vic", },
{ /* sentinel */ }
 };

@@ -1194,8 +1195,14 @@ static int __init host1x_drm_init(void)
if (err < 0)
goto unregister_gr2d;

+   err = platform_driver_register(_vic_driver);
+   if (err < 0)
+   goto unregister_gr3d;
+
return 0;

+unregister_gr3d:
+   platform_driver_unregister(_gr3d_driver);
 unregister_gr2d:
platform_driver_unregister(_gr2d_driver);
 unregister_dpaux:
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 58c83b11..2fc7e42 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -284,5 +284,6 @@ extern struct platform_driver tegra_hdmi_driver;
 extern struct platform_driver tegra_dpaux_driver;
 extern struct platform_driver tegra_gr2d_driver;
 extern struct platform_driver tegra_gr3d_driver;
+extern struct platform_driver tegra_vic_driver;

 #endif /* HOST1X_DRM_H */
diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
new file mode 100644
index 000..fce7c04
--- /dev/null
+++ b/drivers/gpu/drm/tegra/vic.c
@@ -0,0 +1,456 @@
+/*
+ * Copyright (c) 2015, NVIDIA Corporation.
+ *
+ * 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 
+
+#include 
+
+#include "drm.h"
+#include "falcon.h"
+#include "vic.h"
+
+struct vic_config {
+   /* Firmware name */
+   const char *ucode_name;
+};
+
+struct vic {
+   struct falcon falcon;
+   bool booted;
+
+   void __iomem *regs;
+   struct tegra_drm_client client;
+   struct host1x_channel *channel;
+   struct iommu_domain *domain;
+   struct device *dev;
+   struct clk *clk;
+   struct reset_control *rst;
+
+   /* Platform configuration */
+   const struct vic_config *config;
+
+   /* for firewall - this determines if method 1 should be regarded
+* as an address register */
+   bool method_data_is_addr_reg;
+};
+
+static inline struct vic *to_vic(struct tegra_drm_client *client)
+{
+   return container_of(client, struct vic, client);
+}
+
+static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
+{
+   writel(value, vic->regs + offset);
+}
+
+static int vic_runtime_resume(struct device *dev)
+{
+   struct vic *vic = dev_get_drvdata(dev);
+   int err;
+
+   err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
+   vic->clk, vic->rst);
+   if (err < 0)
+   dev_err(dev, "failed to power up device\n");
+
+   return err;
+}
+
+static int vic_runtime_suspend(struct device *dev)
+{
+   struct vic *vic = dev_get_drvdata(dev);
+
+   clk_disable_unprepare(vic->clk);
+   reset_control_assert(vic->rst);
+   tegra_powergate_power_off(TEGRA_POWERGATE_VIC);
+
+   vic->booted = false;
+
+   return 0;
+}
+
+static int vic_boot(struct vic *vic)
+{
+   u32 fce_ucode_size, fce_bin_data_offset;
+   void *hdr;
+   int err = 0;
+
+   if (vic->booted)
+   return 0;
+
+   if (!vic->falcon.firmware.valid) {
+   err = falcon_read_firmware(>falcon,
+  vic->config->ucode_name);
+   if (err < 0)
+   return err;
+   }
+
+   /* ensure that 

[PATCH v2 2/4] drm/tegra: Add VIC support

[Jon Hunter]

Hi Mikko,

On 20/07/15 08:54, Mikko Perttunen wrote:
> From: Arto Merilainen 
> 
> This patch adds support for Video Image Compositor engine which
> can be used for 2d operations.
> 
> Signed-off-by: Andrew Chew 
> Signed-off-by: Arto Merilainen 
> Signed-off-by: Mikko Perttunen 
> ---
>  drivers/gpu/drm/tegra/Makefile |   3 +-
>  drivers/gpu/drm/tegra/drm.c|   7 +
>  drivers/gpu/drm/tegra/drm.h|   1 +
>  drivers/gpu/drm/tegra/vic.c| 456 
> +
>  drivers/gpu/drm/tegra/vic.h|  35 
>  include/linux/host1x.h |   1 +
>  6 files changed, 502 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/tegra/vic.c
>  create mode 100644 drivers/gpu/drm/tegra/vic.h
> 
> diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
> index 93e9a4a..6af3a9a 100644
> --- a/drivers/gpu/drm/tegra/Makefile
> +++ b/drivers/gpu/drm/tegra/Makefile
> @@ -14,6 +14,7 @@ tegra-drm-y := \
>   dpaux.o \
>   gr2d.o \
>   gr3d.o \
> - falcon.o
> + falcon.o \
> + vic.o
>  
>  obj-$(CONFIG_DRM_TEGRA) += tegra-drm.o
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index af4ff86..bc8cc2a 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -1145,6 +1145,7 @@ static const struct of_device_id host1x_drm_subdevs[] = 
> {
>   { .compatible = "nvidia,tegra124-dc", },
>   { .compatible = "nvidia,tegra124-sor", },
>   { .compatible = "nvidia,tegra124-hdmi", },
> + { .compatible = "nvidia,tegra124-vic", },
>   { /* sentinel */ }
>  };
>  
> @@ -1194,8 +1195,14 @@ static int __init host1x_drm_init(void)
>   if (err < 0)
>   goto unregister_gr2d;
>  
> + err = platform_driver_register(_vic_driver);
> + if (err < 0)
> + goto unregister_gr3d;
> +
>   return 0;
>  
> +unregister_gr3d:
> + platform_driver_unregister(_gr3d_driver);
>  unregister_gr2d:
>   platform_driver_unregister(_gr2d_driver);
>  unregister_dpaux:
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 58c83b11..2fc7e42 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -284,5 +284,6 @@ extern struct platform_driver tegra_hdmi_driver;
>  extern struct platform_driver tegra_dpaux_driver;
>  extern struct platform_driver tegra_gr2d_driver;
>  extern struct platform_driver tegra_gr3d_driver;
> +extern struct platform_driver tegra_vic_driver;
>  
>  #endif /* HOST1X_DRM_H */
> diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> new file mode 100644
> index 000..fce7c04
> --- /dev/null
> +++ b/drivers/gpu/drm/tegra/vic.c
> @@ -0,0 +1,456 @@
> +/*
> + * Copyright (c) 2015, NVIDIA Corporation.
> + *
> + * 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 
> +
> +#include 
> +
> +#include "drm.h"
> +#include "falcon.h"
> +#include "vic.h"
> +
> +struct vic_config {
> + /* Firmware name */
> + const char *ucode_name;
> +};
> +
> +struct vic {
> + struct falcon falcon;
> + bool booted;
> +
> + void __iomem *regs;
> + struct tegra_drm_client client;
> + struct host1x_channel *channel;
> + struct iommu_domain *domain;
> + struct device *dev;
> + struct clk *clk;
> + struct reset_control *rst;
> +
> + /* Platform configuration */
> + const struct vic_config *config;
> +
> + /* for firewall - this determines if method 1 should be regarded
> +  * as an address register */
> + bool method_data_is_addr_reg;
> +};
> +
> +static inline struct vic *to_vic(struct tegra_drm_client *client)
> +{
> + return container_of(client, struct vic, client);
> +}
> +
> +static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
> +{
> + writel(value, vic->regs + offset);
> +}
> +
> +static int vic_runtime_resume(struct device *dev)
> +{
> + struct vic *vic = dev_get_drvdata(dev);
> + int err;
> +
> + err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
> + vic->clk, vic->rst);

I would like to deprecate use of the above function [1]. I have been
trying to migrate driver to use a new API instead of the above.

> + if (err < 0)
> + dev_err(dev, "failed to power up device\n");
> +
> + return err;
> +}
> +
> +static int vic_runtime_suspend(struct device *dev)
> +{
> + struct vic *vic = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(vic->clk);
> + reset_control_assert(vic->rst);
> + tegra_powergate_power_off(TEGRA_POWERGATE_VIC);

Similarly here. I would like to get rid of the above API in favour of a
different API to help migrate to generic