* [PATCH v2] drm/msm: handle for EPROBE_DEFER for of_icc_get
@ 2020-07-09 14:34 Jonathan Marek
2020-07-09 15:15 ` Rob Clark
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Marek @ 2020-07-09 14:34 UTC (permalink / raw)
To: freedreno
Cc: Rob Clark, Sean Paul, David Airlie, Daniel Vetter, Jordan Crouse,
Bjorn Andersson, Brian Masney, Fabio Estevam,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVER FOR MSM ADRENO GPU, open list
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@marek.ca>
---
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));
-
for (i = 0; i < ARRAY_SIZE(gpu->rb); i++) {
msm_ringbuffer_destroy(gpu->rb[i]);
gpu->rb[i] = NULL;
--
2.26.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm/msm: handle for EPROBE_DEFER for of_icc_get
2020-07-09 14:34 [PATCH v2] drm/msm: handle for EPROBE_DEFER for of_icc_get Jonathan Marek
@ 2020-07-09 15:15 ` Rob Clark
2020-07-10 23:10 ` Jonathan Marek
0 siblings, 1 reply; 4+ messages in thread
From: Rob Clark @ 2020-07-09 15:15 UTC (permalink / raw)
To: Jonathan Marek
Cc: freedreno, Sean Paul, David Airlie, Daniel Vetter, Jordan Crouse,
Bjorn Andersson, Brian Masney, Fabio Estevam,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVER FOR MSM ADRENO GPU, open list
On Thu, Jul 9, 2020 at 7:35 AM Jonathan Marek <jonathan@marek.ca> 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@marek.ca>
> ---
> 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?
BR,
-R
> for (i = 0; i < ARRAY_SIZE(gpu->rb); i++) {
> msm_ringbuffer_destroy(gpu->rb[i]);
> gpu->rb[i] = NULL;
> --
> 2.26.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm/msm: handle for EPROBE_DEFER for of_icc_get
2020-07-09 15:15 ` Rob Clark
@ 2020-07-10 23:10 ` Jonathan Marek
2020-07-10 23:29 ` Rob Clark
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Marek @ 2020-07-10 23:10 UTC (permalink / raw)
To: Rob Clark
Cc: freedreno, Sean Paul, David Airlie, Daniel Vetter, Jordan Crouse,
Bjorn Andersson, Brian Masney, Fabio Estevam,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVER FOR MSM ADRENO GPU, open list
On 7/9/20 11:15 AM, Rob Clark wrote:
> On Thu, Jul 9, 2020 at 7:35 AM Jonathan Marek <jonathan@marek.ca> 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@marek.ca>
>> ---
>> 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?
> BR,
> -R
>
>> for (i = 0; i < ARRAY_SIZE(gpu->rb); i++) {
>> msm_ringbuffer_destroy(gpu->rb[i]);
>> gpu->rb[i] = NULL;
>> --
>> 2.26.1
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] drm/msm: handle for EPROBE_DEFER for of_icc_get
2020-07-10 23:10 ` Jonathan Marek
@ 2020-07-10 23:29 ` Rob Clark
0 siblings, 0 replies; 4+ messages in thread
From: Rob Clark @ 2020-07-10 23:29 UTC (permalink / raw)
To: Jonathan Marek
Cc: freedreno, Sean Paul, David Airlie, Daniel Vetter, Jordan Crouse,
Bjorn Andersson, Brian Masney, Fabio Estevam,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVER FOR MSM ADRENO GPU, open list
On Fri, Jul 10, 2020 at 4:11 PM Jonathan Marek <jonathan@marek.ca> wrote:
>
> On 7/9/20 11:15 AM, Rob Clark wrote:
> > On Thu, Jul 9, 2020 at 7:35 AM Jonathan Marek <jonathan@marek.ca> 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@marek.ca>
> >> ---
> >> 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
> >>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-10 23:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09 14:34 [PATCH v2] drm/msm: handle for EPROBE_DEFER for of_icc_get Jonathan Marek
2020-07-09 15:15 ` Rob Clark
2020-07-10 23:10 ` Jonathan Marek
2020-07-10 23:29 ` Rob Clark
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).