* [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix
@ 2023-12-01 1:30 Jessica Zhang
2023-12-01 1:30 ` [PATCH 1/2] drm/msm/dpu: Drop enable and frame_count parameters from dpu_hw_setup_misr() Jessica Zhang
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Jessica Zhang @ 2023-12-01 1:30 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Sean Paul, Marijn Suijten,
David Airlie, Daniel Vetter
Cc: quic_abhinavk, linux-arm-msm, dri-devel, freedreno, linux-kernel,
Jessica Zhang
This series drops the frame_count and enable parameters (as they're always
set to the same value). It also sets input_sel=0x1 for INTF.
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
Jessica Zhang (2):
drm/msm/dpu: Drop enable and frame_count parameters from dpu_hw_setup_misr()
drm/msm/dpu: Set input_sel bit for INTF
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 6 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 6 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h | 3 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 18 +++++++-----------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 11 +++++------
8 files changed, 26 insertions(+), 30 deletions(-)
---
base-commit: 4047f50eb64d980fcd581a19bbe6164dab25ebc7
change-id: 20231122-encoder-fixup-61c190b16085
Best regards,
--
Jessica Zhang <quic_jesszhan@quicinc.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] drm/msm/dpu: Drop enable and frame_count parameters from dpu_hw_setup_misr() 2023-12-01 1:30 [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix Jessica Zhang @ 2023-12-01 1:30 ` Jessica Zhang 2023-12-01 7:38 ` Dmitry Baryshkov 2023-12-01 1:30 ` [PATCH 2/2] drm/msm/dpu: Set input_sel bit for INTF Jessica Zhang 2023-12-01 7:39 ` [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix Dmitry Baryshkov 2 siblings, 1 reply; 10+ messages in thread From: Jessica Zhang @ 2023-12-01 1:30 UTC (permalink / raw) To: Rob Clark, Dmitry Baryshkov, Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter Cc: quic_abhinavk, linux-arm-msm, dri-devel, freedreno, linux-kernel, Jessica Zhang Drop the enable and frame_count parameters from dpu_hw_setup_misr() as they are always set to the same values. In addition, replace MISR_FRAME_COUNT_MASK with MISR_FRAME_COUNT as frame_count is always set to the same value. Fixes: 7b37523fb1d1 ("drm/msm/dpu: Move MISR methods to dpu_hw_util") Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 6 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 6 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 17 ++++------------- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 9 +++------ 8 files changed, 21 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 2b83a13b3aa9..79f2b69429c8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2014-2021 The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark <robdclark@gmail.com> @@ -125,7 +125,7 @@ static void dpu_crtc_setup_lm_misr(struct dpu_crtc_state *crtc_state) continue; /* Calculate MISR over 1 frame */ - m->hw_lm->ops.setup_misr(m->hw_lm, true, 1); + m->hw_lm->ops.setup_misr(m->hw_lm); } } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 1cf7ff6caff4..5dbb5d27bbea 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2,7 +2,7 @@ /* * Copyright (C) 2013 Red Hat * Copyright (c) 2014-2018, 2020-2021 The Linux Foundation. All rights reserved. - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. * * Author: Rob Clark <robdclark@gmail.com> */ @@ -255,7 +255,7 @@ void dpu_encoder_setup_misr(const struct drm_encoder *drm_enc) if (!phys->hw_intf || !phys->hw_intf->ops.setup_misr) continue; - phys->hw_intf->ops.setup_misr(phys->hw_intf, true, 1); + phys->hw_intf->ops.setup_misr(phys->hw_intf); } } 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 e8b8908d3e12..3442cf65b86f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. */ @@ -318,9 +318,9 @@ static u32 dpu_hw_intf_get_line_count(struct dpu_hw_intf *intf) return DPU_REG_READ(c, INTF_LINE_COUNT); } -static void dpu_hw_intf_setup_misr(struct dpu_hw_intf *intf, bool enable, u32 frame_count) +static void dpu_hw_intf_setup_misr(struct dpu_hw_intf *intf) { - dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL, enable, frame_count); + dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL); } 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_intf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h index c539025c418b..66a5603dc7ed 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. */ @@ -95,7 +95,7 @@ struct dpu_hw_intf_ops { void (*bind_pingpong_blk)(struct dpu_hw_intf *intf, const enum dpu_pingpong pp); - void (*setup_misr)(struct dpu_hw_intf *intf, bool enable, u32 frame_count); + void (*setup_misr)(struct dpu_hw_intf *intf); int (*collect_misr)(struct dpu_hw_intf *intf, u32 *misr_value); // Tearcheck on INTF since DPU 5.0.0 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 d1c3bd8379ea..f38473e68f79 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved. */ @@ -81,9 +81,9 @@ 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, bool enable, u32 frame_count) +static void dpu_hw_lm_setup_misr(struct dpu_hw_mixer *ctx) { - dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL, enable, frame_count); + dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL); } 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_lm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h index 36992d046a53..98b77cda6547 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved. */ @@ -57,7 +58,7 @@ struct dpu_hw_lm_ops { /** * setup_misr: Enable/disable MISR */ - void (*setup_misr)(struct dpu_hw_mixer *ctx, bool enable, u32 frame_count); + void (*setup_misr)(struct dpu_hw_mixer *ctx); /** * collect_misr: Read MISR signature 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 18b16b2d2bf5..a8a0a4e76b94 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. */ #define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__ @@ -481,9 +481,7 @@ 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, - bool enable, u32 frame_count) +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset) { u32 config = 0; @@ -492,15 +490,8 @@ void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, /* Clear old MISR value (in case it's read before a new value is calculated)*/ wmb(); - if (enable) { - config = (frame_count & MISR_FRAME_COUNT_MASK) | - MISR_CTRL_ENABLE | MISR_CTRL_FREE_RUN_MASK; - - DPU_REG_WRITE(c, misr_ctrl_offset, config); - } else { - DPU_REG_WRITE(c, misr_ctrl_offset, 0); - } - + config = MISR_FRAME_COUNT | MISR_CTRL_ENABLE | MISR_CTRL_FREE_RUN_MASK; + DPU_REG_WRITE(c, misr_ctrl_offset, config); } int dpu_hw_collect_misr(struct dpu_hw_blk_reg_map *c, 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 4bea139081bc..bb496ebe283b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved. */ @@ -13,7 +13,7 @@ #include "dpu_hw_catalog.h" #define REG_MASK(n) ((BIT(n)) - 1) -#define MISR_FRAME_COUNT_MASK 0xFF +#define MISR_FRAME_COUNT 0x1 #define MISR_CTRL_ENABLE BIT(8) #define MISR_CTRL_STATUS BIT(9) #define MISR_CTRL_STATUS_CLEAR BIT(10) @@ -357,10 +357,7 @@ 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, - bool enable, - u32 frame_count); +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset); int dpu_hw_collect_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset, -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: Drop enable and frame_count parameters from dpu_hw_setup_misr() 2023-12-01 1:30 ` [PATCH 1/2] drm/msm/dpu: Drop enable and frame_count parameters from dpu_hw_setup_misr() Jessica Zhang @ 2023-12-01 7:38 ` Dmitry Baryshkov 0 siblings, 0 replies; 10+ messages in thread From: Dmitry Baryshkov @ 2023-12-01 7:38 UTC (permalink / raw) To: Jessica Zhang Cc: Rob Clark, Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter, quic_abhinavk, linux-arm-msm, dri-devel, freedreno, linux-kernel On Fri, 1 Dec 2023 at 03:31, Jessica Zhang <quic_jesszhan@quicinc.com> wrote: > > Drop the enable and frame_count parameters from dpu_hw_setup_misr() as they > are always set to the same values. > > In addition, replace MISR_FRAME_COUNT_MASK with MISR_FRAME_COUNT as > frame_count is always set to the same value. > > Fixes: 7b37523fb1d1 ("drm/msm/dpu: Move MISR methods to dpu_hw_util") No need for the Fixes tag, there was no issue in that (or the previous) commits > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 6 +++--- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 4 ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 6 +++--- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h | 3 ++- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 17 ++++------------- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 9 +++------ > 8 files changed, 21 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 2b83a13b3aa9..79f2b69429c8 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* > - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > * Copyright (c) 2014-2021 The Linux Foundation. All rights reserved. > * Copyright (C) 2013 Red Hat > * Author: Rob Clark <robdclark@gmail.com> > @@ -125,7 +125,7 @@ static void dpu_crtc_setup_lm_misr(struct dpu_crtc_state *crtc_state) > continue; > > /* Calculate MISR over 1 frame */ > - m->hw_lm->ops.setup_misr(m->hw_lm, true, 1); > + m->hw_lm->ops.setup_misr(m->hw_lm); > } > } > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 1cf7ff6caff4..5dbb5d27bbea 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -2,7 +2,7 @@ > /* > * Copyright (C) 2013 Red Hat > * Copyright (c) 2014-2018, 2020-2021 The Linux Foundation. All rights reserved. > - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > * > * Author: Rob Clark <robdclark@gmail.com> > */ > @@ -255,7 +255,7 @@ void dpu_encoder_setup_misr(const struct drm_encoder *drm_enc) > if (!phys->hw_intf || !phys->hw_intf->ops.setup_misr) > continue; > > - phys->hw_intf->ops.setup_misr(phys->hw_intf, true, 1); > + phys->hw_intf->ops.setup_misr(phys->hw_intf); > } > } > > 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 e8b8908d3e12..3442cf65b86f 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* > - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. > */ > > @@ -318,9 +318,9 @@ static u32 dpu_hw_intf_get_line_count(struct dpu_hw_intf *intf) > return DPU_REG_READ(c, INTF_LINE_COUNT); > } > > -static void dpu_hw_intf_setup_misr(struct dpu_hw_intf *intf, bool enable, u32 frame_count) > +static void dpu_hw_intf_setup_misr(struct dpu_hw_intf *intf) > { > - dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL, enable, frame_count); > + dpu_hw_setup_misr(&intf->hw, INTF_MISR_CTRL); > } > > 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_intf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h > index c539025c418b..66a5603dc7ed 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h > @@ -1,6 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0-only */ > /* > - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. > */ > > @@ -95,7 +95,7 @@ struct dpu_hw_intf_ops { > > void (*bind_pingpong_blk)(struct dpu_hw_intf *intf, > const enum dpu_pingpong pp); > - void (*setup_misr)(struct dpu_hw_intf *intf, bool enable, u32 frame_count); > + void (*setup_misr)(struct dpu_hw_intf *intf); > int (*collect_misr)(struct dpu_hw_intf *intf, u32 *misr_value); > > // Tearcheck on INTF since DPU 5.0.0 > 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 d1c3bd8379ea..f38473e68f79 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* > - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved. > */ > > @@ -81,9 +81,9 @@ 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, bool enable, u32 frame_count) > +static void dpu_hw_lm_setup_misr(struct dpu_hw_mixer *ctx) > { > - dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL, enable, frame_count); > + dpu_hw_setup_misr(&ctx->hw, LM_MISR_CTRL); > } > > 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_lm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h > index 36992d046a53..98b77cda6547 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h > @@ -1,5 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0-only */ > /* > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved. > */ > > @@ -57,7 +58,7 @@ struct dpu_hw_lm_ops { > /** > * setup_misr: Enable/disable MISR > */ > - void (*setup_misr)(struct dpu_hw_mixer *ctx, bool enable, u32 frame_count); > + void (*setup_misr)(struct dpu_hw_mixer *ctx); > > /** > * collect_misr: Read MISR signature > 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 18b16b2d2bf5..a8a0a4e76b94 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* > - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. > */ > #define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__ > @@ -481,9 +481,7 @@ 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, > - bool enable, u32 frame_count) > +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset) > { > u32 config = 0; > > @@ -492,15 +490,8 @@ void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, > /* Clear old MISR value (in case it's read before a new value is calculated)*/ > wmb(); > > - if (enable) { > - config = (frame_count & MISR_FRAME_COUNT_MASK) | > - MISR_CTRL_ENABLE | MISR_CTRL_FREE_RUN_MASK; > - > - DPU_REG_WRITE(c, misr_ctrl_offset, config); > - } else { > - DPU_REG_WRITE(c, misr_ctrl_offset, 0); > - } > - > + config = MISR_FRAME_COUNT | MISR_CTRL_ENABLE | MISR_CTRL_FREE_RUN_MASK; > + DPU_REG_WRITE(c, misr_ctrl_offset, config); > } > > int dpu_hw_collect_misr(struct dpu_hw_blk_reg_map *c, > 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 4bea139081bc..bb496ebe283b 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h > @@ -1,6 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0-only */ > /* > - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved. > */ > > @@ -13,7 +13,7 @@ > #include "dpu_hw_catalog.h" > > #define REG_MASK(n) ((BIT(n)) - 1) > -#define MISR_FRAME_COUNT_MASK 0xFF > +#define MISR_FRAME_COUNT 0x1 > #define MISR_CTRL_ENABLE BIT(8) > #define MISR_CTRL_STATUS BIT(9) > #define MISR_CTRL_STATUS_CLEAR BIT(10) > @@ -357,10 +357,7 @@ 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, > - bool enable, > - u32 frame_count); > +void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c, u32 misr_ctrl_offset); > > int dpu_hw_collect_misr(struct dpu_hw_blk_reg_map *c, > u32 misr_ctrl_offset, > > -- > 2.43.0 > -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] drm/msm/dpu: Set input_sel bit for INTF 2023-12-01 1:30 [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix Jessica Zhang 2023-12-01 1:30 ` [PATCH 1/2] drm/msm/dpu: Drop enable and frame_count parameters from dpu_hw_setup_misr() Jessica Zhang @ 2023-12-01 1:30 ` Jessica Zhang 2023-12-01 7:36 ` Dmitry Baryshkov 2023-12-01 7:39 ` [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix Dmitry Baryshkov 2 siblings, 1 reply; 10+ messages in thread From: Jessica Zhang @ 2023-12-01 1:30 UTC (permalink / raw) To: Rob Clark, Dmitry Baryshkov, Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter Cc: quic_abhinavk, linux-arm-msm, dri-devel, freedreno, linux-kernel, Jessica Zhang 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) #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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/msm/dpu: Set input_sel bit for INTF 2023-12-01 1:30 ` [PATCH 2/2] drm/msm/dpu: Set input_sel bit for INTF Jessica Zhang @ 2023-12-01 7:36 ` Dmitry Baryshkov 2023-12-01 21:29 ` Abhinav Kumar 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Baryshkov @ 2023-12-01 7:36 UTC (permalink / raw) To: Jessica Zhang Cc: Rob Clark, Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter, quic_abhinavk, linux-arm-msm, dri-devel, freedreno, linux-kernel 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 > -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/msm/dpu: Set input_sel bit for INTF 2023-12-01 7:36 ` Dmitry Baryshkov @ 2023-12-01 21:29 ` Abhinav Kumar 2023-12-02 19:54 ` Dmitry Baryshkov 0 siblings, 1 reply; 10+ messages in thread From: Abhinav Kumar @ 2023-12-01 21:29 UTC (permalink / raw) To: Dmitry Baryshkov, Jessica Zhang Cc: Rob Clark, Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno, linux-kernel 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 >> > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/msm/dpu: Set input_sel bit for INTF 2023-12-01 21:29 ` Abhinav Kumar @ 2023-12-02 19:54 ` Dmitry Baryshkov 2023-12-13 19:51 ` Jessica Zhang 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Baryshkov @ 2023-12-02 19:54 UTC (permalink / raw) To: Abhinav Kumar, Jessica Zhang Cc: Rob Clark, Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno, linux-kernel 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 >>> >> >> -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/msm/dpu: Set input_sel bit for INTF 2023-12-02 19:54 ` Dmitry Baryshkov @ 2023-12-13 19:51 ` Jessica Zhang 0 siblings, 0 replies; 10+ messages in thread From: Jessica Zhang @ 2023-12-13 19:51 UTC (permalink / raw) To: Dmitry Baryshkov, Abhinav Kumar Cc: Rob Clark, Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno, linux-kernel 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix 2023-12-01 1:30 [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix Jessica Zhang 2023-12-01 1:30 ` [PATCH 1/2] drm/msm/dpu: Drop enable and frame_count parameters from dpu_hw_setup_misr() Jessica Zhang 2023-12-01 1:30 ` [PATCH 2/2] drm/msm/dpu: Set input_sel bit for INTF Jessica Zhang @ 2023-12-01 7:39 ` Dmitry Baryshkov 2023-12-13 2:37 ` [Freedreno] " Jessica Zhang 2 siblings, 1 reply; 10+ messages in thread From: Dmitry Baryshkov @ 2023-12-01 7:39 UTC (permalink / raw) To: Jessica Zhang Cc: Rob Clark, Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter, quic_abhinavk, linux-arm-msm, dri-devel, freedreno, linux-kernel On Fri, 1 Dec 2023 at 03:31, Jessica Zhang <quic_jesszhan@quicinc.com> wrote: > > This series drops the frame_count and enable parameters (as they're always > set to the same value). It also sets input_sel=0x1 for INTF. > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > Jessica Zhang (2): > drm/msm/dpu: Drop enable and frame_count parameters from dpu_hw_setup_misr() > drm/msm/dpu: Set input_sel bit for INTF Please change the order of the commits: fix (input_sel) comes first, then comes the improvement. Otherwise if one needs to backport this fix, they either need to pick up the irrelevant patch, or they have to rework the fix. > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 6 +++--- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 4 ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 6 +++--- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h | 3 ++- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 18 +++++++----------- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 11 +++++------ > 8 files changed, 26 insertions(+), 30 deletions(-) > --- > base-commit: 4047f50eb64d980fcd581a19bbe6164dab25ebc7 > change-id: 20231122-encoder-fixup-61c190b16085 > > Best regards, > -- > Jessica Zhang <quic_jesszhan@quicinc.com> > -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Freedreno] [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix 2023-12-01 7:39 ` [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix Dmitry Baryshkov @ 2023-12-13 2:37 ` Jessica Zhang 0 siblings, 0 replies; 10+ messages in thread From: Jessica Zhang @ 2023-12-13 2:37 UTC (permalink / raw) To: Dmitry Baryshkov Cc: freedreno, David Airlie, quic_abhinavk, dri-devel, linux-kernel, Rob Clark, Daniel Vetter, linux-arm-msm, Marijn Suijten, Sean Paul On 11/30/2023 11:39 PM, Dmitry Baryshkov wrote: > On Fri, 1 Dec 2023 at 03:31, Jessica Zhang <quic_jesszhan@quicinc.com> wrote: >> >> This series drops the frame_count and enable parameters (as they're always >> set to the same value). It also sets input_sel=0x1 for INTF. >> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> --- >> Jessica Zhang (2): >> drm/msm/dpu: Drop enable and frame_count parameters from dpu_hw_setup_misr() >> drm/msm/dpu: Set input_sel bit for INTF > > Please change the order of the commits: fix (input_sel) comes first, > then comes the improvement. > Otherwise if one needs to backport this fix, they either need to pick > up the irrelevant patch, or they have to rework the fix. Acked. Thanks, Jessica Zhang > >> >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 ++-- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++-- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 6 +++--- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 4 ++-- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 6 +++--- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h | 3 ++- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 18 +++++++----------- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 11 +++++------ >> 8 files changed, 26 insertions(+), 30 deletions(-) >> --- >> base-commit: 4047f50eb64d980fcd581a19bbe6164dab25ebc7 >> change-id: 20231122-encoder-fixup-61c190b16085 >> >> Best regards, >> -- >> Jessica Zhang <quic_jesszhan@quicinc.com> >> > > > -- > With best wishes > Dmitry ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-12-13 19:51 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-01 1:30 [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix Jessica Zhang 2023-12-01 1:30 ` [PATCH 1/2] drm/msm/dpu: Drop enable and frame_count parameters from dpu_hw_setup_misr() Jessica Zhang 2023-12-01 7:38 ` Dmitry Baryshkov 2023-12-01 1:30 ` [PATCH 2/2] drm/msm/dpu: Set input_sel bit for INTF Jessica Zhang 2023-12-01 7:36 ` Dmitry Baryshkov 2023-12-01 21:29 ` Abhinav Kumar 2023-12-02 19:54 ` Dmitry Baryshkov 2023-12-13 19:51 ` Jessica Zhang 2023-12-01 7:39 ` [PATCH 0/2] drm/msm/dpu: INTF CRC configuration cleanups and fix Dmitry Baryshkov 2023-12-13 2:37 ` [Freedreno] " Jessica Zhang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).