On 2022-08-05 14:56:30, Dmitry Baryshkov wrote: > The commit 6874f48bb8b0 ("drm/msm: make mdp5/dpu devices master > components") changed the MDP5 driver to look for the interconnect paths > in the MDSS device rather than in the MDP5 device itself. This was left > unnoticed since on my testing devices the interconnects probably didn't > reach the sync state. > > Rather than just using the MDP5 device for ICC path lookups for the MDP5 > devices, introduce an additional helper to check both MDP5/DPU and MDSS > nodes. This will be helpful for the MDP5->DPU conversion, since the > driver will have to check both nodes. > > Fixes: 6874f48bb8b0 ("drm/msm: make mdp5/dpu devices master components") > Reported-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx> > Reported-by: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> Tested-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx> # On sdm630 But I'm not sure about giving my Reviewed-by to this, as I'd rather *correct* the DT bindings for sdm630 and msm8996 to provide interconnects in the MDSS node unless there are strong reasons not to (and I don't consider "backwards compatibility" to be one, this binding "never even existed" if mdp5.txt is to be believed). - Marijn > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 7 ++----- > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 9 +++------ > drivers/gpu/drm/msm/msm_drv.h | 2 ++ > drivers/gpu/drm/msm/msm_io_utils.c | 22 ++++++++++++++++++++++ > 4 files changed, 29 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index e23e2552e802..9eff6c2b1917 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -383,12 +383,9 @@ static int dpu_kms_parse_data_bus_icc_path(struct dpu_kms *dpu_kms) > struct icc_path *path1; > struct drm_device *dev = dpu_kms->dev; > struct device *dpu_dev = dev->dev; > - struct device *mdss_dev = dpu_dev->parent; > > - /* Interconnects are a part of MDSS device tree binding, not the > - * MDP/DPU device. */ > - path0 = of_icc_get(mdss_dev, "mdp0-mem"); > - path1 = of_icc_get(mdss_dev, "mdp1-mem"); > + path0 = msm_icc_get(dpu_dev, "mdp0-mem"); > + path1 = msm_icc_get(dpu_dev, "mdp1-mem"); > > if (IS_ERR_OR_NULL(path0)) > return PTR_ERR_OR_ZERO(path0); > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > index 3d5621a68f85..b0c372fef5d5 100644 > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > @@ -921,12 +921,9 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev) > > static int mdp5_setup_interconnect(struct platform_device *pdev) > { > - /* Interconnects are a part of MDSS device tree binding, not the > - * MDP5 device. */ > - struct device *mdss_dev = pdev->dev.parent; > - struct icc_path *path0 = of_icc_get(mdss_dev, "mdp0-mem"); > - struct icc_path *path1 = of_icc_get(mdss_dev, "mdp1-mem"); > - struct icc_path *path_rot = of_icc_get(mdss_dev, "rotator-mem"); > + struct icc_path *path0 = msm_icc_get(&pdev->dev, "mdp0-mem"); > + struct icc_path *path1 = msm_icc_get(&pdev->dev, "mdp1-mem"); > + struct icc_path *path_rot = msm_icc_get(&pdev->dev, "rotator-mem"); > > if (IS_ERR(path0)) > return PTR_ERR(path0); > diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h > index 08388d742d65..d38510f6dbf5 100644 > --- a/drivers/gpu/drm/msm/msm_drv.h > +++ b/drivers/gpu/drm/msm/msm_drv.h > @@ -441,6 +441,8 @@ void __iomem *msm_ioremap_size(struct platform_device *pdev, const char *name, > phys_addr_t *size); > void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name); > > +struct icc_path *msm_icc_get(struct device *dev, const char *name); > + > #define msm_writel(data, addr) writel((data), (addr)) > #define msm_readl(addr) readl((addr)) > > diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c > index 7b504617833a..d02cd29ce829 100644 > --- a/drivers/gpu/drm/msm/msm_io_utils.c > +++ b/drivers/gpu/drm/msm/msm_io_utils.c > @@ -5,6 +5,8 @@ > * Author: Rob Clark <robdclark@xxxxxxxxx> > */ > > +#include <linux/interconnect.h> > + > #include "msm_drv.h" > > /* > @@ -124,3 +126,23 @@ void msm_hrtimer_work_init(struct msm_hrtimer_work *work, > work->worker = worker; > kthread_init_work(&work->work, fn); > } > + > +struct icc_path *msm_icc_get(struct device *dev, const char *name) > +{ > + struct device *mdss_dev = dev->parent; > + struct icc_path *path; > + > + path = of_icc_get(dev, name); > + if (path) > + return path; > + > + /* > + * If there are no interconnects attached to the corresponding device > + * node, of_icc_get() will return NULL. > + * > + * If the MDP5/DPU device node doesn't have interconnects, lookup the > + * path in the parent (MDSS) device. > + */ > + return of_icc_get(mdss_dev, name); > + > +} > -- > 2.35.1 >