[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <df059f88-6ff6-5906-58f1-4f6c752c4214@quicinc.com>
Date: Mon, 13 Feb 2023 19:09:32 -0800
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Jessica Zhang <quic_jesszhan@...cinc.com>,
Marijn Suijten <marijn.suijten@...ainline.org>,
<phone-devel@...r.kernel.org>,
Neil Armstrong <neil.armstrong@...aro.org>
CC: <~postmarketos/upstreaming@...ts.sr.ht>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...ainline.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Martin Botka <martin.botka@...ainline.org>,
"Jami Kettunen" <jami.kettunen@...ainline.org>,
Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Stephen Boyd <swboyd@...omium.org>,
Vinod Koul <vkoul@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Kuogee Hsieh <quic_khsieh@...cinc.com>,
Konrad Dybcio <konrad.dybcio@...ainline.org>,
"Loic Poulain" <loic.poulain@...aro.org>,
Vinod Polimera <quic_vpolimer@...cinc.com>,
Adam Skladowski <a39.skl@...il.com>,
<linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
<freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 5/7] drm/msm/dpu: Document and enable TEAR interrupts
on DSI interfaces
On 2/13/2023 1:46 PM, Dmitry Baryshkov wrote:
> On 13/02/2023 21:37, Jessica Zhang wrote:
>>
>>
>> On 12/31/2022 1:50 PM, Marijn Suijten wrote:
>>> All SoCs since DPU 5.0.0 (and seemingly up until and including 6.0.0,
>>> but excluding 7.x.x) have the tear interrupt and control registers moved
>>> out of the PINGPONG block and into the INTF block. Wire up the
>>> necessary interrupts and IRQ masks on all supported hardware.
>>
>> Hi Marijn,
>>
>> Thanks for the patch.
>>
>> I saw that in your commit msg, you mentioned that 7.x doesn't have
>> tearcheck in the INTF block -- can you double check that this is correct?
>>
>> I'm working on SM8350 (DPU v7) and I'm seeing that it does have
>> tearcheck in INTF block.
>
> I confirm, according to the vendor drivers INTF TE should be used for
> all DPU >= 5.0, including 7.x and 8.x
>
> However I think I know what Marijn meant here. For 5.x and 6.x these
> IRQs are handled at the address MDSS + 0x6e800 / + 0x6e900 (which means
> offset here should 0x6d800 and 0x6d900) for INTF_1 and INTF_2. Since DPU
> 7.x these IRQ registers were moved close to the main INTF block (0x36800
> and 0x37800 = INTF + 0x800).
>
Got it, then the commit text should remove "control" and just say tear
interrupt registers. It got a bit confusing.
We will add the 7xxx intf tear check support on top of this series.
>>
>>>
>>> Signed-off-by: Marijn Suijten <marijn.suijten@...ainline.org>
>>> ---
>>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 78 +++++++++++--------
>>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +-
>>> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 +++
>>> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 +
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 3 +
>>> 5 files changed, 68 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> index 1cfe94494135..b9b9b5b0b615 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> @@ -86,6 +86,15 @@
>>> #define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN)
>>> +#define IRQ_MSM8998_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
>>> + BIT(MDP_SSPP_TOP0_INTR2) | \
>>> + BIT(MDP_SSPP_TOP0_HIST_INTR) | \
>>> + BIT(MDP_INTF0_INTR) | \
>>> + BIT(MDP_INTF1_INTR) | \
>>> + BIT(MDP_INTF2_INTR) | \
>>> + BIT(MDP_INTF3_INTR) | \
>>> + BIT(MDP_INTF4_INTR))
>>> +
>>> #define IRQ_SDM845_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
>>> BIT(MDP_SSPP_TOP0_INTR2) | \
>>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \
>>> @@ -100,13 +109,15 @@
>>> #define IRQ_QCM2290_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
>>> BIT(MDP_SSPP_TOP0_INTR2) | \
>>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \
>>> - BIT(MDP_INTF1_INTR))
>>> + BIT(MDP_INTF1_INTR) | \
>>> + BIT(MDP_INTF1_TEAR_INTR))
>>> #define IRQ_SC7180_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
>>> BIT(MDP_SSPP_TOP0_INTR2) | \
>>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \
>>> BIT(MDP_INTF0_INTR) | \
>>> - BIT(MDP_INTF1_INTR))
>>> + BIT(MDP_INTF1_INTR) | \
>>> + BIT(MDP_INTF1_TEAR_INTR))
>>> #define IRQ_SC7280_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
>>> BIT(MDP_SSPP_TOP0_INTR2) | \
>>> @@ -120,7 +131,9 @@
>>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \
>>> BIT(MDP_INTF0_INTR) | \
>>> BIT(MDP_INTF1_INTR) | \
>>> + BIT(MDP_INTF1_TEAR_INTR) | \
>>> BIT(MDP_INTF2_INTR) | \
>>> + BIT(MDP_INTF2_TEAR_INTR) | \
>>> BIT(MDP_INTF3_INTR) | \
>>> BIT(MDP_INTF4_INTR))
>>> @@ -129,7 +142,9 @@
>>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \
>>> BIT(MDP_INTF0_INTR) | \
>>> BIT(MDP_INTF1_INTR) | \
>>> + BIT(MDP_INTF1_TEAR_INTR) | \
>>> BIT(MDP_INTF2_INTR) | \
>>> + BIT(MDP_INTF2_TEAR_INTR) | \
>>> BIT(MDP_INTF3_INTR) | \
>>> BIT(MDP_INTF4_INTR) | \
>>> BIT(MDP_INTF5_INTR) | \
>>> @@ -1300,63 +1315,64 @@ static struct dpu_dsc_cfg sdm845_dsc[] = {
>>> /*************************************************************
>>> * INTF sub blocks config
>>> *************************************************************/
>>> -#define INTF_BLK(_name, _id, _base, _type, _ctrl_id, _progfetch,
>>> _features, _reg, _underrun_bit, _vsync_bit) \
>>> +#define INTF_BLK(_name, _id, _base, _len, _type, _ctrl_id,
>>> _progfetch, _features, _reg, _underrun_bit, _vsync_bit, _tear_reg,
>>> _tear_rd_ptr_bit) \
>>> {\
>>> .name = _name, .id = _id, \
>>> - .base = _base, .len = 0x280, \
>>> + .base = _base, .len = _len, \
>>> .features = _features, \
>>> .type = _type, \
>>> .controller_id = _ctrl_id, \
>>> .prog_fetch_lines_worst_case = _progfetch, \
>>> .intr_underrun = DPU_IRQ_IDX(_reg, _underrun_bit), \
>>> .intr_vsync = DPU_IRQ_IDX(_reg, _vsync_bit), \
>>> + .intr_tear_rd_ptr = DPU_IRQ_IDX(_tear_reg, _tear_rd_ptr_bit), \
>>> }
>>> static const struct dpu_intf_cfg msm8998_intf[] = {
>>> - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 25,
>>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
>>> - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 25,
>>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
>>> - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 25,
>>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29),
>>> - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_HDMI, 0, 25,
>>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31),
>>> + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x268, INTF_DP, 0, 25,
>>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1),
>>
>> Just wondering, how were the lengths calculated for the INTF blocks?
>> The values in general seem a little off to me.
>>
>> For example, I'm looking downstream and it seems to me that the length
>> for the INTF_0 on MSM8998 should be 0x280. Similarly for SC7280, I'm
>> seeing that length for INTF + tearcheck should be 0x2c4.
>
> We have discussed INTF lengths in [1]. The current understanding of the
> block lengths can be found at [2]. Please comment there if any of the
> fixed lengths sounds incorrect to you.
>
> [1] https://patchwork.freedesktop.org/patch/522187/
> [2] https://patchwork.freedesktop.org/patch/522227/
>
> [skipped the rest]
>
Please correct my understanding here, it was agreed to fix intf blocks
to 0x2c4 here https://patchwork.freedesktop.org/patch/522227/ but I dont
see this was merged?
It was agreed to first land INTF_TE and then add the higher addresses
but I dont see such a change, am i missing something?
Powered by blists - more mailing lists