All of lore.kernel.org
 help / color / mirror / Atom feed
From: barnabas.czeman@mainlining.org
To: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>,
	Robert Foss <rfoss@kernel.org>, Todor Tomov <todor.too@gmail.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Konrad Dybcio <konradybcio@kernel.org>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Yassine Oudjana <y.oudjana@protonmail.com>
Subject: Re: [PATCH v2] media: qcom: camss: fix VFE pm domain off
Date: Sat, 30 Nov 2024 00:52:18 +0100	[thread overview]
Message-ID: <c7a9a43eea8bd1e6302ae4fa2d79dd80@mainlining.org> (raw)
In-Reply-To: <93028653-9919-460e-83d3-84bf5ade56d4@linaro.org>

On 2024-11-30 00:07, Bryan O'Donoghue wrote:
> On 29/11/2024 22:45, barnabas.czeman@mainlining.org wrote:
>> On 2024-11-29 23:08, Bryan O'Donoghue wrote:
>>> On 29/11/2024 13:46, barnabas.czeman@mainlining.org wrote:
>>>> On 2024-11-29 13:25, Bryan O'Donoghue wrote:
>>>>> On 29/11/2024 11:44, barnabas.czeman@mainlining.org wrote:
>>>>>>> The change does not describe how to reproduce the problem, which 
>>>>>>> commit
>>>>>>> base is tested, which platform is testes, there is no enough 
>>>>>>> information,
>>>>>>> unfortunately.
>>>>>> I can reproduce the problem with megapixels-sensorprofile on 
>>>>>> msm8953 and
>>>>>> it can be reproduced with megapixels on msm8996.
>>>>>> The base is the last commit on next.
>>>>> 
>>>>> Can you verify if vfe_domain_on has run and if so whether or not 
>>>>> genpd_link is NULL when that function exists.
>>>>> 
>>>> I have added some debug logs it seems pm_domain_on and pm_domain_off 
>>>> is called twice on the same object.
>>>> [   63.473360] qcom-camss 1b00020.camss: pm_domain_on 19842ce8 link 
>>>> 42973800
>>>> [   63.481524] qcom-camss 1b00020.camss: pm_domain_on 19840080 link 
>>>> 4e413800
>>>> [   63.481555] qcom-camss 1b00020.camss: pm_domain_on 19842ce8 link 
>>>> 42973800
>>>> [   63.481632] qcom-camss 1b00020.camss: pm_domain_off 19840080 link 
>>>> 4e413800
>>>> [   63.481641] qcom-camss 1b00020.camss: pm_domain_off 19842ce8 link 
>>>> 42973800
>>>> [   63.654004] qcom-camss 1b00020.camss: pm_domain_off 19842ce8 link 
>>>> 0
>>>>> That's the question.
>>>>> 
>>>>> ---
>>>>> bod
>>> 
>>> Could you provide this output ?
>>> 
>>> index 80a62ba112950..b25b8f6b00be1 100644
>>> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
>>> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
>>> @@ -595,6 +595,9 @@ void vfe_isr_reset_ack(struct vfe_device *vfe)
>>>   */
>>>  void vfe_pm_domain_off(struct vfe_device *vfe)
>>>  {
>>> +dev_info(camss->dev, "%s VFE %d genpd %pK genpd_link %pK\n",
>>> +        __func__, vfe->id, vfe->genpd, vfe->genpd_link);
>>> +
>>>         if (!vfe->genpd)
>>>                 return;
>>> 
>>> @@ -609,7 +612,8 @@ void vfe_pm_domain_off(struct vfe_device *vfe)
>>>  int vfe_pm_domain_on(struct vfe_device *vfe)
>>>  {
>>>         struct camss *camss = vfe->camss;
>>> -
>>> +dev_info(camss->dev, "%s VFE %d genpd %pK genpd_link %pK\n",
>>> +        __func__, vfe->id, vfe->genpd, vfe->genpd_link);
>>>         if (!vfe->genpd)
>>>                 return 0;
>>> 
>>> ---
>>> bod
>> I think logging in pm_domain_on should be placed after device_link_add 
>> because only NULL
>> will be visible.
>> [   83.040694] qcom-camss 1b00020.camss: vfe_pm_domain_on VFE 1 genpd 
>> 000000009bd8355f genpd_link 0000000000000000
>> [   83.049293] qcom-camss 1b00020.camss: vfe_pm_domain_on VFE 0 genpd 
>> 00000000bfb65e7c genpd_link 0000000000000000
>> [   83.049353] qcom-camss 1b00020.camss: vfe_pm_domain_on VFE 1 genpd 
>> 000000009bd8355f genpd_link 00000000ccb0acd9
>> [   83.049641] qcom-camss 1b00020.camss: vfe_pm_domain_off VFE 0 genpd 
>> 00000000bfb65e7c genpd_link 00000000348ac3c1
>> [   83.049654] qcom-camss 1b00020.camss: vfe_pm_domain_off VFE 1 genpd 
>> 000000009bd8355f genpd_link 00000000ccb0acd9
>> [   83.241498] qcom-camss 1b00020.camss: vfe_pm_domain_off VFE 1 genpd 
>> 000000009bd8355f genpd_link 0000000000000000
> 
> Could you add
> 
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -786,7 +786,7 @@ int vfe_get(struct vfe_device *vfe)
>         int ret;
> 
>         mutex_lock(&vfe->power_lock);
> -
> +dev_info(vfe->camss->dev, "%s vfe %d power_count %d\n", __func__, 
> vfe->id, vfe->power_count);
>         if (vfe->power_count == 0) {
>                 ret = vfe->res->hw_ops->pm_domain_on(vfe);
>                 if (ret < 0)
> @@ -823,6 +823,7 @@ int vfe_get(struct vfe_device *vfe)
> 
>         mutex_unlock(&vfe->power_lock);
> 
> +dev_info(camss->vfe->dev, "%s vfe %d err=%d\n", __func__, 
> camss->vfe->id, 0);
>         return 0;
> 
>  error_reset:
> @@ -835,7 +836,7 @@ int vfe_get(struct vfe_device *vfe)
> 
>  error_pm_domain:
>         mutex_unlock(&vfe->power_lock);
> -
> +dev_info(camss->vfe->dev, "%s vfe %d err=%d\n", __func__, 
> camss->vfe->id, ret);
>         return ret;
>  }
> 
> ?
> 
> ---
> bod
I have added little more from the logs because it is only failing in 
edge cases megapixels-sensorprofile failing by
different reason quickly and trying to release the device.
[   54.719030] qcom-camss 1b00020.camss: vfe_get vfe 0 err=0
[   54.750124] qcom-camss 1b00020.camss: vfe_get vfe 0 power_count 1
[   54.750236] qcom-camss 1b00020.camss: vfe_get vfe 0 err=0
[   54.751270] qcom-camss 1b00020.camss: vfe_pm_domain_on VFE 0 genpd 
00000000beaef03c genpd_link 00000000251644d9
[   54.751433] qcom-camss 1b00020.camss: vfe_pm_domain_on VFE 1 genpd 
000000007ce2da53 genpd_link 0000000000000000
[   54.755463] qcom-camss 1b00020.camss: vfe_pm_domain_off VFE 0 genpd 
00000000beaef03c genpd_link 00000000251644d9
[   54.755531] qcom-camss 1b00020.camss: vfe_pm_domain_off VFE 1 genpd 
000000007ce2da53 genpd_link 0000000058dcd4d6
[   55.861084] qcom-camss 1b00020.camss: vfe_get vfe 0 power_count 1
[   55.861177] qcom-camss 1b00020.camss: vfe_get vfe 0 err=0
[   55.861778] qcom-camss 1b00020.camss: vfe_pm_domain_on VFE 0 genpd 
00000000beaef03c genpd_link 0000000000000000
[   55.861860] qcom-camss 1b00020.camss: vfe_pm_domain_on VFE 1 genpd 
000000007ce2da53 genpd_link 0000000000000000
[   55.862242] qcom-camss 1b00020.camss: vfe_pm_domain_off VFE 0 genpd 
00000000beaef03c genpd_link 00000000251644d9
[   55.862258] qcom-camss 1b00020.camss: vfe_pm_domain_off VFE 1 genpd 
000000007ce2da53 genpd_link 00000000e41c1881
[  143.911690] qcom-camss 1b00020.camss: vfe_get vfe 0 power_count 2
[  143.911762] qcom-camss 1b00020.camss: vfe_get vfe 0 err=0
[  143.911800] qcom-camss 1b00020.camss: vfe_get vfe 0 power_count 3
[  143.911821] qcom-camss 1b00020.camss: vfe_get vfe 0 err=0
[  143.911858] qcom-camss 1b00020.camss: vfe_get vfe 1 power_count 0
[  143.911871] qcom-camss 1b00020.camss: vfe_pm_domain_on VFE 1 genpd 
000000007ce2da53 genpd_link 0000000000000000
[  143.912393] qcom-camss 1b00020.camss: vfe_get vfe 1 err=0
[  143.912489] qcom-camss 1b00020.camss: vfe_get vfe 1 power_count 1
[  143.912513] qcom-camss 1b00020.camss: vfe_get vfe 1 err=0
[  143.912553] qcom-camss 1b00020.camss: vfe_get vfe 1 power_count 2
[  143.912572] qcom-camss 1b00020.camss: vfe_get vfe 1 err=0
[  143.921750] qcom-camss 1b00020.camss: vfe_get vfe 0 power_count 3
[  143.921802] qcom-camss 1b00020.camss: vfe_get vfe 0 err=0
[  143.922670] qcom-camss 1b00020.camss: vfe_pm_domain_on VFE 0 genpd 
00000000beaef03c genpd_link 0000000000000000
[  143.922725] qcom-camss 1b00020.camss: vfe_pm_domain_on VFE 1 genpd 
000000007ce2da53 genpd_link 00000000d1fcd54b
[  143.922853] qcom-camss 1b00020.camss: vfe_pm_domain_off VFE 0 genpd 
00000000beaef03c genpd_link 00000000251644d9
[  143.922868] qcom-camss 1b00020.camss: vfe_pm_domain_off VFE 1 genpd 
000000007ce2da53 genpd_link 00000000d1fcd54b
[  144.126535] qcom-camss 1b00020.camss: vfe_pm_domain_off VFE 1 genpd 
000000007ce2da53 genpd_link 0000000000000000

  reply	other threads:[~2024-11-29 23:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-28 19:39 [PATCH v2] media: qcom: camss: fix VFE pm domain off Barnabás Czémán
2024-11-29  8:48 ` Vladimir Zapolskiy
2024-11-29 11:06   ` Bryan O'Donoghue
2024-11-29 11:20     ` Vladimir Zapolskiy
2024-11-29 11:23       ` Bryan O'Donoghue
2024-11-29 11:44       ` barnabas.czeman
2024-11-29 12:25         ` Bryan O'Donoghue
2024-11-29 13:46           ` barnabas.czeman
2024-11-29 22:08             ` Bryan O'Donoghue
2024-11-29 22:45               ` barnabas.czeman
2024-11-29 23:07                 ` Bryan O'Donoghue
2024-11-29 23:52                   ` barnabas.czeman [this message]
2024-11-30 21:48                     ` Bryan O'Donoghue
2024-11-30 22:58                       ` barnabas.czeman
2024-12-02 23:10                         ` Bryan O'Donoghue
2024-12-03  1:02                           ` barnabas.czeman
2024-12-03 16:49                             ` Bryan O'Donoghue
2024-12-03 21:41                               ` barnabas.czeman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c7a9a43eea8bd1e6302ae4fa2d79dd80@mainlining.org \
    --to=barnabas.czeman@mainlining.org \
    --cc=bryan.odonoghue@linaro.org \
    --cc=hverkuil@xs4all.nl \
    --cc=konradybcio@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=rfoss@kernel.org \
    --cc=todor.too@gmail.com \
    --cc=vladimir.zapolskiy@linaro.org \
    --cc=y.oudjana@protonmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.