On Wed, 20 Apr 2022 at 22:36, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
>
>
>
> On 4/20/2022 12:26 PM, Dmitry Baryshkov wrote:
> > On 20/04/2022 21:17, Abhinav Kumar wrote:
> >>
> >>
> >> On 4/20/2022 12:49 AM, Dmitry Baryshkov wrote:
> >>> On 20/04/2022 04:46, Abhinav Kumar wrote:
> >>>> Introduce the dpu_encoder_phys_* for the writeback interface
> >>>> to handle writeback specific hardware programming.
> >>>>
> >>>> changes in v2:
> >>>> - rebase on msm-next and fix related dependencies namely
> >>>> the irq cleanup
> >>>> - move cdp_cfg, aspace out of dpu_encoder_phys_wb
> >>>> - leave a comment about wb master
> >>>> - start using _dpu_hw_get_qos_lut from dpu_hw_util
> >>>> - replace hw_pp->merge_3d check with DPU_CTL_ACTIVE_CFG
> >>>>
> >>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> >>>> ---
> >>>> drivers/gpu/drm/msm/Makefile | 1 +
> >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 30 +
> >>>> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 751
> >>>> +++++++++++++++++++++
> >>>> 3 files changed, 782 insertions(+)
> >>>> create mode 100644
> >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> >>>>
> >>>> diff --git a/drivers/gpu/drm/msm/Makefile
> >>>> b/drivers/gpu/drm/msm/Makefile
> >>>> index ca779c1..0387f22 100644
> >>>> --- a/drivers/gpu/drm/msm/Makefile
> >>>> +++ b/drivers/gpu/drm/msm/Makefile
> >>>> @@ -60,6 +60,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
> >>>> disp/dpu1/dpu_encoder.o \
> >>>> disp/dpu1/dpu_encoder_phys_cmd.o \
> >>>> disp/dpu1/dpu_encoder_phys_vid.o \
> >>>> + disp/dpu1/dpu_encoder_phys_wb.o \
> >>>> disp/dpu1/dpu_formats.o \
> >>>> disp/dpu1/dpu_hw_catalog.o \
> >>>> disp/dpu1/dpu_hw_ctl.o \
> >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >>>> index 00951f3..5452f98 100644
> >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> >>>> @@ -150,6 +150,7 @@ struct dpu_encoder_phys_ops {
> >>>> * @INTR_IDX_PINGPONG: Pingpong done unterrupt for cmd mode panel
> >>>> * @INTR_IDX_UNDERRUN: Underrun unterrupt for video and cmd mode
> >>>> panel
> >>>> * @INTR_IDX_RDPTR: Readpointer done unterrupt for cmd mode panel
> >>>> + * @INTR_IDX_WB_DONE: Writeback fone interrupt for virtual connector
> >>>> */
> >>>> enum dpu_intr_idx {
> >>>> INTR_IDX_VSYNC,
> >>>> @@ -157,6 +158,7 @@ enum dpu_intr_idx {
> >>>> INTR_IDX_UNDERRUN,
> >>>> INTR_IDX_CTL_START,
> >>>> INTR_IDX_RDPTR,
> >>>> + INTR_IDX_WB_DONE,
> >>>> INTR_IDX_MAX,
> >>>> };
> >>>> @@ -224,6 +226,27 @@ static inline int
> >>>> dpu_encoder_phys_inc_pending(struct dpu_encoder_phys *phys)
> >>>> }
> >>>> /**
> >>>> + * struct dpu_encoder_phys_wb - sub-class of dpu_encoder_phys to
> >>>> handle command
> >>>> + * mode specific operations
> >>>> + * @base: Baseclass physical encoder structure
> >>>> + * @wbirq_refcount: Reference count of writeback interrupt
> >>>> + * @wb_done_timeout_cnt: number of wb done irq timeout errors
> >>>> + * @wb_cfg: writeback block config to store fb related details
> >>>> + * @wb_conn: backpointer to writeback connector
> >>>> + * @wb_job: backpointer to current writeback job
> >>>> + * @dest: dpu buffer layout for current writeback output buffer
> >>>> + */
> >>>> +struct dpu_encoder_phys_wb {
> >>>> + struct dpu_encoder_phys base;
> >>>> + atomic_t wbirq_refcount;
> >>>> + int wb_done_timeout_cnt;
> >>>> + struct dpu_hw_wb_cfg wb_cfg;
> >>>> + struct drm_writeback_connector *wb_conn;
> >>>> + struct drm_writeback_job *wb_job;
> >>>> + struct dpu_hw_fmt_layout dest;
> >>>> +};
> >>>> +
> >>>> +/**
> >>>> * struct dpu_encoder_phys_cmd - sub-class of dpu_encoder_phys to
> >>>> handle command
> >>>> * mode specific operations
> >>>> * @base: Baseclass physical encoder structure
> >>>> @@ -291,6 +314,13 @@ struct dpu_encoder_phys
> >>>> *dpu_encoder_phys_cmd_init(
> >>>> struct dpu_enc_phys_init_params *p);
> >>>> /**
> >>>> + * dpu_encoder_phys_wb_init - initialize writeback encoder
> >>>> + * @init: Pointer to init info structure with initialization params
> >>>> + */
> >>>> +struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
> >>>> + struct dpu_enc_phys_init_params *p);
> >>>> +
> >>>> +/**
> >>>> * dpu_encoder_helper_trigger_start - control start helper function
> >>>> * This helper function may be optionally specified by physical
> >>>> * encoders if they require ctl_start triggering.
> >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> >>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> >>>> new file mode 100644
> >>>> index 0000000..128317fe
> >>>> --- /dev/null
> >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> >>>> @@ -0,0 +1,751 @@
> >>>> +// SPDX-License-Identifier: GPL-2.0-only
> >>>> +/*
> >>>> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights
> >>>> reserved.
> >>>> + */
> >>>> +
> >>>> +#define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__
> >>>> +
> >>>> +#include <linux/debugfs.h>
> >>>> +
> >>>> +#include "dpu_encoder_phys.h"
> >>>> +#include "dpu_formats.h"
> >>>> +#include "dpu_hw_top.h"
> >>>> +#include "dpu_hw_wb.h"
> >>>> +#include "dpu_hw_lm.h"
> >>>> +#include "dpu_hw_blk.h"
> >>>> +#include "dpu_hw_merge3d.h"
> >>>> +#include "dpu_hw_interrupts.h"
> >>>> +#include "dpu_core_irq.h"
> >>>> +#include "dpu_vbif.h"
> >>>> +#include "dpu_crtc.h"
> >>>> +#include "disp/msm_disp_snapshot.h"
> >>>> +
> >>>> +#define DEFAULT_MAX_WRITEBACK_WIDTH 2048
> >>>> +
> >>>> +#define to_dpu_encoder_phys_wb(x) \
> >>>> + container_of(x, struct dpu_encoder_phys_wb, base)
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_is_master - report wb always as master encoder
> >>>> + */
> >>>> +static bool dpu_encoder_phys_wb_is_master(struct dpu_encoder_phys
> >>>> *phys_enc)
> >>>> +{
> >>>> + /* there is only one physical enc for dpu_writeback */
> >>>> + return true;
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_set_ot_limit - set OT limit for writeback
> >>>> interface
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_set_ot_limit(
> >>>> + struct dpu_encoder_phys *phys_enc)
> >>>> +{
> >>>> + struct dpu_hw_wb *hw_wb = phys_enc->hw_wb;
> >>>> + struct dpu_vbif_set_ot_params ot_params;
> >>>> +
> >>>> + memset(&ot_params, 0, sizeof(ot_params));
> >>>> + ot_params.xin_id = hw_wb->caps->xin_id;
> >>>> + ot_params.num = hw_wb->idx - WB_0;
> >>>> + ot_params.width = phys_enc->cached_mode.hdisplay;
> >>>> + ot_params.height = phys_enc->cached_mode.vdisplay;
> >>>> + ot_params.is_wfd = true;
> >>>> + ot_params.frame_rate = drm_mode_vrefresh(&phys_enc->cached_mode);
> >>>> + ot_params.vbif_idx = hw_wb->caps->vbif_idx;
> >>>> + ot_params.clk_ctrl = hw_wb->caps->clk_ctrl;
> >>>> + ot_params.rd = false;
> >>>> +
> >>>> + dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_set_qos_remap - set QoS remapper for writeback
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_set_qos_remap(
> >>>> + struct dpu_encoder_phys *phys_enc)
> >>>> +{
> >>>> + struct dpu_hw_wb *hw_wb;
> >>>> + struct dpu_vbif_set_qos_params qos_params;
> >>>> +
> >>>> + if (!phys_enc || !phys_enc->parent || !phys_enc->parent->crtc) {
> >>>> + DPU_ERROR("invalid arguments\n");
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + if (!phys_enc->hw_wb || !phys_enc->hw_wb->caps) {
> >>>> + DPU_ERROR("invalid writeback hardware\n");
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + hw_wb = phys_enc->hw_wb;
> >>>> +
> >>>> + memset(&qos_params, 0, sizeof(qos_params));
> >>>> + qos_params.vbif_idx = hw_wb->caps->vbif_idx;
> >>>> + qos_params.xin_id = hw_wb->caps->xin_id;
> >>>> + qos_params.clk_ctrl = hw_wb->caps->clk_ctrl;
> >>>> + qos_params.num = hw_wb->idx - WB_0;
> >>>> + qos_params.is_rt = false;
> >>>> +
> >>>> + DPU_DEBUG("[qos_remap] wb:%d vbif:%d xin:%d is_rt:%d\n",
> >>>> + qos_params.num,
> >>>> + qos_params.vbif_idx,
> >>>> + qos_params.xin_id, qos_params.is_rt);
> >>>> +
> >>>> + dpu_vbif_set_qos_remap(phys_enc->dpu_kms, &qos_params);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_set_qos - set QoS/danger/safe LUTs for
> >>>> writeback
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_set_qos(struct dpu_encoder_phys
> >>>> *phys_enc)
> >>>> +{
> >>>> + struct dpu_hw_wb *hw_wb;
> >>>> + struct dpu_hw_wb_qos_cfg qos_cfg;
> >>>> + struct dpu_mdss_cfg *catalog;
> >>>> + struct dpu_qos_lut_tbl *qos_lut_tb;
> >>>> +
> >>>> + if (!phys_enc || !phys_enc->dpu_kms ||
> >>>> !phys_enc->dpu_kms->catalog) {
> >>>> + DPU_ERROR("invalid parameter(s)\n");
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + catalog = phys_enc->dpu_kms->catalog;
> >>>> +
> >>>> + hw_wb = phys_enc->hw_wb;
> >>>> +
> >>>> + memset(&qos_cfg, 0, sizeof(struct dpu_hw_wb_qos_cfg));
> >>>> + qos_cfg.danger_safe_en = true;
> >>>> + qos_cfg.danger_lut =
> >>>> + catalog->perf.danger_lut_tbl[DPU_QOS_LUT_USAGE_NRT];
> >>>> +
> >>>> + qos_cfg.safe_lut =
> >>>> catalog->perf.safe_lut_tbl[DPU_QOS_LUT_USAGE_NRT];
> >>>> +
> >>>> + qos_lut_tb = &catalog->perf.qos_lut_tbl[DPU_QOS_LUT_USAGE_NRT];
> >>>> + qos_cfg.creq_lut = _dpu_hw_get_qos_lut(qos_lut_tb, 0);
> >>>> +
> >>>> + if (hw_wb->ops.setup_qos_lut)
> >>>> + hw_wb->ops.setup_qos_lut(hw_wb, &qos_cfg);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_setup_fb - setup output framebuffer
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + * @fb: Pointer to output framebuffer
> >>>> + * @wb_roi: Pointer to output region of interest
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_setup_fb(struct dpu_encoder_phys
> >>>> *phys_enc,
> >>>> + struct drm_framebuffer *fb)
> >>>> +{
> >>>> + struct dpu_encoder_phys_wb *wb_enc =
> >>>> to_dpu_encoder_phys_wb(phys_enc);
> >>>> + struct dpu_hw_wb *hw_wb;
> >>>> + struct dpu_hw_wb_cfg *wb_cfg;
> >>>> + struct dpu_hw_wb_cdp_cfg cdp_cfg;
> >>>> +
> >>>> + if (!phys_enc || !phys_enc->dpu_kms ||
> >>>> !phys_enc->dpu_kms->catalog) {
> >>>> + DPU_ERROR("invalid encoder\n");
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + hw_wb = phys_enc->hw_wb;
> >>>> + wb_cfg = &wb_enc->wb_cfg;
> >>>> +
> >>>> + wb_cfg->intf_mode = phys_enc->intf_mode;
> >>>> + wb_cfg->roi.x1 = 0;
> >>>> + wb_cfg->roi.x2 = phys_enc->cached_mode.hdisplay;
> >>>> + wb_cfg->roi.y1 = 0;
> >>>> + wb_cfg->roi.y2 = phys_enc->cached_mode.vdisplay;
> >>>> +
> >>>> + if (hw_wb->ops.setup_roi)
> >>>> + hw_wb->ops.setup_roi(hw_wb, wb_cfg);
> >>>> +
> >>>> + if (hw_wb->ops.setup_outformat)
> >>>> + hw_wb->ops.setup_outformat(hw_wb, wb_cfg);
> >>>> +
> >>>> + if (hw_wb->ops.setup_cdp) {
> >>>> + memset(&cdp_cfg, 0, sizeof(struct dpu_hw_wb_cdp_cfg));
> >>>> +
> >>>> + cdp_cfg.enable = phys_enc->dpu_kms->catalog->perf.cdp_cfg
> >>>> + [DPU_PERF_CDP_USAGE_NRT].wr_enable;
> >>>> + cdp_cfg.ubwc_meta_enable =
> >>>> + DPU_FORMAT_IS_UBWC(wb_cfg->dest.format);
> >>>> + cdp_cfg.tile_amortize_enable =
> >>>> + DPU_FORMAT_IS_UBWC(wb_cfg->dest.format) ||
> >>>> + DPU_FORMAT_IS_TILE(wb_cfg->dest.format);
> >>>> + cdp_cfg.preload_ahead = DPU_WB_CDP_PRELOAD_AHEAD_64;
> >>>> +
> >>>> + hw_wb->ops.setup_cdp(hw_wb, &cdp_cfg);
> >>>> + }
> >>>> +
> >>>> + if (hw_wb->ops.setup_outaddress)
> >>>> + hw_wb->ops.setup_outaddress(hw_wb, wb_cfg);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_setup_cdp - setup chroma down prefetch block
> >>>> + * @phys_enc:Pointer to physical encoder
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_setup_cdp(struct dpu_encoder_phys
> >>>> *phys_enc)
> >>>> +{
> >>>> + struct dpu_hw_wb *hw_wb;
> >>>> + struct dpu_hw_ctl *ctl;
> >>>> +
> >>>> + if (!phys_enc) {
> >>>> + DPU_ERROR("invalid encoder\n");
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + hw_wb = phys_enc->hw_wb;
> >>>> + ctl = phys_enc->hw_ctl;
> >>>> +
> >>>> + if (test_bit(DPU_CTL_ACTIVE_CFG, &ctl->caps->features) &&
> >>>> + (phys_enc->hw_ctl &&
> >>>> + phys_enc->hw_ctl->ops.setup_intf_cfg)) {
> >>>> + struct dpu_hw_intf_cfg intf_cfg = {0};
> >>>> + struct dpu_hw_pingpong *hw_pp = phys_enc->hw_pp;
> >>>> + enum dpu_3d_blend_mode mode_3d;
> >>>> +
> >>>> + mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
> >>>> +
> >>>> + intf_cfg.intf = DPU_NONE;
> >>>> + intf_cfg.wb = hw_wb->idx;
> >>>> +
> >>>> + if (mode_3d && hw_pp && hw_pp->merge_3d)
> >>>> + intf_cfg.merge_3d = hw_pp->merge_3d->idx;
> >>>> +
> >>>> + if (phys_enc->hw_pp->merge_3d &&
> >>>> phys_enc->hw_pp->merge_3d->ops.setup_3d_mode)
> >>>> +
> >>>> phys_enc->hw_pp->merge_3d->ops.setup_3d_mode(phys_enc->hw_pp->merge_3d,
> >>>> + mode_3d);
> >>>> +
> >>>> + /* setup which pp blk will connect to this wb */
> >>>> + if (hw_pp && phys_enc->hw_wb->ops.bind_pingpong_blk)
> >>>> + phys_enc->hw_wb->ops.bind_pingpong_blk(phys_enc->hw_wb,
> >>>> true,
> >>>> + phys_enc->hw_pp->idx);
> >>>> +
> >>>> + phys_enc->hw_ctl->ops.setup_intf_cfg(phys_enc->hw_ctl,
> >>>> &intf_cfg);
> >>>> + } else if (phys_enc->hw_ctl &&
> >>>> phys_enc->hw_ctl->ops.setup_intf_cfg) {
> >>>> + struct dpu_hw_intf_cfg intf_cfg = {0};
> >>>> +
> >>>> + intf_cfg.intf = DPU_NONE;
> >>>> + intf_cfg.wb = hw_wb->idx;
> >>>> + intf_cfg.mode_3d =
> >>>> + dpu_encoder_helper_get_3d_blend_mode(phys_enc);
> >>>> + phys_enc->hw_ctl->ops.setup_intf_cfg(phys_enc->hw_ctl,
> >>>> &intf_cfg);
> >>>> + }
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_atomic_check - verify and fixup given atomic
> >>>> states
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + * @crtc_state: Pointer to CRTC atomic state
> >>>> + * @conn_state: Pointer to connector atomic state
> >>>> + */
> >>>> +static int dpu_encoder_phys_wb_atomic_check(
> >>>> + struct dpu_encoder_phys *phys_enc,
> >>>> + struct drm_crtc_state *crtc_state,
> >>>> + struct drm_connector_state *conn_state)
> >>>> +{
> >>>> + struct drm_framebuffer *fb;
> >>>> + const struct drm_display_mode *mode;
> >>>> +
> >>>> + DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n",
> >>>> + phys_enc->intf_idx, mode->name, mode->hdisplay,
> >>>> mode->vdisplay);
> >>>> +
> >>>> + if (!conn_state->writeback_job || !conn_state->writeback_job->fb)
> >>>> + return 0;
> >>>> +
> >>>> + fb = conn_state->writeback_job->fb;
> >>>> + mode = &crtc_state->mode;
> >>>> +
> >>>> + if (!conn_state || !conn_state->connector) {
> >>>> + DPU_ERROR("invalid connector state\n");
> >>>> + return -EINVAL;
> >>>> + } else if (conn_state->connector->status !=
> >>>> + connector_status_connected) {
> >>>> + DPU_ERROR("connector not connected %d\n",
> >>>> + conn_state->connector->status);
> >>>> + return -EINVAL;
> >>>> + }
> >>>> +
> >>>> + DPU_DEBUG("[fb_id:%u][fb:%u,%u]\n", fb->base.id,
> >>>> + fb->width, fb->height);
> >>>> +
> >>>> + if (fb->width != mode->hdisplay) {
> >>>> + DPU_ERROR("invalid fb w=%d, mode w=%d\n", fb->width,
> >>>> + mode->hdisplay);
> >>>> + return -EINVAL;
> >>>> + } else if (fb->height != mode->vdisplay) {
> >>>> + DPU_ERROR("invalid fb h=%d, mode h=%d\n", fb->height,
> >>>> + mode->vdisplay);
> >>>> + return -EINVAL;
> >>>> + } else if (fb->width > DEFAULT_MAX_WRITEBACK_WIDTH) {
> >>>> + DPU_ERROR("invalid fb w=%d, maxlinewidth=%u\n",
> >>>> + fb->width, DEFAULT_MAX_WRITEBACK_WIDTH);
> >>>> + return -EINVAL;
> >>>> + }
> >>>
> >>> Being not an expert in the DRM WB API, I have a question: are those
> >>> hardware limitations or the DRM requirement that we can not write to
> >>> the region inside the attached FB?
> >>
> >> Both. The first two checks are indeed DRM requirement and infact igt
> >> writeback tests this to ensure that the driver rejects any invalid FBs
> >> which are not equal to the mode that was set.
> >>
> >> https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L409
> >
> >
> > The test is writing to the fb which is smaller than the used video mode
> > (which is an obvious mistake). Is it correst to write back to the FB
> > that is bigger than the selected resolution?
>
> Ah, so your question is whether writeback can just write to a portion of
> the larger FB.
>
> Yes, its possible . So thats another follow-up feature to support WB_ROI.
>
> WB can be made to program a smaller ROI in the larger buffer. It is
> possible. Just not supported in this version and even downstream even
> though the code is present, its not verified extensively.
>
> When we add support for it, we will relax these conditions based on the
> feature.
Ok, thanks for the info.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>
> >
> >> The last check is coming from our hardware >
> >>>
> >>>> +
> >>>> + return 0;
> >>>> +}
> >>>> +
> >>>> +
> >>>> +/**
> >>>> + * _dpu_encoder_phys_wb_update_flush - flush hardware update
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + */
> >>>> +static void _dpu_encoder_phys_wb_update_flush(struct
> >>>> dpu_encoder_phys *phys_enc)
> >>>> +{
> >>>> + struct dpu_hw_wb *hw_wb;
> >>>> + struct dpu_hw_ctl *hw_ctl;
> >>>> + struct dpu_hw_pingpong *hw_pp;
> >>>> + u32 pending_flush = 0;
> >>>> +
> >>>> + if (!phys_enc)
> >>>> + return;
> >>>> +
> >>>> + hw_wb = phys_enc->hw_wb;
> >>>> + hw_pp = phys_enc->hw_pp;
> >>>> + hw_ctl = phys_enc->hw_ctl;
> >>>> +
> >>>> + DPU_DEBUG("[wb:%d]\n", hw_wb->idx - WB_0);
> >>>> +
> >>>> + if (!hw_ctl) {
> >>>> + DPU_DEBUG("[wb:%d] no ctl assigned\n", hw_wb->idx - WB_0);
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + if (hw_ctl->ops.update_pending_flush_wb)
> >>>> + hw_ctl->ops.update_pending_flush_wb(hw_ctl, hw_wb->idx);
> >>>> +
> >>>> + if (hw_ctl->ops.update_pending_flush_merge_3d && hw_pp &&
> >>>> hw_pp->merge_3d)
> >>>> + hw_ctl->ops.update_pending_flush_merge_3d(hw_ctl,
> >>>> + hw_pp->merge_3d->idx);
> >>>> +
> >>>> + if (hw_ctl->ops.get_pending_flush)
> >>>> + pending_flush = hw_ctl->ops.get_pending_flush(hw_ctl);
> >>>> +
> >>>> + DPU_DEBUG("Pending flush mask for CTL_%d is 0x%x, WB %d\n",
> >>>> + hw_ctl->idx - CTL_0, pending_flush,
> >>>> + hw_wb->idx - WB_0);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_setup - setup writeback encoder
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_setup(
> >>>> + struct dpu_encoder_phys *phys_enc)
> >>>> +{
> >>>> + struct dpu_hw_wb *hw_wb = phys_enc->hw_wb;
> >>>> + struct drm_display_mode mode = phys_enc->cached_mode;
> >>>> + struct drm_framebuffer *fb = NULL;
> >>>> +
> >>>> + DPU_DEBUG("[mode_set:%d, \"%s\",%d,%d]\n",
> >>>> + hw_wb->idx - WB_0, mode.name,
> >>>> + mode.hdisplay, mode.vdisplay);
> >>>> +
> >>>> + dpu_encoder_phys_wb_set_ot_limit(phys_enc);
> >>>> +
> >>>> + dpu_encoder_phys_wb_set_qos_remap(phys_enc);
> >>>> +
> >>>> + dpu_encoder_phys_wb_set_qos(phys_enc);
> >>>> +
> >>>> + dpu_encoder_phys_wb_setup_fb(phys_enc, fb);
> >>>> +
> >>>> + dpu_encoder_phys_wb_setup_cdp(phys_enc);
> >>>> +
> >>>> +}
> >>>> +
> >>>> +static void _dpu_encoder_phys_wb_frame_done_helper(void *arg)
> >>>> +{
> >>>> + struct dpu_encoder_phys *phys_enc = arg;
> >>>> + struct dpu_encoder_phys_wb *wb_enc =
> >>>> to_dpu_encoder_phys_wb(phys_enc);
> >>>> +
> >>>> + struct dpu_hw_wb *hw_wb = phys_enc->hw_wb;
> >>>> + unsigned long lock_flags;
> >>>> + u32 event = DPU_ENCODER_FRAME_EVENT_DONE;
> >>>> +
> >>>> + DPU_DEBUG("[wb:%d]\n", hw_wb->idx - WB_0);
> >>>> +
> >>>> + if (phys_enc->parent_ops->handle_frame_done)
> >>>> + phys_enc->parent_ops->handle_frame_done(phys_enc->parent,
> >>>> + phys_enc, event);
> >>>> +
> >>>> + if (phys_enc->parent_ops->handle_vblank_virt)
> >>>> + phys_enc->parent_ops->handle_vblank_virt(phys_enc->parent,
> >>>> + phys_enc);
> >>>> +
> >>>> + spin_lock_irqsave(phys_enc->enc_spinlock, lock_flags);
> >>>> + atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0);
> >>>> + spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
> >>>> +
> >>>> + if (wb_enc->wb_conn)
> >>>> + drm_writeback_signal_completion(wb_enc->wb_conn, 0);
> >>>> +
> >>>> + /* Signal any waiting atomic commit thread */
> >>>> + wake_up_all(&phys_enc->pending_kickoff_wq);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_done_irq - writeback interrupt handler
> >>>> + * @arg: Pointer to writeback encoder
> >>>> + * @irq_idx: interrupt index
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_done_irq(void *arg, int irq_idx)
> >>>> +{
> >>>> + _dpu_encoder_phys_wb_frame_done_helper(arg);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_irq_ctrl - irq control of WB
> >>>> + * @phys: Pointer to physical encoder
> >>>> + * @enable: indicates enable or disable interrupts
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_irq_ctrl(
> >>>> + struct dpu_encoder_phys *phys, bool enable)
> >>>> +{
> >>>> +
> >>>> + struct dpu_encoder_phys_wb *wb_enc = to_dpu_encoder_phys_wb(phys);
> >>>> + int ret = 0;
> >>>> + int refcount;
> >>>> +
> >>>> + refcount = atomic_read(&wb_enc->wbirq_refcount);
> >>>> +
> >>>> + if (enable && atomic_inc_return(&wb_enc->wbirq_refcount) == 1) {
> >>>> + dpu_core_irq_register_callback(phys->dpu_kms,
> >>>> + phys->irq[INTR_IDX_WB_DONE],
> >>>> dpu_encoder_phys_wb_done_irq, phys);
> >>>> + if (ret)
> >>>> + atomic_dec_return(&wb_enc->wbirq_refcount);
> >>>> + } else if (!enable &&
> >>>> + atomic_dec_return(&wb_enc->wbirq_refcount) == 0) {
> >>>> + dpu_core_irq_unregister_callback(phys->dpu_kms,
> >>>> phys->irq[INTR_IDX_WB_DONE]);
> >>>> + if (ret)
> >>>> + atomic_inc_return(&wb_enc->wbirq_refcount);
> >>>> + }
> >>>> +}
> >>>> +
> >>>> +static void dpu_encoder_phys_wb_atomic_mode_set(
> >>>> + struct dpu_encoder_phys *phys_enc,
> >>>> + struct drm_crtc_state *crtc_state,
> >>>> + struct drm_connector_state *conn_state)
> >>>> +{
> >>>> +
> >>>> + phys_enc->irq[INTR_IDX_WB_DONE] =
> >>>> phys_enc->hw_wb->caps->intr_wb_done;
> >>>> +}
> >>>> +
> >>>> +static void _dpu_encoder_phys_wb_handle_wbdone_timeout(
> >>>> + struct dpu_encoder_phys *phys_enc)
> >>>> +{
> >>>> + struct dpu_encoder_phys_wb *wb_enc =
> >>>> to_dpu_encoder_phys_wb(phys_enc);
> >>>> + u32 frame_event = DPU_ENCODER_FRAME_EVENT_ERROR;
> >>>> +
> >>>> + wb_enc->wb_done_timeout_cnt++;
> >>>> +
> >>>> + if (wb_enc->wb_done_timeout_cnt == 1)
> >>>> + msm_disp_snapshot_state(phys_enc->parent->dev);
> >>>> +
> >>>> + atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0);
> >>>> +
> >>>> + /* request a ctl reset before the next kickoff */
> >>>> + phys_enc->enable_state = DPU_ENC_ERR_NEEDS_HW_RESET;
> >>>> +
> >>>> + if (wb_enc->wb_conn)
> >>>> + drm_writeback_signal_completion(wb_enc->wb_conn, 0);
> >>>> +
> >>>> + if (phys_enc->parent_ops->handle_frame_done)
> >>>> + phys_enc->parent_ops->handle_frame_done(
> >>>> + phys_enc->parent, phys_enc, frame_event);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_wait_for_commit_done - wait until request is
> >>>> committed
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + */
> >>>> +static int dpu_encoder_phys_wb_wait_for_commit_done(
> >>>> + struct dpu_encoder_phys *phys_enc)
> >>>> +{
> >>>> + unsigned long ret;
> >>>> + struct dpu_encoder_wait_info wait_info;
> >>>> + struct dpu_encoder_phys_wb *wb_enc =
> >>>> to_dpu_encoder_phys_wb(phys_enc);
> >>>> +
> >>>> + wait_info.wq = &phys_enc->pending_kickoff_wq;
> >>>> + wait_info.atomic_cnt = &phys_enc->pending_kickoff_cnt;
> >>>> + wait_info.timeout_ms = KICKOFF_TIMEOUT_MS;
> >>>> +
> >>>> + ret = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_WB_DONE,
> >>>> + dpu_encoder_phys_wb_done_irq, &wait_info);
> >>>> + if (ret == -ETIMEDOUT)
> >>>> + _dpu_encoder_phys_wb_handle_wbdone_timeout(phys_enc);
> >>>> + else if (!ret)
> >>>> + wb_enc->wb_done_timeout_cnt = 0;
> >>>> +
> >>>> + return ret;
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_prepare_for_kickoff - pre-kickoff processing
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + * Returns: Zero on success
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_prepare_for_kickoff(
> >>>> + struct dpu_encoder_phys *phys_enc)
> >>>> +{
> >>>> + struct dpu_encoder_phys_wb *wb_enc =
> >>>> to_dpu_encoder_phys_wb(phys_enc);
> >>>> + struct drm_connector *drm_conn;
> >>>> + struct drm_connector_state *state;
> >>>> +
> >>>> + DPU_DEBUG("[wb:%d]\n", phys_enc->hw_wb->idx - WB_0);
> >>>> +
> >>>> + if (!wb_enc->wb_conn || !wb_enc->wb_job) {
> >>>> + DPU_ERROR("invalid wb_conn or wb_job\n");
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + drm_conn = &wb_enc->wb_conn->base;
> >>>> + state = drm_conn->state;
> >>>> +
> >>>> + if (wb_enc->wb_conn && wb_enc->wb_job)
> >>>> + drm_writeback_queue_job(wb_enc->wb_conn, state);
> >>>> +
> >>>> + dpu_encoder_phys_wb_setup(phys_enc);
> >>>> +
> >>>> + _dpu_encoder_phys_wb_update_flush(phys_enc);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_needs_single_flush - trigger flush processing
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + */
> >>>> +static bool dpu_encoder_phys_wb_needs_single_flush(struct
> >>>> dpu_encoder_phys *phys_enc)
> >>>> +{
> >>>> + DPU_DEBUG("[wb:%d]\n", phys_enc->hw_wb->idx - WB_0);
> >>>> + return false;
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_handle_post_kickoff - post-kickoff processing
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_handle_post_kickoff(
> >>>> + struct dpu_encoder_phys *phys_enc)
> >>>> +{
> >>>> + DPU_DEBUG("[wb:%d]\n", phys_enc->hw_wb->idx - WB_0);
> >>>> +
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_enable - enable writeback encoder
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_enable(struct dpu_encoder_phys
> >>>> *phys_enc)
> >>>> +{
> >>>> + DPU_DEBUG("[wb:%d]\n", phys_enc->hw_wb->idx - WB_0);
> >>>> + phys_enc->enable_state = DPU_ENC_ENABLED;
> >>>> +}
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_disable - disable writeback encoder
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_disable(struct dpu_encoder_phys
> >>>> *phys_enc)
> >>>> +{
> >>>> + struct dpu_hw_wb *hw_wb = phys_enc->hw_wb;
> >>>> + struct dpu_hw_ctl *hw_ctl = phys_enc->hw_ctl;
> >>>> +
> >>>> + DPU_DEBUG("[wb:%d]\n", hw_wb->idx - WB_0);
> >>>> +
> >>>> + if (phys_enc->enable_state == DPU_ENC_DISABLED) {
> >>>> + DPU_ERROR("encoder is already disabled\n");
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + /* reset h/w before final flush */
> >>>> + if (phys_enc->hw_ctl->ops.clear_pending_flush)
> >>>> + phys_enc->hw_ctl->ops.clear_pending_flush(phys_enc->hw_ctl);
> >>>> +
> >>>> + /*
> >>>> + * New CTL reset sequence from 5.0 MDP onwards.
> >>>> + * If has_3d_merge_reset is not set, legacy reset
> >>>> + * sequence is executed.
> >>>> + *
> >>>> + * Legacy reset sequence has not been implemented yet.
> >>>> + * Any target earlier than SM8150 will need it and when
> >>>> + * WB support is added to those targets will need to add
> >>>> + * the legacy teardown sequence as well.
> >>>> + */
> >>>> + if (hw_ctl->caps->features & BIT(DPU_CTL_ACTIVE_CFG))
> >>>> + dpu_encoder_helper_phys_cleanup(phys_enc);
> >>>> +
> >>>> + phys_enc->enable_state = DPU_ENC_DISABLED;
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_destroy - destroy writeback encoder
> >>>> + * @phys_enc: Pointer to physical encoder
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_destroy(struct dpu_encoder_phys
> >>>> *phys_enc)
> >>>> +{
> >>>> + DPU_DEBUG("[wb:%d]\n", phys_enc->intf_idx - INTF_0);
> >>>> +
> >>>> + if (!phys_enc)
> >>>> + return;
> >>>> +
> >>>> + kfree(phys_enc);
> >>>> +}
> >>>> +
> >>>> +static void dpu_encoder_phys_wb_prepare_wb_job(struct
> >>>> dpu_encoder_phys *phys_enc,
> >>>> + struct drm_writeback_job *job)
> >>>> +{
> >>>> + const struct msm_format *format;
> >>>> + struct msm_gem_address_space *aspace;
> >>>> + struct dpu_hw_wb_cfg *wb_cfg;
> >>>> + int ret;
> >>>> + struct dpu_encoder_phys_wb *wb_enc =
> >>>> to_dpu_encoder_phys_wb(phys_enc);
> >>>> +
> >>>> + if (!job->fb)
> >>>> + return;
> >>>> +
> >>>> + wb_enc->wb_job = job;
> >>>> + wb_enc->wb_conn = job->connector;
> >>>> + aspace = phys_enc->dpu_kms->base.aspace;
> >>>> +
> >>>> + wb_cfg = &wb_enc->wb_cfg;
> >>>> +
> >>>> + memset(wb_cfg, 0, sizeof(struct dpu_hw_wb_cfg));
> >>>> +
> >>>> + ret = msm_framebuffer_prepare(job->fb, aspace, false);
> >>>> + if (ret) {
> >>>> + DPU_ERROR("prep fb failed, %d\n", ret);
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + format = msm_framebuffer_format(job->fb);
> >>>> +
> >>>> + wb_cfg->dest.format = dpu_get_dpu_format_ext(
> >>>> + format->pixel_format, job->fb->modifier);
> >>>> + if (!wb_cfg->dest.format) {
> >>>> + /* this error should be detected during atomic_check */
> >>>> + DPU_ERROR("failed to get format %x\n", format->pixel_format);
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + ret = dpu_format_populate_layout(aspace, job->fb, &wb_cfg->dest);
> >>>> + if (ret) {
> >>>> + DPU_DEBUG("failed to populate layout %d\n", ret);
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + wb_cfg->dest.width = job->fb->width;
> >>>> + wb_cfg->dest.height = job->fb->height;
> >>>> + wb_cfg->dest.num_planes = wb_cfg->dest.format->num_planes;
> >>>> +
> >>>> + if ((wb_cfg->dest.format->fetch_planes == DPU_PLANE_PLANAR) &&
> >>>> + (wb_cfg->dest.format->element[0] == C1_B_Cb))
> >>>> + swap(wb_cfg->dest.plane_addr[1], wb_cfg->dest.plane_addr[2]);
> >>>> +
> >>>> + DPU_DEBUG("[fb_offset:%8.8x,%8.8x,%8.8x,%8.8x]\n",
> >>>> + wb_cfg->dest.plane_addr[0], wb_cfg->dest.plane_addr[1],
> >>>> + wb_cfg->dest.plane_addr[2], wb_cfg->dest.plane_addr[3]);
> >>>> +
> >>>> + DPU_DEBUG("[fb_stride:%8.8x,%8.8x,%8.8x,%8.8x]\n",
> >>>> + wb_cfg->dest.plane_pitch[0], wb_cfg->dest.plane_pitch[1],
> >>>> + wb_cfg->dest.plane_pitch[2], wb_cfg->dest.plane_pitch[3]);
> >>>> +}
> >>>> +
> >>>> +static void dpu_encoder_phys_wb_cleanup_wb_job(struct
> >>>> dpu_encoder_phys *phys_enc,
> >>>> + struct drm_writeback_job *job)
> >>>> +{
> >>>> + struct dpu_encoder_phys_wb *wb_enc =
> >>>> to_dpu_encoder_phys_wb(phys_enc);
> >>>> + struct msm_gem_address_space *aspace;
> >>>> +
> >>>> + if (!job->fb)
> >>>> + return;
> >>>> +
> >>>> + aspace = phys_enc->dpu_kms->base.aspace;
> >>>> +
> >>>> + msm_framebuffer_cleanup(job->fb, aspace, false);
> >>>> + wb_enc->wb_job = NULL;
> >>>> + wb_enc->wb_conn = NULL;
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_init_ops - initialize writeback operations
> >>>> + * @ops: Pointer to encoder operation table
> >>>> + */
> >>>> +static void dpu_encoder_phys_wb_init_ops(struct
> >>>> dpu_encoder_phys_ops *ops)
> >>>> +{
> >>>> + ops->is_master = dpu_encoder_phys_wb_is_master;
> >>>> + ops->atomic_mode_set = dpu_encoder_phys_wb_atomic_mode_set;
> >>>> + ops->enable = dpu_encoder_phys_wb_enable;
> >>>> + ops->disable = dpu_encoder_phys_wb_disable;
> >>>> + ops->destroy = dpu_encoder_phys_wb_destroy;
> >>>> + ops->atomic_check = dpu_encoder_phys_wb_atomic_check;
> >>>> + ops->wait_for_commit_done =
> >>>> dpu_encoder_phys_wb_wait_for_commit_done;
> >>>> + ops->prepare_for_kickoff =
> >>>> dpu_encoder_phys_wb_prepare_for_kickoff;
> >>>> + ops->handle_post_kickoff =
> >>>> dpu_encoder_phys_wb_handle_post_kickoff;
> >>>> + ops->needs_single_flush = dpu_encoder_phys_wb_needs_single_flush;
> >>>> + ops->trigger_start = dpu_encoder_helper_trigger_start;
> >>>> + ops->prepare_wb_job = dpu_encoder_phys_wb_prepare_wb_job;
> >>>> + ops->cleanup_wb_job = dpu_encoder_phys_wb_cleanup_wb_job;
> >>>> + ops->irq_control = dpu_encoder_phys_wb_irq_ctrl;
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * dpu_encoder_phys_wb_init - initialize writeback encoder
> >>>> + * @init: Pointer to init info structure with initialization params
> >>>> + */
> >>>> +struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
> >>>> + struct dpu_enc_phys_init_params *p)
> >>>> +{
> >>>> + struct dpu_encoder_phys *phys_enc = NULL;
> >>>> + struct dpu_encoder_phys_wb *wb_enc = NULL;
> >>>> + int ret = 0;
> >>>> + int i;
> >>>> +
> >>>> + DPU_DEBUG("\n");
> >>>> +
> >>>> + if (!p || !p->parent) {
> >>>> + DPU_ERROR("invalid params\n");
> >>>> + ret = -EINVAL;
> >>>> + goto fail_alloc;
> >>>> + }
> >>>> +
> >>>> + wb_enc = kzalloc(sizeof(*wb_enc), GFP_KERNEL);
> >>>> + if (!wb_enc) {
> >>>> + DPU_ERROR("failed to allocate wb phys_enc enc\n");
> >>>> + ret = -ENOMEM;
> >>>> + goto fail_alloc;
> >>>> + }
> >>>> +
> >>>> + phys_enc = &wb_enc->base;
> >>>> + phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
> >>>> + phys_enc->intf_idx = p->intf_idx;
> >>>> +
> >>>> + dpu_encoder_phys_wb_init_ops(&phys_enc->ops);
> >>>> + phys_enc->parent = p->parent;
> >>>> + phys_enc->parent_ops = p->parent_ops;
> >>>> + phys_enc->dpu_kms = p->dpu_kms;
> >>>> + phys_enc->split_role = p->split_role;
> >>>> + phys_enc->intf_mode = INTF_MODE_WB_LINE;
> >>>> + phys_enc->intf_idx = p->intf_idx;
> >>>> + phys_enc->enc_spinlock = p->enc_spinlock;
> >>>> +
> >>>> + atomic_set(&wb_enc->wbirq_refcount, 0);
> >>>> +
> >>>> + for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
> >>>> + phys_enc->irq[i] = -EINVAL;
> >>>> +
> >>>> + atomic_set(&phys_enc->pending_kickoff_cnt, 0);
> >>>> + atomic_set(&phys_enc->vblank_refcount, 0);
> >>>> + wb_enc->wb_done_timeout_cnt = 0;
> >>>> +
> >>>> + init_waitqueue_head(&phys_enc->pending_kickoff_wq);
> >>>> + phys_enc->enable_state = DPU_ENC_DISABLED;
> >>>> +
> >>>> + DPU_DEBUG("Created dpu_encoder_phys for wb %d\n",
> >>>> + phys_enc->intf_idx);
> >>>> +
> >>>> + return phys_enc;
> >>>> +
> >>>> +fail_alloc:
> >>>> + return ERR_PTR(ret);
> >>>> +}
> >>>
> >>>
> >
> >
--
With best wishes
Dmitry
