All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark yao <mark.yao@rock-chips.com>
To: Daniel Kurtz <djkurtz@chromium.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>
Subject: Re: [PATCH] drm/rockchip: vop: power off until vop standby take effect
Date: Wed, 04 Feb 2015 11:56:24 +0800	[thread overview]
Message-ID: <54D19868.8000906@rock-chips.com> (raw)
In-Reply-To: <CAGS+omAss_Xk3gmDAVRoejof5mKA=L4rWKXb_VF=n2HsUE0Ybg@mail.gmail.com>

On 2015年02月04日 11:48, Daniel Kurtz wrote:
> On Feb 4, 2015 11:38 AM, "Mark yao" <mark.yao@rock-chips.com> wrote:
>> On 2015年02月02日 15:53, Daniel Vetter wrote:
>>> On Mon, Feb 02, 2015 at 10:30:09AM +0800, Mark yao wrote:
>>>> On 2015年02月02日 10:07, Daniel Kurtz wrote:
>>>>> Hi Mark, Heiko,
>>>>>
>>>>> On Sat, Jan 31, 2015 at 4:41 PM, Mark Yao <mark.yao@rock-chips.com> wrote:
>>>>>> Vop standby will take effect end of current frame,
>>>>>> if dsp_hold_valid_irq happen, it means vop standby complete.
>>>>>>
>>>>>> we must wait standby complete when we want to disable aclk,
>>>>>> if not, memory bus maybe dead.
>>>>>>
>>>>>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>>>>>> ---
>>>>>>    drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   76 ++++++++++++++++++++++-----
>>>>>>    1 file changed, 63 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>>>> index fb25836..47ea61f 100644
>>>>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>>>> @@ -89,6 +89,7 @@ struct vop {
>>>>>>           /* mutex vsync_ work */
>>>>>>           struct mutex vsync_mutex;
>>>>>>           bool vsync_work_pending;
>>>>>> +       struct completion dsp_hold_completion;
>>>>>>
>>>>>>           const struct vop_data *data;
>>>>>>
>>>>>> @@ -382,6 +383,34 @@ static bool is_alpha_support(uint32_t format)
>>>>>>           }
>>>>>>    }
>>>>>>
>>>>>> +static void vop_dsp_hold_valid_irq_enable(struct vop *vop)
>>>>>> +{
>>>>>> +       unsigned long flags;
>>>>>> +
>>>>>> +       BUG_ON(!vop->is_enabled);
>>>>> Re: Heiko "use a WARN_ON":
>>>>>
>>>>> If the VOP clock is off, then the system will just hang when trying to
>>>>> write the VOP register so in this case, BUG_ON gives a more reliable
>>>>> crash dump than the hang.
>>>>     In this way, you are right, if vop clocks is disabled, write vop register
>>>> will hang system, and the WARN_ON
>>> if (WARN_ON(cond))
>>>          return;
>>>
>>> is what we commonly use in i915. Because it's really not any good to use
>>> BUG_ON in drm drivers, at least if you expect anyone to use fbcon on top
>>> of it (i.e. anything even remotely resembling a normal distro): Then the
>>> intial modeset is all done under console_lock, which means that _none_ of
>>> your BUG_ON will ever get out over serial console.
>>>
>>> The problem is fbcon locking, but no fool yet signed up to fix it (it's
>>> horrible, I looked ...).
>>> -Daniel
>> I know this problem, if fbcon locking, and BUG_ON, we can't get any log from serial console, Trouble
>> me many times. So know, if no drm log output, the first thing I do is remove the fbcon lock, and then
>> we can see the crash dump.
>>
>> But here, I expect to use BUG_ON, because if BUG happen, vop register will hang system, and WARN_ON
>> may be have no chance to output.
> Yeah, but danvet is suggesting we just return immediately on the
> WARN_ON condition, to avoid accessing the vop registers and hanging.
>
> Thus, the WARN_ON will have a chance to put out (unless the early
> return path goes off and finds some other unprotected path goes to
> access unlocked registers).
>
> -Dan
All right, I got it.
>> Mark
>>
>
>


-- 
Mark Yao 姚智情

Rockchip 福州瑞芯微电子有限公司

Fuzhou Headquarters

Addr:No.21 Building, A District, Fuzhou

Software Park, 89 Soft Avenue, Tongpan Road,

Gulou District, Fuzhou (350003)

地址:福建省福州市铜盘路软件大道89号软件园A区21号楼 (350003)

Tel:(86-0591)83991906 - 8296

E-mail:mark.yao@rock-chips.com


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Mark yao <mark.yao@rock-chips.com>
To: Daniel Kurtz <djkurtz@chromium.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	David Airlie <airlied@linux.ie>, Rob Clark <robdclark@gmail.com>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>
Subject: Re: [PATCH] drm/rockchip: vop: power off until vop standby take effect
Date: Wed, 04 Feb 2015 11:56:24 +0800	[thread overview]
Message-ID: <54D19868.8000906@rock-chips.com> (raw)
In-Reply-To: <CAGS+omAss_Xk3gmDAVRoejof5mKA=L4rWKXb_VF=n2HsUE0Ybg@mail.gmail.com>

On 2015年02月04日 11:48, Daniel Kurtz wrote:
> On Feb 4, 2015 11:38 AM, "Mark yao" <mark.yao@rock-chips.com> wrote:
>> On 2015年02月02日 15:53, Daniel Vetter wrote:
>>> On Mon, Feb 02, 2015 at 10:30:09AM +0800, Mark yao wrote:
>>>> On 2015年02月02日 10:07, Daniel Kurtz wrote:
>>>>> Hi Mark, Heiko,
>>>>>
>>>>> On Sat, Jan 31, 2015 at 4:41 PM, Mark Yao <mark.yao@rock-chips.com> wrote:
>>>>>> Vop standby will take effect end of current frame,
>>>>>> if dsp_hold_valid_irq happen, it means vop standby complete.
>>>>>>
>>>>>> we must wait standby complete when we want to disable aclk,
>>>>>> if not, memory bus maybe dead.
>>>>>>
>>>>>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>>>>>> ---
>>>>>>    drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   76 ++++++++++++++++++++++-----
>>>>>>    1 file changed, 63 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>>>> index fb25836..47ea61f 100644
>>>>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>>>> @@ -89,6 +89,7 @@ struct vop {
>>>>>>           /* mutex vsync_ work */
>>>>>>           struct mutex vsync_mutex;
>>>>>>           bool vsync_work_pending;
>>>>>> +       struct completion dsp_hold_completion;
>>>>>>
>>>>>>           const struct vop_data *data;
>>>>>>
>>>>>> @@ -382,6 +383,34 @@ static bool is_alpha_support(uint32_t format)
>>>>>>           }
>>>>>>    }
>>>>>>
>>>>>> +static void vop_dsp_hold_valid_irq_enable(struct vop *vop)
>>>>>> +{
>>>>>> +       unsigned long flags;
>>>>>> +
>>>>>> +       BUG_ON(!vop->is_enabled);
>>>>> Re: Heiko "use a WARN_ON":
>>>>>
>>>>> If the VOP clock is off, then the system will just hang when trying to
>>>>> write the VOP register so in this case, BUG_ON gives a more reliable
>>>>> crash dump than the hang.
>>>>     In this way, you are right, if vop clocks is disabled, write vop register
>>>> will hang system, and the WARN_ON
>>> if (WARN_ON(cond))
>>>          return;
>>>
>>> is what we commonly use in i915. Because it's really not any good to use
>>> BUG_ON in drm drivers, at least if you expect anyone to use fbcon on top
>>> of it (i.e. anything even remotely resembling a normal distro): Then the
>>> intial modeset is all done under console_lock, which means that _none_ of
>>> your BUG_ON will ever get out over serial console.
>>>
>>> The problem is fbcon locking, but no fool yet signed up to fix it (it's
>>> horrible, I looked ...).
>>> -Daniel
>> I know this problem, if fbcon locking, and BUG_ON, we can't get any log from serial console, Trouble
>> me many times. So know, if no drm log output, the first thing I do is remove the fbcon lock, and then
>> we can see the crash dump.
>>
>> But here, I expect to use BUG_ON, because if BUG happen, vop register will hang system, and WARN_ON
>> may be have no chance to output.
> Yeah, but danvet is suggesting we just return immediately on the
> WARN_ON condition, to avoid accessing the vop registers and hanging.
>
> Thus, the WARN_ON will have a chance to put out (unless the early
> return path goes off and finds some other unprotected path goes to
> access unlocked registers).
>
> -Dan
All right, I got it.
>> Mark
>>
>
>


-- 
Mark Yao 姚智情

Rockchip 福州瑞芯微电子有限公司

Fuzhou Headquarters

Addr:No.21 Building, A District, Fuzhou

Software Park, 89 Soft Avenue, Tongpan Road,

Gulou District, Fuzhou (350003)

地址:福建省福州市铜盘路软件大道89号软件园A区21号楼 (350003)

Tel:(86-0591)83991906 - 8296

E-mail:mark.yao@rock-chips.com



  reply	other threads:[~2015-02-04  3:56 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-31  8:41 [PATCH] drm/rockchip: vop: power off until vop standby take effect Mark Yao
2015-01-31  8:41 ` Mark Yao
2015-01-31 12:49 ` Heiko Stübner
2015-01-31 12:49   ` Heiko Stübner
2015-02-02  0:45   ` Mark yao
2015-02-02  0:45     ` Mark yao
2015-02-02  2:07 ` Daniel Kurtz
2015-02-02  2:07   ` Daniel Kurtz
2015-02-02  2:30   ` Mark yao
2015-02-02  2:30     ` Mark yao
2015-02-02  7:53     ` Daniel Vetter
2015-02-02  7:53       ` Daniel Vetter
2015-02-04  3:38       ` Mark yao
2015-02-04  3:38         ` Mark yao
2015-02-04  3:48         ` Daniel Kurtz
2015-02-04  3:48           ` Daniel Kurtz
2015-02-04  3:56           ` Mark yao [this message]
2015-02-04  3:56             ` Mark yao

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=54D19868.8000906@rock-chips.com \
    --to=mark.yao@rock-chips.com \
    --cc=djkurtz@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    /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.