Re: [Freedreno] [PATCH 5/5] drm/msm/mdp5: Use the interconnect API

[Rob Clark]

On Tue, Jun 18, 2019 at 1:44 PM Jeffrey Hugo  wrote:
>
> On Tue, Jun 18, 2019 at 2:25 PM Rob Clark  wrote:
> >
> > From: Georgi Djakov 
> >
> > The interconnect API provides an interface for consumer drivers to
> > express their bandwidth needs in the SoC. This data is aggregated
> > and the on-chip interconnect hardware is configured to the most
> > appropriate power/performance profile.
> >
> > Use the API to configure the interconnects and request bandwidth
> > between DDR and the display hardware (MDP port(s) and rotator
> > downscaler).
> >
> > v2: update the path names to be consistent with dpu, handle the NULL
> > path case, updated commit msg from Georgi.
> >
> > Signed-off-by: Georgi Djakov 
> > Signed-off-by: Rob Clark 
> > ---
> >  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c 
> > b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > index 97179bec8902..eeac429acf40 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > @@ -16,6 +16,7 @@
> >   * this program.  If not, see .
> >   */
> >
> > +#include 
> >  #include 
> >
> >  #include "msm_drv.h"
> > @@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {
> >
> >  static int mdp5_dev_probe(struct platform_device *pdev)
> >  {
> > +   struct icc_path *path0 = of_icc_get(&pdev->dev, "mdp0-mem");
> > +   struct icc_path *path1 = of_icc_get(&pdev->dev, "mdp1-mem");
> > +   struct icc_path *path_rot = of_icc_get(&pdev->dev, "rotator-mem");
> > +
> > +   if (IS_ERR_OR_NULL(path0))
> > +   return PTR_ERR_OR_ZERO(path0);
>
> Umm, am I misunderstanding something?  It seems like of_icc_get()
> returns NULL if the property doesn't exist.  Won't this be backwards
> incompatible?  Existing DTs won't specify the property, and I don't
> believe the property is supported on all targets.  Seems like we'll
> break things by not calling the below component_add() if the
> interconnect is not supported, specified, or the interconnect driver
> is not compiled.

hmm, right, I guess I should test this w/out the dts patch.. probably
should just revert back to the previous logic..

BR,
-R

> > +   icc_set_bw(path0, 0, MBps_to_icc(6400));
> > +
> > +   if (!IS_ERR_OR_NULL(path1))
> > +   icc_set_bw(path1, 0, MBps_to_icc(6400));
> > +   if (!IS_ERR_OR_NULL(path_rot))
> > +   icc_set_bw(path_rot, 0, MBps_to_icc(6400));
> > +
> > DBG("");
> > return component_add(&pdev->dev, &mdp5_ops);
> >  }
> > --
> > 2.20.1
> >
> > ___
> > Freedreno mailing list
> > Freedreno@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/freedreno
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 5/5] drm/msm/mdp5: Use the interconnect API

[Jeffrey Hugo]

On Tue, Jun 18, 2019 at 2:25 PM Rob Clark  wrote:
>
> From: Georgi Djakov 
>
> The interconnect API provides an interface for consumer drivers to
> express their bandwidth needs in the SoC. This data is aggregated
> and the on-chip interconnect hardware is configured to the most
> appropriate power/performance profile.
>
> Use the API to configure the interconnects and request bandwidth
> between DDR and the display hardware (MDP port(s) and rotator
> downscaler).
>
> v2: update the path names to be consistent with dpu, handle the NULL
> path case, updated commit msg from Georgi.
>
> Signed-off-by: Georgi Djakov 
> Signed-off-by: Rob Clark 
> ---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c 
> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 97179bec8902..eeac429acf40 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -16,6 +16,7 @@
>   * this program.  If not, see .
>   */
>
> +#include 
>  #include 
>
>  #include "msm_drv.h"
> @@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {
>
>  static int mdp5_dev_probe(struct platform_device *pdev)
>  {
> +   struct icc_path *path0 = of_icc_get(&pdev->dev, "mdp0-mem");
> +   struct icc_path *path1 = of_icc_get(&pdev->dev, "mdp1-mem");
> +   struct icc_path *path_rot = of_icc_get(&pdev->dev, "rotator-mem");
> +
> +   if (IS_ERR_OR_NULL(path0))
> +   return PTR_ERR_OR_ZERO(path0);

Umm, am I misunderstanding something?  It seems like of_icc_get()
returns NULL if the property doesn't exist.  Won't this be backwards
incompatible?  Existing DTs won't specify the property, and I don't
believe the property is supported on all targets.  Seems like we'll
break things by not calling the below component_add() if the
interconnect is not supported, specified, or the interconnect driver
is not compiled.

> +   icc_set_bw(path0, 0, MBps_to_icc(6400));
> +
> +   if (!IS_ERR_OR_NULL(path1))
> +   icc_set_bw(path1, 0, MBps_to_icc(6400));
> +   if (!IS_ERR_OR_NULL(path_rot))
> +   icc_set_bw(path_rot, 0, MBps_to_icc(6400));
> +
> DBG("");
> return component_add(&pdev->dev, &mdp5_ops);
>  }
> --
> 2.20.1
>
> ___
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 5/5] drm/msm/mdp5: Use the interconnect API

[Rob Clark]

From: Georgi Djakov 

The interconnect API provides an interface for consumer drivers to
express their bandwidth needs in the SoC. This data is aggregated
and the on-chip interconnect hardware is configured to the most
appropriate power/performance profile.

Use the API to configure the interconnects and request bandwidth
between DDR and the display hardware (MDP port(s) and rotator
downscaler).

v2: update the path names to be consistent with dpu, handle the NULL
path case, updated commit msg from Georgi.

Signed-off-by: Georgi Djakov 
Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 97179bec8902..eeac429acf40 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -16,6 +16,7 @@
  * this program.  If not, see .
  */
 
+#include 
 #include 
 
 #include "msm_drv.h"
@@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {
 
 static int mdp5_dev_probe(struct platform_device *pdev)
 {
+   struct icc_path *path0 = of_icc_get(&pdev->dev, "mdp0-mem");
+   struct icc_path *path1 = of_icc_get(&pdev->dev, "mdp1-mem");
+   struct icc_path *path_rot = of_icc_get(&pdev->dev, "rotator-mem");
+
+   if (IS_ERR_OR_NULL(path0))
+   return PTR_ERR_OR_ZERO(path0);
+   icc_set_bw(path0, 0, MBps_to_icc(6400));
+
+   if (!IS_ERR_OR_NULL(path1))
+   icc_set_bw(path1, 0, MBps_to_icc(6400));
+   if (!IS_ERR_OR_NULL(path_rot))
+   icc_set_bw(path_rot, 0, MBps_to_icc(6400));
+
DBG("");
return component_add(&pdev->dev, &mdp5_ops);
 }
-- 
2.20.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 5/5] drm/msm/mdp5: Use the interconnect API

[Georgi Djakov]

On 5/8/19 23:42, Rob Clark wrote:
> From: Georgi Djakov 
> 

Let's put some text in the commit message:

The interconnect API provides an interface for consumer drivers to express
their bandwidth needs in the SoC. This data is aggregated and the on-chip
interconnect hardware is configured to the most appropriate power/performance
profile.

Use the API to configure the interconnects and request bandwidth between DDR and
the display hardware (MDP port(s) and rotator downscaler).


> Signed-off-by: Georgi Djakov 
> Signed-off-by: Rob Clark 
> ---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c 
> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 97179bec8902..54d2b4c2b09f 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -16,6 +16,7 @@
>   * this program.  If not, see .
>   */
>  
> +#include 
>  #include 
>  
>  #include "msm_drv.h"
> @@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {
>  
>  static int mdp5_dev_probe(struct platform_device *pdev)
>  {
> + struct icc_path *path0 = of_icc_get(&pdev->dev, "port0");
> + struct icc_path *path1 = of_icc_get(&pdev->dev, "port1");
> + struct icc_path *path_rot = of_icc_get(&pdev->dev, "rotator");

It would be better change just the names to "mdp0-mem', "mdp1-mem",
"rotator-mem" for consistency and denote that the path target is the DDR memory.

> +
> + if (IS_ERR(path0))
> + return PTR_ERR(path0);
> + icc_set_bw(path0, 0, MBps_to_icc(6400));
> +
> + if (!IS_ERR(path1))
> + icc_set_bw(path1, 0, MBps_to_icc(6400));
> + if (!IS_ERR(path_rot))
> + icc_set_bw(path_rot, 0, MBps_to_icc(6400));
> +
>   DBG("");
>   return component_add(&pdev->dev, &mdp5_ops);
>  }
> 

Thanks,
Georgi
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 5/5] drm/msm/mdp5: Use the interconnect API

[Sean Paul]

On Wed, May 08, 2019 at 01:42:15PM -0700, Rob Clark wrote:
> From: Georgi Djakov 
> 
> Signed-off-by: Georgi Djakov 
> Signed-off-by: Rob Clark 
> ---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c 
> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 97179bec8902..54d2b4c2b09f 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -16,6 +16,7 @@
>   * this program.  If not, see .
>   */
>  
> +#include 
>  #include 
>  
>  #include "msm_drv.h"
> @@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {
>  
>  static int mdp5_dev_probe(struct platform_device *pdev)
>  {
> + struct icc_path *path0 = of_icc_get(&pdev->dev, "port0");
> + struct icc_path *path1 = of_icc_get(&pdev->dev, "port1");
> + struct icc_path *path_rot = of_icc_get(&pdev->dev, "rotator");
> +
> + if (IS_ERR(path0))

Same comments here re: NULL value

> + return PTR_ERR(path0);
> + icc_set_bw(path0, 0, MBps_to_icc(6400));
> +
> + if (!IS_ERR(path1))
> + icc_set_bw(path1, 0, MBps_to_icc(6400));
> + if (!IS_ERR(path_rot))
> + icc_set_bw(path_rot, 0, MBps_to_icc(6400));
> +
>   DBG("");
>   return component_add(&pdev->dev, &mdp5_ops);
>  }
> -- 
> 2.20.1
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 5/5] drm/msm/mdp5: Use the interconnect API

[Rob Clark]

From: Georgi Djakov 

Signed-off-by: Georgi Djakov 
Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 97179bec8902..54d2b4c2b09f 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -16,6 +16,7 @@
  * this program.  If not, see .
  */
 
+#include 
 #include 
 
 #include "msm_drv.h"
@@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {
 
 static int mdp5_dev_probe(struct platform_device *pdev)
 {
+   struct icc_path *path0 = of_icc_get(&pdev->dev, "port0");
+   struct icc_path *path1 = of_icc_get(&pdev->dev, "port1");
+   struct icc_path *path_rot = of_icc_get(&pdev->dev, "rotator");
+
+   if (IS_ERR(path0))
+   return PTR_ERR(path0);
+   icc_set_bw(path0, 0, MBps_to_icc(6400));
+
+   if (!IS_ERR(path1))
+   icc_set_bw(path1, 0, MBps_to_icc(6400));
+   if (!IS_ERR(path_rot))
+   icc_set_bw(path_rot, 0, MBps_to_icc(6400));
+
DBG("");
return component_add(&pdev->dev, &mdp5_ops);
 }
-- 
2.20.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno