Phone-Devel Archive on lore.kernel.org.
 help / color / Atom feed
From: AngeloGioacchino Del Regno  <angelogioacchino.delregno@somainline.org>
To: abhinavk@codeaurora.org, Marijn Suijten <marijn.suijten@somainline.org>
Cc: phone-devel@vger.kernel.org, freedreno@lists.freedesktop.org,
	Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>,
	David Airlie <airlied@linux.ie>,
	linux-arm-msm@vger.kernel.org,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Rob Clark <robdclark@gmail.com>,
	Martin Botka <martin.botka@somainline.org>,
	~postmarketos/upstreaming@lists.sr.ht,
	Daniel Vetter <daniel@ffwll.ch>, Sean Paul <sean@poorly.run>
Subject: Re: [Freedreno] [PATCH 1/3] drm/msm/mdp5: Configure PP_SYNC_HEIGHT to double the vtotal
Date: Wed, 7 Apr 2021 21:11:53 +0200
Message-ID: <04860f05-f79f-de0b-13d1-aba85065b4da@somainline.org> (raw)
In-Reply-To: <6413863d04df9743e2d7e81beff5c3e8@codeaurora.org>

Il 07/04/21 20:19, abhinavk@codeaurora.org ha scritto:
> Hi Marijn
> 
> On 2021-04-06 14:47, Marijn Suijten wrote:
>> Leaving this at a close-to-maximum register value 0xFFF0 means it takes
>> very long for the MDSS to generate a software vsync interrupt when the
>> hardware TE interrupt doesn't arrive.  Configuring this to double the
>> vtotal (like some downstream kernels) leads to a frame to take at most
>> twice before the vsync signal, until hardware TE comes up.
>>
>> In this case the hardware interrupt responsible for providing this
>> signal - "disp-te" gpio - is not hooked up to the mdp5 vsync/pp logic at
>> all.  This solves severe panel update issues observed on at least the
>> Xperia Loire and Tone series, until said gpio is properly hooked up to
>> an irq.
> 
> The reason the CONFIG_HEIGHT was at such a high value is to make sure that
> we always get the TE only from the panel vsync and not false positives 
> coming
> from the tear check logic itself.
> 
> When you say that disp-te gpio is not hooked up, is it something 
> incorrect with
> the schematic OR panel is not generating the TE correctly?
> 

Sometimes, some panels aren't getting correctly configured by the 
OEM/ODM in the first place: especially when porting devices from 
downstream to upstream, developers often get in a situation in which 
their TE line is either misconfigured or the DriverIC is not configured 
to raise V-Sync interrupts.
Please remember: some DDICs need a "commands sequence" to enable 
generating the TE interrupts, sometimes this is not standard, and 
sometimes OEMs/ODMs are not even doing that in their downstream code 
(but instead they work around it in creative ways "for reasons", even 
though their DDIC supports indeed sending TE events).

This mostly happens when bringing up devices that have autorefresh 
enabled from the bootloader (when the bootloader sets up the splash 
screen) by using simple-panel as a (hopefully) temporary solution to get 
through the initial stages of porting.

We are not trying to cover cases related to incorrect schematics or 
hardware mistakes here, as the fix for that - as you know - is to just 
fix your hardware.
What we're trying to do here is to stop freezes and, in some cases, 
lockups, other than false positives making the developer go offroad when 
the platform shows that something is wrong during early porting.

Also, sometimes, some DDICs will not generate TE interrupts when 
expected... in these cases we get a PP timeout and a MDP5 recovery: this 
is totally avoidable if we rely on the 2*vtotal, as we wouldn't get 
through the very time consuming task of recovering the entire MDP.

Of course, if something is wrong in the MDP and the block really needs 
recovery, this "trick" won't save anyone and the recovery will anyway be 
triggered, as the PP-done will anyway timeout.

>>
>> Suggested-by: AngeloGioacchino Del Regno
>> <angelogioacchino.delregno@somainline.org>
>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>> Reviewed-by: AngeloGioacchino Del Regno
>> <angelogioacchino.delregno@somainline.org>
>> ---
>>  drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
>> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
>> index ff2c1d583c79..2d5ac03dbc17 100644
>> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
>> @@ -51,7 +51,7 @@ static int pingpong_tearcheck_setup(struct
>> drm_encoder *encoder,
>>
>>      mdp5_write(mdp5_kms, REG_MDP5_PP_SYNC_CONFIG_VSYNC(pp_id), cfg);
>>      mdp5_write(mdp5_kms,
>> -        REG_MDP5_PP_SYNC_CONFIG_HEIGHT(pp_id), 0xfff0);
>> +        REG_MDP5_PP_SYNC_CONFIG_HEIGHT(pp_id), (2 * mode->vtotal));
>>      mdp5_write(mdp5_kms,
>>          REG_MDP5_PP_VSYNC_INIT_VAL(pp_id), mode->vdisplay);
>>      mdp5_write(mdp5_kms, REG_MDP5_PP_RD_PTR_IRQ(pp_id), 
>> mode->vdisplay + 1);


  reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-06 21:47 [PATCH 0/3] drm/msm/mdp5: Emit vsync signal often enough Marijn Suijten
2021-04-06 21:47 ` [PATCH 1/3] drm/msm/mdp5: Configure PP_SYNC_HEIGHT to double the vtotal Marijn Suijten
2021-04-07 18:19   ` [Freedreno] " abhinavk
2021-04-07 19:11     ` AngeloGioacchino Del Regno [this message]
2021-04-08 19:05       ` Rob Clark
     [not found]         ` <CAK7fi1aUXy2i8zY0Cb5Svq0s1H9cSAvY4hq+BsiWgdphwm-ebA@mail.gmail.com>
2021-04-09  0:08           ` Rob Clark
2021-04-09  8:22             ` Marijn Suijten
2021-04-06 21:47 ` [PATCH 2/3] drm/msm/mdp5: Do not multiply vclk line count by 100 Marijn Suijten
2021-04-06 21:47 ` [PATCH 3/3] drm/msm/mdp5: Disable pingpong autorefresh at tearcheck init Marijn Suijten

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=04860f05-f79f-de0b-13d1-aba85065b4da@somainline.org \
    --to=angelogioacchino.delregno@somainline.org \
    --cc=abhinavk@codeaurora.org \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=konrad.dybcio@somainline.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=martin.botka@somainline.org \
    --cc=phone-devel@vger.kernel.org \
    --cc=robdclark@gmail.com \
    --cc=saiprakash.ranjan@codeaurora.org \
    --cc=sean@poorly.run \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /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

Phone-Devel Archive on lore.kernel.org.

Archives are clonable:
	git clone --mirror https://lore.kernel.org/phone-devel/0 phone-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 phone-devel phone-devel/ https://lore.kernel.org/phone-devel \
		phone-devel@vger.kernel.org
	public-inbox-index phone-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.phone-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git