From: Georgi Djakov <[email protected]>
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 <[email protected]>
Signed-off-by: Rob Clark <[email protected]>
---
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 <http://www.gnu.org/licenses/>.
*/
+#include <linux/interconnect.h>
#include <linux/of_irq.h>
#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
On Tue, Jun 18, 2019 at 2:25 PM Rob Clark <[email protected]> wrote:
>
> From: Georgi Djakov <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Rob Clark <[email protected]>
> ---
> 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 <http://www.gnu.org/licenses/>.
> */
>
> +#include <linux/interconnect.h>
> #include <linux/of_irq.h>
>
> #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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/freedreno
On Tue, Jun 18, 2019 at 1:44 PM Jeffrey Hugo <[email protected]> wrote:
>
> On Tue, Jun 18, 2019 at 2:25 PM Rob Clark <[email protected]> wrote:
> >
> > From: Georgi Djakov <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Rob Clark <[email protected]>
> > ---
> > 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 <http://www.gnu.org/licenses/>.
> > */
> >
> > +#include <linux/interconnect.h>
> > #include <linux/of_irq.h>
> >
> > #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
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/freedreno
From: Georgi Djakov <[email protected]>
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.
v3: split out icc setup into it's own function, and rework logic
slightly so no interconnect paths is not fatal.
Signed-off-by: Georgi Djakov <[email protected]>
Signed-off-by: Rob Clark <[email protected]>
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 38 ++++++++++++++++++++++++
1 file changed, 38 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..1c55401956c4 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 <http://www.gnu.org/licenses/>.
*/
+#include <linux/interconnect.h>
#include <linux/of_irq.h>
#include "msm_drv.h"
@@ -1048,9 +1049,46 @@ static const struct component_ops mdp5_ops = {
.unbind = mdp5_unbind,
};
+static int mdp5_setup_interconnect(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(path0))
+ return PTR_ERR(path0);
+
+ if (!path0) {
+ /* no interconnect support is not necessarily a fatal
+ * condition, the platform may simply not have an
+ * interconnect driver yet. But warn about it in case
+ * bootloader didn't setup bus clocks high enough for
+ * scanout.
+ */
+ dev_warn(&pdev->dev, "No interconnect support may cause display underflows!\n");
+ return 0;
+ }
+
+ 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));
+
+ return 0;
+}
+
static int mdp5_dev_probe(struct platform_device *pdev)
{
+ int ret;
+
DBG("");
+
+ ret = mdp5_setup_interconnect(pdev);
+ if (ret)
+ return ret;
+
return component_add(&pdev->dev, &mdp5_ops);
}
--
2.20.1
On Tue, Jun 18, 2019 at 4:10 PM Rob Clark <[email protected]> wrote:
>
> From: Georgi Djakov <[email protected]>
>
> 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.
> v3: split out icc setup into it's own function, and rework logic
> slightly so no interconnect paths is not fatal.
>
> Signed-off-by: Georgi Djakov <[email protected]>
> Signed-off-by: Rob Clark <[email protected]>
Looks good to me.
Reviewed-By: Jeffrey Hugo <[email protected]>