| Message ID | 20200423200937.1039257-4-paul.kocialkowski@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: rockchip: rga: PX30 support and YUV2YUV fix | expand |
Hey Paul, Thanks for the patch! On Thu, 2020-04-23 at 22:09 +0200, Paul Kocialkowski wrote: > The PX30 SoC has a RGA block, so add the associated compatible to > support it. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > drivers/media/platform/rockchip/rga/rga.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c > index 9d122429706e..4fb4615662b7 100644 > --- a/drivers/media/platform/rockchip/rga/rga.c > +++ b/drivers/media/platform/rockchip/rga/rga.c > @@ -955,6 +955,9 @@ static const struct dev_pm_ops rga_pm = { > }; > > static const struct of_device_id rockchip_rga_match[] = { > + { > + .compatible = "rockchip,px30-rga", > + }, Please note that if you don't have anything px30-specific, then you don't need the compatible in the driver. You can have something like: compatible = "rockchip,px30-rga", "rockchip,rk3288-rga" so you need to add it to the bindings. See Justin Swartz recent patches for rk3228. Down the road, if you find something specific for px30, you can make the driver aware. Cheers, Ezequiel
Hi Ezequiel, On Fri 24 Apr 20, 09:54, Ezequiel Garcia wrote: > Hey Paul, > > Thanks for the patch! > > On Thu, 2020-04-23 at 22:09 +0200, Paul Kocialkowski wrote: > > The PX30 SoC has a RGA block, so add the associated compatible to > > support it. > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > --- > > drivers/media/platform/rockchip/rga/rga.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c > > index 9d122429706e..4fb4615662b7 100644 > > --- a/drivers/media/platform/rockchip/rga/rga.c > > +++ b/drivers/media/platform/rockchip/rga/rga.c > > @@ -955,6 +955,9 @@ static const struct dev_pm_ops rga_pm = { > > }; > > > > static const struct of_device_id rockchip_rga_match[] = { > > + { > > + .compatible = "rockchip,px30-rga", > > + }, > > Please note that if you don't have anything px30-specific, > then you don't need the compatible in the driver. > > You can have something like: > > compatible = "rockchip,px30-rga", "rockchip,rk3288-rga" > > so you need to add it to the bindings. See Justin Swartz > recent patches for rk3228. Thanks for the instruction! I've been a bit confused about that because RK3399 has its own compatible (without a 2nd rk3288 compatible) although there's nothing different with it either. All of these rockchip platforms come with what they call "RGA2", that seems to have no variation across platforms (downstream rockchip even has a single compatible for it). Should we add the rk3288 compatible to the rk3399 dtsi? I guess we can't remove it from the driver at this point, for backward compatibility with previous dts (what a strange idea...). > Down the road, if you find something specific for px30, > you can make the driver aware. Makes sense, yes. Cheers, Paul
On Fri, 2020-04-24 at 15:55 +0200, Paul Kocialkowski wrote: > Hi Ezequiel, > > On Fri 24 Apr 20, 09:54, Ezequiel Garcia wrote: > > Hey Paul, > > > > Thanks for the patch! > > > > On Thu, 2020-04-23 at 22:09 +0200, Paul Kocialkowski wrote: > > > The PX30 SoC has a RGA block, so add the associated compatible to > > > support it. > > > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > > --- > > > drivers/media/platform/rockchip/rga/rga.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c > > > index 9d122429706e..4fb4615662b7 100644 > > > --- a/drivers/media/platform/rockchip/rga/rga.c > > > +++ b/drivers/media/platform/rockchip/rga/rga.c > > > @@ -955,6 +955,9 @@ static const struct dev_pm_ops rga_pm = { > > > }; > > > > > > static const struct of_device_id rockchip_rga_match[] = { > > > + { > > > + .compatible = "rockchip,px30-rga", > > > + }, > > > > Please note that if you don't have anything px30-specific, > > then you don't need the compatible in the driver. > > > > You can have something like: > > > > compatible = "rockchip,px30-rga", "rockchip,rk3288-rga" > > > > so you need to add it to the bindings. See Justin Swartz > > recent patches for rk3228. > > Thanks for the instruction! > > I've been a bit confused about that because RK3399 has its own compatible > (without a 2nd rk3288 compatible) although there's nothing different with it > either. All of these rockchip platforms come with what they call "RGA2", that > seems to have no variation across platforms (downstream rockchip even has a > single compatible for it). > Yep, and that's an anti pattern, so you can expect to see that elsewhere. > Should we add the rk3288 compatible to the rk3399 dtsi? I guess we > can't remove it from the driver at this point, for backward compatibility > with previous dts (what a strange idea...). > No, we don't necesarily need/have to do anything with rk3399, not sure if there's any gain at this point. > > Down the road, if you find something specific for px30, > > you can make the driver aware. > > Makes sense, yes. > Cool. Thanks, Ezequiel
diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c index 9d122429706e..4fb4615662b7 100644 --- a/drivers/media/platform/rockchip/rga/rga.c +++ b/drivers/media/platform/rockchip/rga/rga.c @@ -955,6 +955,9 @@ static const struct dev_pm_ops rga_pm = { }; static const struct of_device_id rockchip_rga_match[] = { + { + .compatible = "rockchip,px30-rga", + }, { .compatible = "rockchip,rk3288-rga", },
The PX30 SoC has a RGA block, so add the associated compatible to support it. Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> --- drivers/media/platform/rockchip/rga/rga.c | 3 +++ 1 file changed, 3 insertions(+)