| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b2fcc9f5-ca11-ab87-e40b-9c6d2662325b@linaro.org>
Date: Fri, 19 May 2023 01:40:19 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Kuogee Hsieh <quic_khsieh@...cinc.com>,
Marijn Suijten <marijn.suijten@...ainline.org>
Cc: dri-devel@...ts.freedesktop.org, robdclark@...il.com,
sean@...rly.run, swboyd@...omium.org, dianders@...omium.org,
vkoul@...nel.org, daniel@...ll.ch, airlied@...il.com,
agross@...nel.org, andersson@...nel.org, quic_abhinavk@...cinc.com,
quic_jesszhan@...cinc.com, quic_sbillaka@...cinc.com,
freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of
interface
On 19/05/2023 01:09, Kuogee Hsieh wrote:
>
> On 5/17/2023 3:31 PM, Marijn Suijten wrote:
>>
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>>> @@ -139,6 +139,11 @@ static inline void
>>> dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>>> CTL_DSPP_n_FLUSH(dspp - DSPP_0),
>>> ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
>>> }
>>> +
>>> + if (ctx->pending_flush_mask & BIT(DSC_IDX))
>>> + DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
>>> + ctx->pending_dsc_flush_mask);
>> Again, when do we reset this mask to 0? (v8 review)
>
> can not find it.
>
> let me add a separate patch to fix this.
The pending_dsc_flush_mask was added in this patch, so the reset should
be a part of this patch too.
>
>>
>>> +
>>> DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
>>> }
>>> @@ -285,6 +290,13 @@ static void
>>> dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
>>> ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
>>> }
>>> +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl
>>> *ctx,
>>> + enum dpu_dsc dsc_num)
>>> +{
>>> + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
>>> + ctx->pending_flush_mask |= BIT(DSC_IDX);
>>> +}
>>> +
>>> static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl
>>> *ctx,
>>> enum dpu_dspp dspp, u32 dspp_sub_blk)
>>> {
>>> @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct
>>> dpu_hw_ctl *ctx,
>>> if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
>>> mode_sel = CTL_DEFAULT_GROUP_ID << 28;
>>> - if (cfg->dsc)
>>> - DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
>>> -
>>> if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
>>> mode_sel |= BIT(17);
>>> @@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct
>>> dpu_hw_ctl *ctx,
>>> if (cfg->merge_3d)
>>> DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
>>> BIT(cfg->merge_3d - MERGE_3D_0));
>>> - if (cfg->dsc) {
>>> - DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
>> Again, this bugfix of now wrapping DSC_IDX in BIT() should go in a
>> separate Fixes: patch to have this semantic change documented. (v8
>> review)
> That will be this patch. let me add Fixes at this patch
_separate_ patch.
>>
>>> +
>>> + if (cfg->dsc)
>>> DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
>>> - }
>>> }
>>> static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
>>> @@ -630,6 +638,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops
>>> *ops,
>>> ops->update_pending_flush_merge_3d =
>>> dpu_hw_ctl_update_pending_flush_merge_3d_v1;
>>> ops->update_pending_flush_wb =
>>> dpu_hw_ctl_update_pending_flush_wb_v1;
>>> + ops->update_pending_flush_dsc =
>>> + dpu_hw_ctl_update_pending_flush_dsc_v1;
>>> } else {
>>> ops->trigger_flush = dpu_hw_ctl_trigger_flush;
>>> ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> index 6292002..d5f3ef8 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> @@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
>>> enum dpu_dspp blk, u32 dspp_sub_blk);
>>> /**
>>> + * OR in the given flushbits to the cached pending_(dsc_)flush_mask
>>> + * No effect on hardware
>>> + * @ctx: ctl path ctx pointer
>>> + * @blk: interface block index
>>> + */
>>> + void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
>>> + enum dpu_dsc blk);
>>> +
>>> + /**
>>> * Write the value of the pending_flush_mask to hardware
>>> * @ctx : ctl path ctx pointer
>>> */
>>> @@ -229,6 +238,9 @@ struct dpu_hw_ctl_ops {
>>> * @pending_flush_mask: storage for pending ctl_flush managed via ops
>>> * @pending_intf_flush_mask: pending INTF flush
>>> * @pending_wb_flush_mask: pending WB flush
>> The above is all capitalized, so...:
>>
>>> + * @pending_merge_3d_flush_mask: pending merge_3d flush
>> MERGE_3D?
>>
>>> + * @pending_dspp_flush_mask: pending dspp flush
>> DSPP
>>
>>> + * @pending_dsc_flush_mask: pending dsc flush
>> DSC
>>
>> - Marijn
>>
>>> * @ops: operation list
>>> */
>>> struct dpu_hw_ctl {
>>> @@ -245,6 +257,7 @@ struct dpu_hw_ctl {
>>> u32 pending_wb_flush_mask;
>>> u32 pending_merge_3d_flush_mask;
>>> u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
>>> + u32 pending_dsc_flush_mask;
>>> /* ops */
>>> struct dpu_hw_ctl_ops ops;
>>> --
>>> 2.7.4
>>>
--
With best wishes
Dmitry
Powered by blists - more mailing lists