| Message ID | 20190618202425.15259-6-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | drm/msm: mdp5+dpu interconnect support | expand |
On Tue, Jun 18, 2019 at 2:25 PM Rob Clark <robdclark@gmail.com> wrote: > > From: Georgi Djakov <georgi.djakov@linaro.org> > > 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 <georgi.djakov@linaro.org> > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > 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 > Freedreno@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno
On Tue, Jun 18, 2019 at 1:44 PM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote: > > On Tue, Jun 18, 2019 at 2:25 PM Rob Clark <robdclark@gmail.com> wrote: > > > > From: Georgi Djakov <georgi.djakov@linaro.org> > > > > 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 <georgi.djakov@linaro.org> > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > 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 > > Freedreno@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/freedreno
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); }