Message ID | 20231130-encoder-fixup-v1-2-585c54cd046e@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp810371vqy; Thu, 30 Nov 2023 17:32:04 -0800 (PST) X-Google-Smtp-Source: AGHT+IFmkj5rjdRMwgzRPdF0w20t3QEDG2TFwaSuDPv4OengcbHLArTzsCvED5QbNT0T684PJ8FM X-Received: by 2002:a17:903:2596:b0:1cf:9d88:a0ef with SMTP id jb22-20020a170903259600b001cf9d88a0efmr20105921plb.59.1701394324117; Thu, 30 Nov 2023 17:32:04 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701394324; cv=none; d=google.com; s=arc-20160816; b=07idQ63VY/5yTOQaTefEevdA5bFoux227RgsKtc9Q50kIhr+nO3AzUrxpKKqW0HaSj sKJ7q5f/qAtiihwq8yG4mwRQWkjAniH4Qj21ZbqecOnENTob3szxKKEM6Ytv6rMPic09 10wEGMs+JnJJL70E12RrcgxkjpTjWdXQSqw+TBvUYF6HehdGHpzSVYE5hEb7A2MmBnkH Bbz8tL0oqxbDC91zAlPmMyTZpzEkM1jg05j0HzXWormAU4p0pdoKevKGgcerBFk3QYxi Ys/mO/vYE5IiqEoHauGOVa6nt45vptAcnPV0pk+kjHPbReHDIq0+cauu7Txk8RSvGR3A HcEA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:in-reply-to:references:message-id :content-transfer-encoding:mime-version:subject:date:from :dkim-signature; bh=/NU2dbFQNyWKS8CYNdbXTOILj/VJ3QBEhaVjuOI+lD8=; fh=4S0D3yLzwdUhbjz05zrMQ6SXS3TL5RZU1HuJ+K/LHRo=; b=zEaVfASOlK71BGLkHtJJax11vWrKoYFbmJKdAYRzMeiPP14Q9JHcoQFdIAPz4gxDpO Jl9SXEPrRdT+KRh9wO7MMNtk+ClMjH6uS+gK2bTkyVOSsb3oTpAoKhGw2Fuy7oaK3Gsk PNMYAi082lYZaH0xDtIxzvH8Zp758JWjSnNvErL6JEIrowx76QEBnnT2U8snge8aMegt o5c0ai4FxsiNi/EzThmVpYKoZewTarOGXDokA7tyGTsPw8+YT/Jj3diAm+Oe6IJWdbPd gsW/M6WO+iqjzy72pZTxHe10x5ihhlcWo586e1G54wv3E46mhgK4fPdBhE4ajgh43Zi5 jE8A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=VrQ70tFE; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id u7-20020a17090341c700b001d018b5a0dcsi2490733ple.411.2023.11.30.17.32.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Nov 2023 17:32:04 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=VrQ70tFE; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 30E4B80A8B66; Thu, 30 Nov 2023 17:31:15 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230007AbjLABbD (ORCPT <rfc822;ruipengqi7@gmail.com> + 99 others); Thu, 30 Nov 2023 20:31:03 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54728 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229521AbjLABbA (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 30 Nov 2023 20:31:00 -0500 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EB4B810F3; Thu, 30 Nov 2023 17:31:06 -0800 (PST) Received: from pps.filterd (m0279872.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3B10L6iB000664; Fri, 1 Dec 2023 01:31:01 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : date : subject : mime-version : content-type : content-transfer-encoding : message-id : references : in-reply-to : to : cc; s=qcppdkim1; bh=/NU2dbFQNyWKS8CYNdbXTOILj/VJ3QBEhaVjuOI+lD8=; b=VrQ70tFEB1EPxbl9tUEYn9Dybw6vjaIs9O2e98tLwVEeow/MOHbKIcnbj0BUofsgu6Ft xJMCgTQ2r28Px46Xp2wEBlUTqIgyngneuYV/gVcF/fj7v72aRmFILq7lqTLTqiw8YBdE 1e8y8JRdRfW5NksW9o5omGuu/asKqQYHSy9TiYfPeHWVbs8KAhhoOvnZqaK5AhErUHbP N2tePEC6HBfdGSnvdLQZQnw+Y7TzthPohCTmiMjxgznA6nlFrYDcxH64VnN5+DNXmFzA 0NynNBwjKzG4fnTO7W/RNwtOkANlKD0aSlmAt7v90K0nZIJDFwJsMSFXbf5MyNTnoC0p MQ== Received: from nasanppmta01.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3upgfd3c8u-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 01 Dec 2023 01:31:01 +0000 Received: from nasanex01b.na.qualcomm.com (nasanex01b.na.qualcomm.com [10.46.141.250]) by NASANPPMTA01.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 3B11V0JD024463 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 1 Dec 2023 01:31:00 GMT Received: from jesszhan-linux.qualcomm.com (10.80.80.8) by nasanex01b.na.qualcomm.com (10.46.141.250) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.40; Thu, 30 Nov 2023 17:31:00 -0800 From: Jessica Zhang <quic_jesszhan@quicinc.com> Date: Thu, 30 Nov 2023 17:30:34 -0800 Subject: [PATCH 2/2] drm/msm/dpu: Set input_sel bit for INTF MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-ID: <20231130-encoder-fixup-v1-2-585c54cd046e@quicinc.com> References: <20231130-encoder-fixup-v1-0-585c54cd046e@quicinc.com> In-Reply-To: <20231130-encoder-fixup-v1-0-585c54cd046e@quicinc.com> To: Rob Clark <robdclark@gmail.com>, Dmitry Baryshkov <dmitry.baryshkov@linaro.org>, Sean Paul <sean@poorly.run>, Marijn Suijten <marijn.suijten@somainline.org>, David Airlie <airlied@gmail.com>, "Daniel Vetter" <daniel@ffwll.ch> CC: <quic_abhinavk@quicinc.com>, <linux-arm-msm@vger.kernel.org>, <dri-devel@lists.freedesktop.org>, <freedreno@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>, Jessica Zhang <quic_jesszhan@quicinc.com> X-Mailer: b4 0.13-dev-53db1 X-Developer-Signature: v=1; a=ed25519-sha256; t=1701394259; l=3663; i=quic_jesszhan@quicinc.com; s=20230329; h=from:subject:message-id; bh=f4MgpUR7qdkeHqkjRkQnRto4i/L5a9mnRaHCbV37d7s=; b=YaRowAfYu9UtWmL0tdfOtCJLCUYSpZfgImYT6w+enn5a+q8W013WGwVAs8OLuaN1d6ykP84Gn Rn0uCPBwnL8AjxV6SfA9Kw1+xPKpSKcYTJvqW8BYXu4najOkg+BzhS4 X-Developer-Key: i=quic_jesszhan@quicinc.com; a=ed25519; pk=gAUCgHZ6wTJOzQa3U0GfeCDH7iZLlqIEPo4rrjfDpWE= X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nasanex01b.na.qualcomm.com (10.46.141.250) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-ORIG-GUID: SclDWkzTrjjqxZt7KBFQUeHklgcM3nok X-Proofpoint-GUID: SclDWkzTrjjqxZt7KBFQUeHklgcM3nok X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-11-30_25,2023-11-30_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 lowpriorityscore=0 spamscore=0 malwarescore=0 impostorscore=0 priorityscore=1501 bulkscore=0 clxscore=1015 mlxscore=0 adultscore=0 phishscore=0 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311060000 definitions=main-2312010008 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 30 Nov 2023 17:31:15 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784041255139646996 X-GMAIL-MSGID: 1784041255139646996 |
Series |
drm/msm/dpu: INTF CRC configuration cleanups and fix
|
|
Commit Message
Jessica Zhang
Dec. 1, 2023, 1:30 a.m. UTC
Set the input_sel bit for encoders as it was missed in the initial
implementation.
Reported-by: Rob Clark <robdclark@gmail.com>
Fixes: 91143873a05d ("drm/msm/dpu: Add MISR register support for interface")
Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/39
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 7 ++++++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 +++-
4 files changed, 11 insertions(+), 4 deletions(-)
Comments
On Fri, 1 Dec 2023 at 03:31, Jessica Zhang <quic_jesszhan@quicinc.com> wrote: > > Set the input_sel bit for encoders as it was missed in the initial > implementation. > > Reported-by: Rob Clark <robdclark@gmail.com> > Fixes: 91143873a05d ("drm/msm/dpu: Add MISR register support for interface") > Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/39 > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 2 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 7 ++++++- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 +++- > 4 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > index 3442cf65b86f..d0884997ecb7 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > @@ -320,7 +320,7 @@ static u32 dpu_hw_intf_get_line_count(struct dpu_hw_intf *intf) > > static void dpu_hw_intf_setup_misr(struct dpu_hw_intf *intf) > { > - dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL); > + dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL, true); > } > > static int dpu_hw_intf_collect_misr(struct dpu_hw_intf *intf, u32 *misr_value) > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c > index f38473e68f79..77b14107c84a 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c > @@ -83,7 +83,7 @@ static void dpu_hw_lm_setup_border_color(struct dpu_hw_mixer *ctx, > > static void dpu_hw_lm_setup_misr(struct dpu_hw_mixer *ctx) > { > - dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL); > + dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL, false); > } > > static int dpu_hw_lm_collect_misr(struct dpu_hw_mixer *ctx, u32 *misr_value) > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c > index a8a0a4e76b94..f441df47fdde 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c > @@ -481,7 +481,8 @@ void _dpu_hw_setup_qos_lut(struct dpu_hw_blk_reg_map *c, u32 offset, > cfg->danger_safe_en ? QOS_QOS_CTRL_DANGER_SAFE_EN : 0); > } > > -void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset) > +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset, > + bool set_input_sel) > { > u32 config = 0; > > @@ -491,6 +492,10 @@ void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset) > wmb(); > > config = MISR_FRAME_COUNT | MISR_CTRL_ENABLE | MISR_CTRL_FREE_RUN_MASK; > + > + if (set_input_sel) > + config |= MISR_CTRL_INPUT_SEL; > + > DPU_REG_WRITE(c, misr_ctrl_offset, config); > } > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h > index bb496ebe283b..793670d62414 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h > @@ -17,6 +17,7 @@ > #define MISR_CTRL_ENABLE BIT(8) > #define MISR_CTRL_STATUS BIT(9) > #define MISR_CTRL_STATUS_CLEAR BIT(10) > +#define MISR_CTRL_INPUT_SEL BIT(24) The public apq8916 TRM documents this as a 4-bit field. I think this was followed into the later generations. Can we please document it correctly and use an uint instead of just bool for set_input_sel? > #define MISR_CTRL_FREE_RUN_MASK BIT(31) > > /* > @@ -357,7 +358,8 @@ void _dpu_hw_setup_qos_lut(struct dpu_hw_blk_reg_map *c, u32 offset, > bool qos_8lvl, > const struct dpu_hw_qos_cfg *cfg); > > -void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset); > +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset, > + bool set_input_sel); > > int dpu_hw_collect_misr(struct dpu_hw_blk_reg_map *c, > u32 misr_ctrl_offset, > > -- > 2.43.0 >
On 11/30/2023 11:36 PM, Dmitry Baryshkov wrote: > On Fri, 1 Dec 2023 at 03:31, Jessica Zhang <quic_jesszhan@quicinc.com> wrote: >> >> Set the input_sel bit for encoders as it was missed in the initial >> implementation. >> >> Reported-by: Rob Clark <robdclark@gmail.com> >> Fixes: 91143873a05d ("drm/msm/dpu: Add MISR register support for interface") >> Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/39 >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 2 +- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 7 ++++++- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 +++- >> 4 files changed, 11 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >> index 3442cf65b86f..d0884997ecb7 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >> @@ -320,7 +320,7 @@ static u32 dpu_hw_intf_get_line_count(struct dpu_hw_intf *intf) >> >> static void dpu_hw_intf_setup_misr(struct dpu_hw_intf *intf) >> { >> - dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL); >> + dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL, true); >> } >> >> static int dpu_hw_intf_collect_misr(struct dpu_hw_intf *intf, u32 *misr_value) >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c >> index f38473e68f79..77b14107c84a 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c >> @@ -83,7 +83,7 @@ static void dpu_hw_lm_setup_border_color(struct dpu_hw_mixer *ctx, >> >> static void dpu_hw_lm_setup_misr(struct dpu_hw_mixer *ctx) >> { >> - dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL); >> + dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL, false); >> } >> >> static int dpu_hw_lm_collect_misr(struct dpu_hw_mixer *ctx, u32 *misr_value) >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c >> index a8a0a4e76b94..f441df47fdde 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c >> @@ -481,7 +481,8 @@ void _dpu_hw_setup_qos_lut(struct dpu_hw_blk_reg_map *c, u32 offset, >> cfg->danger_safe_en ? QOS_QOS_CTRL_DANGER_SAFE_EN : 0); >> } >> >> -void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset) >> +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset, >> + bool set_input_sel) >> { >> u32 config = 0; >> >> @@ -491,6 +492,10 @@ void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset) >> wmb(); >> >> config = MISR_FRAME_COUNT | MISR_CTRL_ENABLE | MISR_CTRL_FREE_RUN_MASK; >> + >> + if (set_input_sel) >> + config |= MISR_CTRL_INPUT_SEL; >> + >> DPU_REG_WRITE(c, misr_ctrl_offset, config); >> } >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h >> index bb496ebe283b..793670d62414 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h >> @@ -17,6 +17,7 @@ >> #define MISR_CTRL_ENABLE BIT(8) >> #define MISR_CTRL_STATUS BIT(9) >> #define MISR_CTRL_STATUS_CLEAR BIT(10) >> +#define MISR_CTRL_INPUT_SEL BIT(24) > > The public apq8916 TRM documents this as a 4-bit field. I think this > was followed into the later generations. Can we please document it > correctly and use an uint instead of just bool for set_input_sel? > Can you pls point us to this document you are referring? I was not aware that bit level details are revealed in external documents :) Even though its a 4-bit field, it only takes a 0 or 1 as others are undefined. Exposing all the bits will only cause more confusion like it did for others thinking that input select is actually configurable when its not. I think what we should do is just pass "misr_type" to this API to tell whether its lm misr or intf misr and set BIT(24) based on that. >> #define MISR_CTRL_FREE_RUN_MASK BIT(31) >> >> /* >> @@ -357,7 +358,8 @@ void _dpu_hw_setup_qos_lut(struct dpu_hw_blk_reg_map *c, u32 offset, >> bool qos_8lvl, >> const struct dpu_hw_qos_cfg *cfg); >> >> -void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset); >> +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset, >> + bool set_input_sel); >> >> int dpu_hw_collect_misr(struct dpu_hw_blk_reg_map *c, >> u32 misr_ctrl_offset, >> >> -- >> 2.43.0 >> > >
On 01/12/2023 23:29, Abhinav Kumar wrote: > > > On 11/30/2023 11:36 PM, Dmitry Baryshkov wrote: >> On Fri, 1 Dec 2023 at 03:31, Jessica Zhang <quic_jesszhan@quicinc.com> >> wrote: >>> >>> Set the input_sel bit for encoders as it was missed in the initial >>> implementation. >>> >>> Reported-by: Rob Clark <robdclark@gmail.com> >>> Fixes: 91143873a05d ("drm/msm/dpu: Add MISR register support for >>> interface") >>> Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/39 >>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >>> --- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 2 +- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 7 ++++++- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 +++- >>> 4 files changed, 11 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>> index 3442cf65b86f..d0884997ecb7 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>> @@ -320,7 +320,7 @@ static u32 dpu_hw_intf_get_line_count(struct >>> dpu_hw_intf *intf) >>> >>> static void dpu_hw_intf_setup_misr(struct dpu_hw_intf *intf) >>> { >>> - dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL); >>> + dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL, true); >>> } >>> >>> static int dpu_hw_intf_collect_misr(struct dpu_hw_intf *intf, u32 >>> *misr_value) >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c >>> index f38473e68f79..77b14107c84a 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c >>> @@ -83,7 +83,7 @@ static void dpu_hw_lm_setup_border_color(struct >>> dpu_hw_mixer *ctx, >>> >>> static void dpu_hw_lm_setup_misr(struct dpu_hw_mixer *ctx) >>> { >>> - dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL); >>> + dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL, false); >>> } >>> >>> static int dpu_hw_lm_collect_misr(struct dpu_hw_mixer *ctx, u32 >>> *misr_value) >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c >>> index a8a0a4e76b94..f441df47fdde 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c >>> @@ -481,7 +481,8 @@ void _dpu_hw_setup_qos_lut(struct >>> dpu_hw_blk_reg_map *c, u32 offset, >>> cfg->danger_safe_en ? >>> QOS_QOS_CTRL_DANGER_SAFE_EN : 0); >>> } >>> >>> -void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 >>> misr_ctrl_offset) >>> +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 >>> misr_ctrl_offset, >>> + bool set_input_sel) >>> { >>> u32 config = 0; >>> >>> @@ -491,6 +492,10 @@ void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map >>> *c, u32 misr_ctrl_offset) >>> wmb(); >>> >>> config = MISR_FRAME_COUNT | MISR_CTRL_ENABLE | >>> MISR_CTRL_FREE_RUN_MASK; >>> + >>> + if (set_input_sel) >>> + config |= MISR_CTRL_INPUT_SEL; >>> + >>> DPU_REG_WRITE(c, misr_ctrl_offset, config); >>> } >>> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h >>> index bb496ebe283b..793670d62414 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h >>> @@ -17,6 +17,7 @@ >>> #define MISR_CTRL_ENABLE BIT(8) >>> #define MISR_CTRL_STATUS BIT(9) >>> #define MISR_CTRL_STATUS_CLEAR BIT(10) >>> +#define MISR_CTRL_INPUT_SEL BIT(24) >> >> The public apq8916 TRM documents this as a 4-bit field. I think this >> was followed into the later generations. Can we please document it >> correctly and use an uint instead of just bool for set_input_sel? >> > > Can you pls point us to this document you are referring? I have this link in my bookmarks, which doesn't seem to work no longer: https://developer.qualcomm.com/download/sd410/snapdragon-410e-technical-reference-manual.pdf?referrer=node/29241 96boards forum has several links and mentions of this doc. > > I was not aware that bit level details are revealed in external > documents :) > > Even though its a 4-bit field, it only takes a 0 or 1 as others are > undefined. > > Exposing all the bits will only cause more confusion like it did for > others thinking that input select is actually configurable when its not. > > I think what we should do is just pass "misr_type" to this API to tell > whether its lm misr or intf misr and set BIT(24) based on that. This would be another simplification. Can we instead just use values 0 and 1 instead and maybe document that by default everybody should use 0. > > >>> #define MISR_CTRL_FREE_RUN_MASK BIT(31) >>> >>> /* >>> @@ -357,7 +358,8 @@ void _dpu_hw_setup_qos_lut(struct >>> dpu_hw_blk_reg_map *c, u32 offset, >>> bool qos_8lvl, >>> const struct dpu_hw_qos_cfg *cfg); >>> >>> -void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 >>> misr_ctrl_offset); >>> +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 >>> misr_ctrl_offset, >>> + bool set_input_sel); >>> >>> int dpu_hw_collect_misr(struct dpu_hw_blk_reg_map *c, >>> u32 misr_ctrl_offset, >>> >>> -- >>> 2.43.0 >>> >> >>
On 12/2/2023 11:54 AM, Dmitry Baryshkov wrote: > On 01/12/2023 23:29, Abhinav Kumar wrote: >> >> >> On 11/30/2023 11:36 PM, Dmitry Baryshkov wrote: >>> On Fri, 1 Dec 2023 at 03:31, Jessica Zhang >>> <quic_jesszhan@quicinc.com> wrote: >>>> >>>> Set the input_sel bit for encoders as it was missed in the initial >>>> implementation. >>>> >>>> Reported-by: Rob Clark <robdclark@gmail.com> >>>> Fixes: 91143873a05d ("drm/msm/dpu: Add MISR register support for >>>> interface") >>>> Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/39 >>>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >>>> --- >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +- >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 2 +- >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 7 ++++++- >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 +++- >>>> 4 files changed, 11 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>>> index 3442cf65b86f..d0884997ecb7 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>>> @@ -320,7 +320,7 @@ static u32 dpu_hw_intf_get_line_count(struct >>>> dpu_hw_intf *intf) >>>> >>>> static void dpu_hw_intf_setup_misr(struct dpu_hw_intf *intf) >>>> { >>>> - dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL); >>>> + dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL, true); >>>> } >>>> >>>> static int dpu_hw_intf_collect_misr(struct dpu_hw_intf *intf, u32 >>>> *misr_value) >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c >>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c >>>> index f38473e68f79..77b14107c84a 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c >>>> @@ -83,7 +83,7 @@ static void dpu_hw_lm_setup_border_color(struct >>>> dpu_hw_mixer *ctx, >>>> >>>> static void dpu_hw_lm_setup_misr(struct dpu_hw_mixer *ctx) >>>> { >>>> - dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL); >>>> + dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL, false); >>>> } >>>> >>>> static int dpu_hw_lm_collect_misr(struct dpu_hw_mixer *ctx, u32 >>>> *misr_value) >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c >>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c >>>> index a8a0a4e76b94..f441df47fdde 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c >>>> @@ -481,7 +481,8 @@ void _dpu_hw_setup_qos_lut(struct >>>> dpu_hw_blk_reg_map *c, u32 offset, >>>> cfg->danger_safe_en ? >>>> QOS_QOS_CTRL_DANGER_SAFE_EN : 0); >>>> } >>>> >>>> -void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 >>>> misr_ctrl_offset) >>>> +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 >>>> misr_ctrl_offset, >>>> + bool set_input_sel) >>>> { >>>> u32 config = 0; >>>> >>>> @@ -491,6 +492,10 @@ void dpu_hw_setup_misr(struct >>>> dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset) >>>> wmb(); >>>> >>>> config = MISR_FRAME_COUNT | MISR_CTRL_ENABLE | >>>> MISR_CTRL_FREE_RUN_MASK; >>>> + >>>> + if (set_input_sel) >>>> + config |= MISR_CTRL_INPUT_SEL; >>>> + >>>> DPU_REG_WRITE(c, misr_ctrl_offset, config); >>>> } >>>> >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h >>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h >>>> index bb496ebe283b..793670d62414 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h >>>> @@ -17,6 +17,7 @@ >>>> #define MISR_CTRL_ENABLE BIT(8) >>>> #define MISR_CTRL_STATUS BIT(9) >>>> #define MISR_CTRL_STATUS_CLEAR BIT(10) >>>> +#define MISR_CTRL_INPUT_SEL BIT(24) >>> >>> The public apq8916 TRM documents this as a 4-bit field. I think this >>> was followed into the later generations. Can we please document it >>> correctly and use an uint instead of just bool for set_input_sel? >>> >> >> Can you pls point us to this document you are referring? > > I have this link in my bookmarks, which doesn't seem to work no longer: > > https://developer.qualcomm.com/download/sd410/snapdragon-410e-technical-reference-manual.pdf?referrer=node/29241 > > 96boards forum has several links and mentions of this doc. > >> >> I was not aware that bit level details are revealed in external >> documents :) >> >> Even though its a 4-bit field, it only takes a 0 or 1 as others are >> undefined. >> >> Exposing all the bits will only cause more confusion like it did for >> others thinking that input select is actually configurable when its not. >> >> I think what we should do is just pass "misr_type" to this API to tell >> whether its lm misr or intf misr and set BIT(24) based on that. > > This would be another simplification. Can we instead just use values 0 > and 1 instead and maybe document that by default everybody should use 0. Hi Dmitry, Acked. Will change the input_sel parameter to a u8 and add a note that it should be 0x0 by default with an exception for encoders. Thanks, Jessica Zhang > >> >> >>>> #define MISR_CTRL_FREE_RUN_MASK BIT(31) >>>> >>>> /* >>>> @@ -357,7 +358,8 @@ void _dpu_hw_setup_qos_lut(struct >>>> dpu_hw_blk_reg_map *c, u32 offset, >>>> bool qos_8lvl, >>>> const struct dpu_hw_qos_cfg *cfg); >>>> >>>> -void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 >>>> misr_ctrl_offset); >>>> +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 >>>> misr_ctrl_offset, >>>> + bool set_input_sel); >>>> >>>> int dpu_hw_collect_misr(struct dpu_hw_blk_reg_map *c, >>>> u32 misr_ctrl_offset, >>>> >>>> -- >>>> 2.43.0 >>>> >>> >>> > > -- > With best wishes > Dmitry >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c index 3442cf65b86f..d0884997ecb7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c @@ -320,7 +320,7 @@ static u32 dpu_hw_intf_get_line_count(struct dpu_hw_intf *intf) static void dpu_hw_intf_setup_misr(struct dpu_hw_intf *intf) { - dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL); + dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL, true); } static int dpu_hw_intf_collect_misr(struct dpu_hw_intf *intf, u32 *misr_value) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c index f38473e68f79..77b14107c84a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c @@ -83,7 +83,7 @@ static void dpu_hw_lm_setup_border_color(struct dpu_hw_mixer *ctx, static void dpu_hw_lm_setup_misr(struct dpu_hw_mixer *ctx) { - dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL); + dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL, false); } static int dpu_hw_lm_collect_misr(struct dpu_hw_mixer *ctx, u32 *misr_value) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c index a8a0a4e76b94..f441df47fdde 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c @@ -481,7 +481,8 @@ void _dpu_hw_setup_qos_lut(struct dpu_hw_blk_reg_map *c, u32 offset, cfg->danger_safe_en ? QOS_QOS_CTRL_DANGER_SAFE_EN : 0); } -void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset) +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset, + bool set_input_sel) { u32 config = 0; @@ -491,6 +492,10 @@ void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset) wmb(); config = MISR_FRAME_COUNT | MISR_CTRL_ENABLE | MISR_CTRL_FREE_RUN_MASK; + + if (set_input_sel) + config |= MISR_CTRL_INPUT_SEL; + DPU_REG_WRITE(c, misr_ctrl_offset, config); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h index bb496ebe283b..793670d62414 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h @@ -17,6 +17,7 @@ #define MISR_CTRL_ENABLE BIT(8) #define MISR_CTRL_STATUS BIT(9) #define MISR_CTRL_STATUS_CLEAR BIT(10) +#define MISR_CTRL_INPUT_SEL BIT(24) #define MISR_CTRL_FREE_RUN_MASK BIT(31) /* @@ -357,7 +358,8 @@ void _dpu_hw_setup_qos_lut(struct dpu_hw_blk_reg_map *c, u32 offset, bool qos_8lvl, const struct dpu_hw_qos_cfg *cfg); -void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset); +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset, + bool set_input_sel); int dpu_hw_collect_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset,