| Message ID | 20161118024436.13447-1-robertcnelson@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Sorry, messed up the cover-header, patches 3 & 4 in this series where cherry picked form ti's v4.4.x tree: http://git.ti.com/cgit/cgit.cgi/ti-linux-kernel/ti-linux-kernel.git/commit/arch/arm/boot/dts/dra7.dtsi?h=ti-linux-4.4.y&id=8067f5a5619ce45657d3729ab3adb9e5b1294f0d http://git.ti.com/cgit/cgit.cgi/ti-linux-kernel/ti-linux-kernel.git/commit/Documentation/devicetree/bindings/gpu/ti-bb2d.txt?h=ti-linux-4.4.y&id=ddadad0828f8e5ad7e89b11dd243249228ff2997 "ti,gc320-gpu-subsystem" seems like the best middle ground option, the gc320 is found on one omap4770, omap5, dra7.. "ti,omap-gpu-subsystem" might clash with the naming for the sgx544 3D core found on these devices, then some day, ti could use "ti,sgx<num>-gpu-subsystem" For BeagleBoard.org & BeagleBoard-x15 users we have this working in the updated images. For people looking to build the packages, i have it up here: https://gist.github.com/RobertCNelson/fc6d07157b0fcc13b9c28c5832fdc74b Regards, On Thu, Nov 17, 2016 at 8:44 PM, Robert Nelson <robertcnelson@gmail.com> wrote: > Signed-off-by: Robert Nelson <robertcnelson@gmail.com> > CC: Julien <jboulnois@gmail.com> > CC: Christian Gmeiner <christian.gmeiner@gmail.com> > CC: Russell King <rmk+kernel@arm.linux.org.uk> > CC: Lucas Stach <l.stach@pengutronix.de> > CC: Nishanth Menon <nm@ti.com> > CC: Tomi Valkeinen <tomi.valkeinen@ti.com> > CC: Tony Lindgren <tony@atomide.com> > --- > Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 1 + > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt > index ed5e0a7..9fa259d 100644 > --- a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt > +++ b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt > @@ -8,6 +8,7 @@ Required properties: > - compatible: Should be one of > "fsl,imx-gpu-subsystem" > "marvell,dove-gpu-subsystem" > + "ti,gc320-gpu-subsystem" > - cores: Should contain a list of phandles pointing to Vivante GPU devices > > example: > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > index a6799b0..ce51270 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > @@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev) > static const struct of_device_id dt_match[] = { > { .compatible = "fsl,imx-gpu-subsystem" }, > { .compatible = "marvell,dove-gpu-subsystem" }, > + { .compatible = "ti,gc320-gpu-subsystem" }, > {} > }; > MODULE_DEVICE_TABLE(of, dt_match); > -- > 2.10.2 >
On 11/17/2016 08:44 PM, Robert Nelson wrote: Could we write at least a oneline commit message? :) Might want to state that since the TI gpu systems are a mixed bunch and certain SoCs may have more than 1 GPU integrated, we indicate clearly the rev here? > Signed-off-by: Robert Nelson <robertcnelson@gmail.com> > CC: Julien <jboulnois@gmail.com> > CC: Christian Gmeiner <christian.gmeiner@gmail.com> > CC: Russell King <rmk+kernel@arm.linux.org.uk> > CC: Lucas Stach <l.stach@pengutronix.de> > CC: Nishanth Menon <nm@ti.com> > CC: Tomi Valkeinen <tomi.valkeinen@ti.com> > CC: Tony Lindgren <tony@atomide.com> > --- > Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 1 + > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt > index ed5e0a7..9fa259d 100644 > --- a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt > +++ b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt > @@ -8,6 +8,7 @@ Required properties: > - compatible: Should be one of > "fsl,imx-gpu-subsystem" > "marvell,dove-gpu-subsystem" > + "ti,gc320-gpu-subsystem" > - cores: Should contain a list of phandles pointing to Vivante GPU devices > > example: > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > index a6799b0..ce51270 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > @@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev) > static const struct of_device_id dt_match[] = { > { .compatible = "fsl,imx-gpu-subsystem" }, > { .compatible = "marvell,dove-gpu-subsystem" }, > + { .compatible = "ti,gc320-gpu-subsystem" }, > {} > }; > MODULE_DEVICE_TABLE(of, dt_match); >
On Thu, Nov 17, 2016 at 08:53:38PM -0600, Nishanth Menon wrote: > >diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > >index a6799b0..ce51270 100644 > >--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > >+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > >@@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev) > > static const struct of_device_id dt_match[] = { > > { .compatible = "fsl,imx-gpu-subsystem" }, > > { .compatible = "marvell,dove-gpu-subsystem" }, > >+ { .compatible = "ti,gc320-gpu-subsystem" }, We need to get away from this ever-increasing set of compatible strings here, as this is not long-term maintainable. What we should have is a common compatible which describes that the node is compatible with this driver, and then use SoC specific compatible strings later if we need to (eg, because of some GPU subsystem SoC specifics.) So, I'd suggest that we update the documentation and add: "vivante,gc-gpu-subsystem" as a common compatible now, and if necessary move on to more specific compatibles if we need to later. Also, I'd strongly suggest that no compatibles should contain the ID number of the GPU core for exactly the same reason - Vivante GPU cores vary according to features, and we don't want to end up with a long list of specific compatibles (eg) "ti,gc2000-and-gc320-and-gc355-gpu-subsystem" because TI decides to integrate a 3d, 2d and VG core.
Am Freitag, den 18.11.2016, 12:13 +0000 schrieb Russell King - ARM Linux: > On Thu, Nov 17, 2016 at 08:53:38PM -0600, Nishanth Menon wrote: > > >diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > > >index a6799b0..ce51270 100644 > > >--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > > >+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > > >@@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev) > > > static const struct of_device_id dt_match[] = { > > > { .compatible = "fsl,imx-gpu-subsystem" }, > > > { .compatible = "marvell,dove-gpu-subsystem" }, > > >+ { .compatible = "ti,gc320-gpu-subsystem" }, > > We need to get away from this ever-increasing set of compatible > strings here, as this is not long-term maintainable. > > What we should have is a common compatible which describes that > the node is compatible with this driver, and then use SoC specific > compatible strings later if we need to (eg, because of some GPU > subsystem SoC specifics.) > > So, I'd suggest that we update the documentation and add: > > "vivante,gc-gpu-subsystem" > > as a common compatible now, and if necessary move on to more specific > compatibles if we need to later. > > Also, I'd strongly suggest that no compatibles should contain the ID > number of the GPU core for exactly the same reason - Vivante GPU cores > vary according to features, and we don't want to end up with a long > list of specific compatibles (eg) > "ti,gc2000-and-gc320-and-gc355-gpu-subsystem" because TI > decides to integrate a 3d, 2d and VG core. > All of the above sounds sensible and I would prefer if the patches are reworked to take those things into account. Regards, Lucas
On Fri, Nov 18, 2016 at 7:34 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > Am Freitag, den 18.11.2016, 12:13 +0000 schrieb Russell King - ARM > Linux: >> On Thu, Nov 17, 2016 at 08:53:38PM -0600, Nishanth Menon wrote: >> > >diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c >> > >index a6799b0..ce51270 100644 >> > >--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c >> > >+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c >> > >@@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev) >> > > static const struct of_device_id dt_match[] = { >> > > { .compatible = "fsl,imx-gpu-subsystem" }, >> > > { .compatible = "marvell,dove-gpu-subsystem" }, >> > >+ { .compatible = "ti,gc320-gpu-subsystem" }, >> >> We need to get away from this ever-increasing set of compatible >> strings here, as this is not long-term maintainable. >> >> What we should have is a common compatible which describes that >> the node is compatible with this driver, and then use SoC specific >> compatible strings later if we need to (eg, because of some GPU >> subsystem SoC specifics.) >> >> So, I'd suggest that we update the documentation and add: >> >> "vivante,gc-gpu-subsystem" >> >> as a common compatible now, and if necessary move on to more specific >> compatibles if we need to later. >> >> Also, I'd strongly suggest that no compatibles should contain the ID >> number of the GPU core for exactly the same reason - Vivante GPU cores >> vary according to features, and we don't want to end up with a long >> list of specific compatibles (eg) >> "ti,gc2000-and-gc320-and-gc355-gpu-subsystem" because TI >> decides to integrate a 3d, 2d and VG core. >> > All of the above sounds sensible and I would prefer if the patches are > reworked to take those things into account. Thanks for reviewing guys, the "vivante,gc-gpu-subsystem" will work perfectly fine for me, as the etnaviv driver does a great job at auto detecting the hardware.. Regards,
diff --git a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt index ed5e0a7..9fa259d 100644 --- a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt +++ b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt @@ -8,6 +8,7 @@ Required properties: - compatible: Should be one of "fsl,imx-gpu-subsystem" "marvell,dove-gpu-subsystem" + "ti,gc320-gpu-subsystem" - cores: Should contain a list of phandles pointing to Vivante GPU devices example: diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c index a6799b0..ce51270 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c @@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev) static const struct of_device_id dt_match[] = { { .compatible = "fsl,imx-gpu-subsystem" }, { .compatible = "marvell,dove-gpu-subsystem" }, + { .compatible = "ti,gc320-gpu-subsystem" }, {} }; MODULE_DEVICE_TABLE(of, dt_match);
Signed-off-by: Robert Nelson <robertcnelson@gmail.com> CC: Julien <jboulnois@gmail.com> CC: Christian Gmeiner <christian.gmeiner@gmail.com> CC: Russell King <rmk+kernel@arm.linux.org.uk> CC: Lucas Stach <l.stach@pengutronix.de> CC: Nishanth Menon <nm@ti.com> CC: Tomi Valkeinen <tomi.valkeinen@ti.com> CC: Tony Lindgren <tony@atomide.com> --- Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 1 + drivers/gpu/drm/etnaviv/etnaviv_drv.c | 1 + 2 files changed, 2 insertions(+)