All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Sricharan" <sricharan@codeaurora.org>
To: 'Stephen Boyd' <sboyd@codeaurora.org>
Cc: 'Rajendra Nayak' <rnayak@codeaurora.org>,
	mturquette@baylibre.com, linux-clk@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	stanimir.varbanov@linaro.org
Subject: RE: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus core0/1 clks
Date: Mon, 14 Nov 2016 09:21:24 +0530	[thread overview]
Message-ID: <01be01d23e2a$628f4230$27adc690$@codeaurora.org> (raw)
In-Reply-To: <20161110233001.GM16026@codeaurora.org>

Hi Stephen,

>>
>> So the above is the sequence which is actually carried out on the
>> firmware side. The same can be done in host as well.
>> The clocks stuck issue indeed is not there with this.
>
>Great! We've finally connected on what the actual problem is.
>
>> But with the above sequence we need to add a step to do inverse
>> of STEP3 above (ie write the registers to de-assert hw_signal),
>> to keep the subdomains in off, till firmware uses it. So the
>> above sequence helps to avoid masking the halt check, although
>> the host really does not wants to use these clocks, except
>> setting it up for the firmware.
>>
>
>Right, but knowing that the clocks failed to turn on in the first
>place is much safer than silently ignoring the failure.
>Otherwise, we could hand over control to the firmware, and the
>firmware would fail to operate the hardware, and we're stuck with
>debugging the firmware now. That sounds quite painful to figure
>out.
>

Right, i already debugged this sort of a scenario which was quite
paintful sometime back :)

>If we properly toggle the video hw bits in coordination with
>firmware and turn on/off the clocks with the GDSC ON, then
>debugging is made simpler. The point is, we don't want to lose
>robustness by silencing halt checks. The semantics of
>clk_enable() means the clock is running, and that won't be true
>here unless we ensure the GDSC is enabled.
>

ok, which means with this approach, this patch can be dropped and
the other bits needs to be added to the video driver. I will follow that
up with Stanimir in his video driver patches.

Regards,
 Sricharan

WARNING: multiple messages have this Message-ID (diff)
From: "Sricharan" <sricharan@codeaurora.org>
To: "'Stephen Boyd'" <sboyd@codeaurora.org>
Cc: "'Rajendra Nayak'" <rnayak@codeaurora.org>,
	<mturquette@baylibre.com>, <linux-clk@vger.kernel.org>,
	<linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<stanimir.varbanov@linaro.org>
Subject: RE: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus core0/1 clks
Date: Mon, 14 Nov 2016 09:21:24 +0530	[thread overview]
Message-ID: <01be01d23e2a$628f4230$27adc690$@codeaurora.org> (raw)
In-Reply-To: <20161110233001.GM16026@codeaurora.org>

Hi Stephen,

>>
>> So the above is the sequence which is actually carried out on the
>> firmware side. The same can be done in host as well.
>> The clocks stuck issue indeed is not there with this.
>
>Great! We've finally connected on what the actual problem is.
>
>> But with the above sequence we need to add a step to do inverse
>> of STEP3 above (ie write the registers to de-assert hw_signal),
>> to keep the subdomains in off, till firmware uses it. So the
>> above sequence helps to avoid masking the halt check, although
>> the host really does not wants to use these clocks, except
>> setting it up for the firmware.
>>
>
>Right, but knowing that the clocks failed to turn on in the first
>place is much safer than silently ignoring the failure.
>Otherwise, we could hand over control to the firmware, and the
>firmware would fail to operate the hardware, and we're stuck with
>debugging the firmware now. That sounds quite painful to figure
>out.
>

Right, i already debugged this sort of a scenario which was quite
paintful sometime back :)

>If we properly toggle the video hw bits in coordination with
>firmware and turn on/off the clocks with the GDSC ON, then
>debugging is made simpler. The point is, we don't want to lose
>robustness by silencing halt checks. The semantics of
>clk_enable() means the clock is running, and that won't be true
>here unless we ensure the GDSC is enabled.
>

ok, which means with this approach, this patch can be dropped and
the other bits needs to be added to the video driver. I will follow that
up with Stanimir in his video driver patches.

Regards,
 Sricharan

  reply	other threads:[~2016-11-14  3:51 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-24 10:18 [PATCH 0/3] clk: qcom: Add support for hw controlled gdscs/clocks Sricharan R
2016-10-24 10:18 ` [PATCH 1/3] clk: qcom: gdsc: Add support for gdscs with HW control Sricharan R
2016-10-25 13:01   ` Stanimir Varbanov
2016-10-26  4:12     ` Sricharan
2016-10-26  4:12       ` Sricharan
2016-11-02  0:18   ` Stephen Boyd
2016-11-02  6:50     ` Sricharan
2016-11-02  6:50       ` Sricharan
2016-11-02  6:53       ` Sricharan
2016-11-02  6:53         ` Sricharan
2016-11-02 17:59       ` 'Stephen Boyd'
2016-11-03 13:30         ` Sricharan
2016-11-03 13:30           ` Sricharan
2016-11-03 20:05           ` 'Stephen Boyd'
2016-10-24 10:18 ` [PATCH 2/3] clk: qcom: Put venus core0/1 gdscs to hw control mode Sricharan R
2016-10-24 10:18 ` [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus core0/1 clks Sricharan R
2016-11-03 20:34   ` Stephen Boyd
2016-11-04  9:09     ` Sricharan
2016-11-04  9:09       ` Sricharan
2016-11-04 20:18       ` 'Stephen Boyd'
2016-11-07  5:48         ` Rajendra Nayak
2016-11-08 22:33           ` 'Stephen Boyd'
2016-11-09 16:56             ` Sricharan
2016-11-09 16:56               ` Sricharan
2016-11-10  2:32               ` Rajendra Nayak
2016-11-10  3:28                 ` Sricharan
2016-11-10  3:28                   ` Sricharan
2016-11-10 23:30               ` 'Stephen Boyd'
2016-11-14  3:51                 ` Sricharan [this message]
2016-11-14  3:51                   ` Sricharan
2016-12-12 15:40             ` Stanimir Varbanov

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='01be01d23e2a$628f4230$27adc690$@codeaurora.org' \
    --to=sricharan@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=rnayak@codeaurora.org \
    --cc=sboyd@codeaurora.org \
    --cc=stanimir.varbanov@linaro.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.