On Fri, Jul 10, 2020 at 4:11 PM Jonathan Marek <jonathan@xxxxxxxx> wrote: > > On 7/9/20 11:15 AM, Rob Clark wrote: > > On Thu, Jul 9, 2020 at 7:35 AM Jonathan Marek <jonathan@xxxxxxxx> wrote: > >> > >> Check for errors instead of silently not using icc if the msm driver > >> probes before the interconnect driver. > >> > >> Allow ENODATA for ocmem path, as it is optional and this error > >> is returned when "gfx-mem" path is provided but not "ocmem". > >> > >> Remove the WARN_ON in msm_gpu_cleanup because INIT_LIST_HEAD won't have > >> been called on the list yet when going through the defer error path. > >> > >> Changes in v2: > >> * Changed to not only check for EPROBE_DEFER > >> > >> Signed-off-by: Jonathan Marek <jonathan@xxxxxxxx> > >> --- > >> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 17 ++++++++++++++--- > >> drivers/gpu/drm/msm/msm_gpu.c | 2 -- > >> 2 files changed, 14 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > >> index 89673c7ed473..0f5217202eb5 100644 > >> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > >> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > >> @@ -940,12 +940,20 @@ static int adreno_get_pwrlevels(struct device *dev, > >> */ > >> gpu->icc_path = of_icc_get(dev, NULL); > >> } > >> - if (IS_ERR(gpu->icc_path)) > >> + if (IS_ERR(gpu->icc_path)) { > >> + ret = PTR_ERR(gpu->icc_path); > >> gpu->icc_path = NULL; > >> + return ret; > >> + } > >> > >> gpu->ocmem_icc_path = of_icc_get(dev, "ocmem"); > >> - if (IS_ERR(gpu->ocmem_icc_path)) > >> + if (IS_ERR(gpu->ocmem_icc_path)) { > >> + ret = PTR_ERR(gpu->ocmem_icc_path); > >> gpu->ocmem_icc_path = NULL; > >> + /* allow -ENODATA, ocmem icc is optional */ > >> + if (ret != -ENODATA) > >> + return ret; > >> + } > >> > >> return 0; > >> } > >> @@ -996,6 +1004,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > >> struct adreno_platform_config *config = pdev->dev.platform_data; > >> struct msm_gpu_config adreno_gpu_config = { 0 }; > >> struct msm_gpu *gpu = &adreno_gpu->base; > >> + int ret; > >> > >> adreno_gpu->funcs = funcs; > >> adreno_gpu->info = adreno_info(config->rev); > >> @@ -1007,7 +1016,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > >> > >> adreno_gpu_config.nr_rings = nr_rings; > >> > >> - adreno_get_pwrlevels(&pdev->dev, gpu); > >> + ret = adreno_get_pwrlevels(&pdev->dev, gpu); > >> + if (ret) > >> + return ret; > >> > >> pm_runtime_set_autosuspend_delay(&pdev->dev, > >> adreno_gpu->info->inactive_period); > >> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c > >> index a22d30622306..ccf9a0dd9706 100644 > >> --- a/drivers/gpu/drm/msm/msm_gpu.c > >> +++ b/drivers/gpu/drm/msm/msm_gpu.c > >> @@ -959,8 +959,6 @@ void msm_gpu_cleanup(struct msm_gpu *gpu) > >> > >> DBG("%s", gpu->name); > >> > >> - WARN_ON(!list_empty(&gpu->active_list)); > >> - > > > > hmm, not a huge fan of removing the WARN_ON().. can we just init the > > list head earlier? > > > > There doesn't seem to be a nice way of doing that. Would it be > reasonable to instead detect that msm_gpu_init wasn't called (checking > if gpu->dev is NULL?), and just skip the msm_gpu_cleanup() call in > adreno_gpu_cleanup() in that case? Hmm, you can't just call msm_gpu_init() before looking up the icc path in adreno_gpu_init()? BR, -R > > > BR, > > -R > > > >> for (i = 0; i < ARRAY_SIZE(gpu->rb); i++) { > >> msm_ringbuffer_destroy(gpu->rb[i]); > >> gpu->rb[i] = NULL; > >> -- > >> 2.26.1 > >> _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel