| Message ID | 1644009445-17320-9-git-send-email-quic_abhinavk@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Add writeback block support for DPU | expand |
On 05/02/2022 00:17, Abhinav Kumar wrote: > Introduce the dpu_encoder_phys_* for the writeback interface > to handle writeback specific hardware programming. > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Mostly looks ok, see minor comments bellow. > --- > drivers/gpu/drm/msm/Makefile | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 36 +- > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 813 +++++++++++++++++++++ > 3 files changed, 849 insertions(+), 1 deletion(-) > 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 c43ef35..3abaf84 100644 > --- a/drivers/gpu/drm/msm/Makefile > +++ b/drivers/gpu/drm/msm/Makefile > @@ -53,6 +53,7 @@ msm-y := \ > 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 7b3354d..80da0a9 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > @@ -159,6 +159,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, > @@ -166,6 +167,7 @@ enum dpu_intr_idx { > INTR_IDX_UNDERRUN, > INTR_IDX_CTL_START, > INTR_IDX_RDPTR, > + INTR_IDX_WB_DONE, > INTR_IDX_MAX, > }; > > @@ -196,7 +198,7 @@ struct dpu_encoder_irq { > * @hw_ctl: Hardware interface to the ctl registers > * @hw_pp: Hardware interface to the ping pong registers > * @hw_intf: Hardware interface to the intf registers > - * @hw_wb: Hardware interface to the wb registers > + * @hw_wb: Hardware interface to the wb registers > * @dpu_kms: Pointer to the dpu_kms top level > * @cached_mode: DRM mode cached at mode_set time, acted on in enable > * @enabled: Whether the encoder has enabled and running a mode > @@ -250,6 +252,31 @@ 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 > + * @cdp_cfg: chroma down prefetch block config for wb > + * @aspace: address space to be used for wb framebuffer > + * @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 dpu_hw_wb_cdp_cfg cdp_cfg; What about moving them to the stack rather storing them in the structure? > + struct msm_gem_address_space *aspace; > + 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 > @@ -317,6 +344,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..783f83e > --- /dev/null > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > @@ -0,0 +1,813 @@ > +// 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> the header is unused > + > +#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) > +{ > + return true; > +} A comment that there is only one physical enc for WB would be helpful here. But maybe we should just pull a generic is_master into dpu_encoder_phys.h (it would work here too). > + > +/** > + * 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); > +} > + > +//this can be moved to some common file? C99 comment and missing "FIXME: " :-D Yes, let's pull it into dpu_hw_util.c And if you (by chance) would see a value in unifying other QoS/CDP-related functions let's move them to the dpu_hw_util.c too. > +static u64 _dpu_encoder_phys_wb_get_qos_lut(struct dpu_qos_lut_tbl *tbl, > + u32 total_fl) > +{ > + int i; > + > + if (!tbl || !tbl->nentry || !tbl->entries) > + return 0; > + > + for (i = 0; i < tbl->nentry; i++) > + if (total_fl <= tbl->entries[i].fl) > + return tbl->entries[i].lut; > + > + /* if last fl is zero, use as default */ > + if (!tbl->entries[i-1].fl) > + return tbl->entries[i-1].lut; > + > + return 0; > +} > + > +/** > + * 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_encoder_phys_wb_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 || > + !phys_enc->connector) { > + DPU_ERROR("invalid encoder\n"); > + return; > + } > + > + hw_wb = phys_enc->hw_wb; > + wb_cfg = &wb_enc->wb_cfg; > + cdp_cfg = &wb_enc->cdp_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, true); > + } 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, true); > + } > +} > + > +/** > + * 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; > + } > + > + 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_encoder_helper_register_irq(phys, INTR_IDX_WB_DONE); > + if (ret) > + atomic_dec_return(&wb_enc->wbirq_refcount); > + } else if (!enable && > + atomic_dec_return(&wb_enc->wbirq_refcount) == 0) { > + dpu_encoder_helper_unregister_irq(phys, INTR_IDX_WB_DONE); > + if (ret) > + atomic_inc_return(&wb_enc->wbirq_refcount); > + } > +} > + > +/** > + * dpu_encoder_phys_wb_mode_set - set display mode > + * @phys_enc: Pointer to physical encoder > + * @mode: Pointer to requested display mode > + * @adj_mode: Pointer to adjusted display mode > + */ > +static void dpu_encoder_phys_wb_mode_set( > + struct dpu_encoder_phys *phys_enc, > + struct drm_display_mode *mode, > + struct drm_display_mode *adj_mode) > +{ > + struct dpu_encoder_irq *irq; > + > + if (adj_mode) { > + phys_enc->cached_mode = *adj_mode; > + drm_mode_debug_printmodeline(adj_mode); > + DPU_DEBUG("caching mode:\n"); > + } > + > + irq = &phys_enc->irq[INTR_IDX_WB_DONE]; > + irq->irq_idx = 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, > + &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; > + > + 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 (phys_enc->hw_pp->merge_3d) This is a bit.. counterintuitive. I'd prefer hw_ctl->caps->features & BIT(DPU_ACTIVE_CTL). > + dpu_encoder_helper_phys_cleanup(phys_enc); > + > + phys_enc->enable_state = DPU_ENC_DISABLED; > +} > + > +/** > + * dpu_encoder_phys_wb_get_hw_resources - get hardware resources > + * @phys_enc: Pointer to physical encoder > + * @hw_res: Pointer to encoder resources > + */ > +static void dpu_encoder_phys_wb_get_hw_resources( > + struct dpu_encoder_phys *phys_enc, > + struct dpu_encoder_hw_resources *hw_res) > +{ > + if (!phys_enc) { > + DPU_ERROR("invalid encoder\n"); > + return; > + } > + > + hw_res->wbs[phys_enc->intf_idx - WB_0] = INTF_MODE_WB_LINE; > + DPU_DEBUG("[wb:%d] intf_mode=%d\n", phys_enc->intf_idx - WB_0, > + hw_res->wbs[phys_enc->intf_idx - WB_0]); > +} > + > +/** > + * 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 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; > + wb_enc->aspace = phys_enc->dpu_kms->base.aspace; Any particular reason to cache it in wb_enc? > + > + wb_cfg = &wb_enc->wb_cfg; > + > + memset(wb_cfg, 0, sizeof(struct dpu_hw_wb_cfg)); > + > + ret = msm_framebuffer_prepare(job->fb, wb_enc->aspace); > + 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(wb_enc->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); > + > + if (!job->fb) > + return; > + > + msm_framebuffer_cleanup(job->fb, wb_enc->aspace); > + // revisit this after everything else works Everything works now, doesn't it? > + 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->mode_set = dpu_encoder_phys_wb_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->get_hw_resources = dpu_encoder_phys_wb_get_hw_resources; > + 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; > + > + struct dpu_encoder_irq *irq; > + 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 < INTR_IDX_MAX; i++) { > + irq = &phys_enc->irq[i]; > + INIT_LIST_HEAD(&irq->cb.list); > + irq->irq_idx = -EINVAL; > + irq->cb.arg = phys_enc; > + } > + > + irq = &phys_enc->irq[INTR_IDX_WB_DONE]; > + irq->name = "wb_done"; > + irq->intr_idx = INTR_IDX_WB_DONE; > + irq->cb.func = dpu_encoder_phys_wb_done_irq; > + > + 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); > +}
On 2/4/2022 3:19 PM, Dmitry Baryshkov wrote: > On 05/02/2022 00:17, Abhinav Kumar wrote: >> Introduce the dpu_encoder_phys_* for the writeback interface >> to handle writeback specific hardware programming. >> >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > > Mostly looks ok, see minor comments bellow. > >> --- >> drivers/gpu/drm/msm/Makefile | 1 + >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 36 +- >> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 813 >> +++++++++++++++++++++ >> 3 files changed, 849 insertions(+), 1 deletion(-) >> 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 c43ef35..3abaf84 100644 >> --- a/drivers/gpu/drm/msm/Makefile >> +++ b/drivers/gpu/drm/msm/Makefile >> @@ -53,6 +53,7 @@ msm-y := \ >> 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 7b3354d..80da0a9 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >> @@ -159,6 +159,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, >> @@ -166,6 +167,7 @@ enum dpu_intr_idx { >> INTR_IDX_UNDERRUN, >> INTR_IDX_CTL_START, >> INTR_IDX_RDPTR, >> + INTR_IDX_WB_DONE, >> INTR_IDX_MAX, >> }; >> @@ -196,7 +198,7 @@ struct dpu_encoder_irq { >> * @hw_ctl: Hardware interface to the ctl registers >> * @hw_pp: Hardware interface to the ping pong registers >> * @hw_intf: Hardware interface to the intf registers >> - * @hw_wb: Hardware interface to the wb registers >> + * @hw_wb: Hardware interface to the wb registers >> * @dpu_kms: Pointer to the dpu_kms top level >> * @cached_mode: DRM mode cached at mode_set time, acted on in >> enable >> * @enabled: Whether the encoder has enabled and running a mode >> @@ -250,6 +252,31 @@ 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 >> + * @cdp_cfg: chroma down prefetch block config for wb >> + * @aspace: address space to be used for wb framebuffer >> + * @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 dpu_hw_wb_cdp_cfg cdp_cfg; > > What about moving them to the stack rather storing them in the structure? Yes cdp_cfg can be moved to the stack. Will do it in the next version. wb_cfg cannot because for drm_wb, prepare_wb_job has the job's fb attributes which are used to setup the wb. But we commit it only in the encoder_kickoff in dpu_encoder_phys_wb_setup_fb(). So its shared across two methods. > >> + struct msm_gem_address_space *aspace; >> + 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 >> @@ -317,6 +344,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..783f83e >> --- /dev/null >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c >> @@ -0,0 +1,813 @@ >> +// 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> > > the header is unused ack > >> + >> +#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) >> +{ >> + return true; >> +} > > A comment that there is only one physical enc for WB would be helpful > here. But maybe we should just pull a generic is_master into > dpu_encoder_phys.h (it would work here too). I am just following what we have today for phys_vid and phys_cmd. But yes, I can leave a comment for sure. > >> + >> +/** >> + * 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); >> +} >> + >> +//this can be moved to some common file? > > C99 comment and missing "FIXME: " :-D Yes I will remove this since i am addressing this now. > > Yes, let's pull it into dpu_hw_util.c > > And if you (by chance) would see a value in unifying other > QoS/CDP-related functions let's move them to the dpu_hw_util.c too. Yes I think I can move some utility functions like below to the util file. Will take care of it in next version. > >> +static u64 _dpu_encoder_phys_wb_get_qos_lut(struct dpu_qos_lut_tbl *tbl, >> + u32 total_fl) >> +{ >> + int i; >> + >> + if (!tbl || !tbl->nentry || !tbl->entries) >> + return 0; >> + >> + for (i = 0; i < tbl->nentry; i++) >> + if (total_fl <= tbl->entries[i].fl) >> + return tbl->entries[i].lut; >> + >> + /* if last fl is zero, use as default */ >> + if (!tbl->entries[i-1].fl) >> + return tbl->entries[i-1].lut; >> + >> + return 0; >> +} >> + >> +/** >> + * 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_encoder_phys_wb_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 || >> + !phys_enc->connector) { >> + DPU_ERROR("invalid encoder\n"); >> + return; >> + } >> + >> + hw_wb = phys_enc->hw_wb; >> + wb_cfg = &wb_enc->wb_cfg; >> + cdp_cfg = &wb_enc->cdp_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, true); >> + } 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, true); >> + } >> +} >> + >> +/** >> + * 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; >> + } >> + >> + 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_encoder_helper_register_irq(phys, INTR_IDX_WB_DONE); >> + if (ret) >> + atomic_dec_return(&wb_enc->wbirq_refcount); >> + } else if (!enable && >> + atomic_dec_return(&wb_enc->wbirq_refcount) == 0) { >> + dpu_encoder_helper_unregister_irq(phys, INTR_IDX_WB_DONE); >> + if (ret) >> + atomic_inc_return(&wb_enc->wbirq_refcount); >> + } >> +} >> + >> +/** >> + * dpu_encoder_phys_wb_mode_set - set display mode >> + * @phys_enc: Pointer to physical encoder >> + * @mode: Pointer to requested display mode >> + * @adj_mode: Pointer to adjusted display mode >> + */ >> +static void dpu_encoder_phys_wb_mode_set( >> + struct dpu_encoder_phys *phys_enc, >> + struct drm_display_mode *mode, >> + struct drm_display_mode *adj_mode) >> +{ >> + struct dpu_encoder_irq *irq; >> + >> + if (adj_mode) { >> + phys_enc->cached_mode = *adj_mode; >> + drm_mode_debug_printmodeline(adj_mode); >> + DPU_DEBUG("caching mode:\n"); >> + } >> + >> + irq = &phys_enc->irq[INTR_IDX_WB_DONE]; >> + irq->irq_idx = 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, >> + &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; >> + >> + 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 (phys_enc->hw_pp->merge_3d) > > This is a bit.. counterintuitive. I'd prefer hw_ctl->caps->features & > BIT(DPU_ACTIVE_CTL). Your suggestion should work. But consider it this way. Checking caps is telling us if the chipset supports merge3d which is sufficient in this case but phys_enc->hw_pp->merge_3d is telling us whether merge_3d is also being used in this RM session. So i just thought its a better check to cover both. > >> + dpu_encoder_helper_phys_cleanup(phys_enc); >> + >> + phys_enc->enable_state = DPU_ENC_DISABLED; >> +} >> + >> +/** >> + * dpu_encoder_phys_wb_get_hw_resources - get hardware resources >> + * @phys_enc: Pointer to physical encoder >> + * @hw_res: Pointer to encoder resources >> + */ >> +static void dpu_encoder_phys_wb_get_hw_resources( >> + struct dpu_encoder_phys *phys_enc, >> + struct dpu_encoder_hw_resources *hw_res) >> +{ >> + if (!phys_enc) { >> + DPU_ERROR("invalid encoder\n"); >> + return; >> + } >> + >> + hw_res->wbs[phys_enc->intf_idx - WB_0] = INTF_MODE_WB_LINE; >> + DPU_DEBUG("[wb:%d] intf_mode=%d\n", phys_enc->intf_idx - WB_0, >> + hw_res->wbs[phys_enc->intf_idx - WB_0]); >> +} >> + >> +/** >> + * 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 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; >> + wb_enc->aspace = phys_enc->dpu_kms->base.aspace; > > Any particular reason to cache it in wb_enc? We can technically get this from phys_enc->dpu_kms->base.aspace so I can drop this. > >> + >> + wb_cfg = &wb_enc->wb_cfg; >> + >> + memset(wb_cfg, 0, sizeof(struct dpu_hw_wb_cfg)); >> + >> + ret = msm_framebuffer_prepare(job->fb, wb_enc->aspace); >> + 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(wb_enc->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); >> + >> + if (!job->fb) >> + return; >> + >> + msm_framebuffer_cleanup(job->fb, wb_enc->aspace); >> + // revisit this after everything else works > > Everything works now, doesn't it? Yeah, everything works. i forgot to get rid of this comment :) Had it in a bunch of other places too and missed one while cleaning up. > >> + 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->mode_set = dpu_encoder_phys_wb_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->get_hw_resources = dpu_encoder_phys_wb_get_hw_resources; >> + 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; >> + >> + struct dpu_encoder_irq *irq; >> + 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 < INTR_IDX_MAX; i++) { >> + irq = &phys_enc->irq[i]; >> + INIT_LIST_HEAD(&irq->cb.list); >> + irq->irq_idx = -EINVAL; >> + irq->cb.arg = phys_enc; >> + } >> + >> + irq = &phys_enc->irq[INTR_IDX_WB_DONE]; >> + irq->name = "wb_done"; >> + irq->intr_idx = INTR_IDX_WB_DONE; >> + irq->cb.func = dpu_encoder_phys_wb_done_irq; >> + >> + 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); >> +} > >
On 15/04/2022 01:16, Abhinav Kumar wrote: > > > On 2/4/2022 3:19 PM, Dmitry Baryshkov wrote: >> On 05/02/2022 00:17, Abhinav Kumar wrote: >>> Introduce the dpu_encoder_phys_* for the writeback interface >>> to handle writeback specific hardware programming. >>> >>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >> >> Mostly looks ok, see minor comments bellow. >> >>> --- >>> drivers/gpu/drm/msm/Makefile | 1 + >>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 36 +- >>> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 813 >>> +++++++++++++++++++++ >>> 3 files changed, 849 insertions(+), 1 deletion(-) >>> 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 c43ef35..3abaf84 100644 >>> --- a/drivers/gpu/drm/msm/Makefile >>> +++ b/drivers/gpu/drm/msm/Makefile >>> @@ -53,6 +53,7 @@ msm-y := \ >>> 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 7b3354d..80da0a9 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >>> @@ -159,6 +159,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, >>> @@ -166,6 +167,7 @@ enum dpu_intr_idx { >>> INTR_IDX_UNDERRUN, >>> INTR_IDX_CTL_START, >>> INTR_IDX_RDPTR, >>> + INTR_IDX_WB_DONE, >>> INTR_IDX_MAX, >>> }; >>> @@ -196,7 +198,7 @@ struct dpu_encoder_irq { >>> * @hw_ctl: Hardware interface to the ctl registers >>> * @hw_pp: Hardware interface to the ping pong registers >>> * @hw_intf: Hardware interface to the intf registers >>> - * @hw_wb: Hardware interface to the wb registers >>> + * @hw_wb: Hardware interface to the wb registers >>> * @dpu_kms: Pointer to the dpu_kms top level >>> * @cached_mode: DRM mode cached at mode_set time, acted on in >>> enable >>> * @enabled: Whether the encoder has enabled and running a mode >>> @@ -250,6 +252,31 @@ 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 >>> + * @cdp_cfg: chroma down prefetch block config for wb >>> + * @aspace: address space to be used for wb framebuffer >>> + * @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 dpu_hw_wb_cdp_cfg cdp_cfg; >> >> What about moving them to the stack rather storing them in the structure? > > Yes cdp_cfg can be moved to the stack. Will do it in the next version. > > wb_cfg cannot because for drm_wb, prepare_wb_job has the job's fb > attributes which are used to setup the wb. > > But we commit it only in the encoder_kickoff in > dpu_encoder_phys_wb_setup_fb(). > > So its shared across two methods. > >> >>> + struct msm_gem_address_space *aspace; >>> + 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 >>> @@ -317,6 +344,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..783f83e >>> --- /dev/null >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c >>> @@ -0,0 +1,813 @@ >>> +// 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> >> >> the header is unused > ack >> >>> + >>> +#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) >>> +{ >>> + return true; >>> +} >> >> A comment that there is only one physical enc for WB would be helpful >> here. But maybe we should just pull a generic is_master into >> dpu_encoder_phys.h (it would work here too). > > I am just following what we have today for phys_vid and phys_cmd. > But yes, I can leave a comment for sure. > >> >>> + >>> +/** >>> + * 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); >>> +} >>> + >>> +//this can be moved to some common file? >> >> C99 comment and missing "FIXME: " :-D > Yes I will remove this since i am addressing this now. >> >> Yes, let's pull it into dpu_hw_util.c >> >> And if you (by chance) would see a value in unifying other >> QoS/CDP-related functions let's move them to the dpu_hw_util.c too. > > Yes I think I can move some utility functions like below to the util > file. Will take care of it in next version. > >> >>> +static u64 _dpu_encoder_phys_wb_get_qos_lut(struct dpu_qos_lut_tbl >>> *tbl, >>> + u32 total_fl) >>> +{ >>> + int i; >>> + >>> + if (!tbl || !tbl->nentry || !tbl->entries) >>> + return 0; >>> + >>> + for (i = 0; i < tbl->nentry; i++) >>> + if (total_fl <= tbl->entries[i].fl) >>> + return tbl->entries[i].lut; >>> + >>> + /* if last fl is zero, use as default */ >>> + if (!tbl->entries[i-1].fl) >>> + return tbl->entries[i-1].lut; >>> + >>> + return 0; >>> +} >>> + >>> +/** >>> + * 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_encoder_phys_wb_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 || >>> + !phys_enc->connector) { >>> + DPU_ERROR("invalid encoder\n"); >>> + return; >>> + } >>> + >>> + hw_wb = phys_enc->hw_wb; >>> + wb_cfg = &wb_enc->wb_cfg; >>> + cdp_cfg = &wb_enc->cdp_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, true); >>> + } 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, true); >>> + } >>> +} >>> + >>> +/** >>> + * 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; >>> + } >>> + >>> + 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_encoder_helper_register_irq(phys, INTR_IDX_WB_DONE); >>> + if (ret) >>> + atomic_dec_return(&wb_enc->wbirq_refcount); >>> + } else if (!enable && >>> + atomic_dec_return(&wb_enc->wbirq_refcount) == 0) { >>> + dpu_encoder_helper_unregister_irq(phys, INTR_IDX_WB_DONE); >>> + if (ret) >>> + atomic_inc_return(&wb_enc->wbirq_refcount); >>> + } >>> +} >>> + >>> +/** >>> + * dpu_encoder_phys_wb_mode_set - set display mode >>> + * @phys_enc: Pointer to physical encoder >>> + * @mode: Pointer to requested display mode >>> + * @adj_mode: Pointer to adjusted display mode >>> + */ >>> +static void dpu_encoder_phys_wb_mode_set( >>> + struct dpu_encoder_phys *phys_enc, >>> + struct drm_display_mode *mode, >>> + struct drm_display_mode *adj_mode) >>> +{ >>> + struct dpu_encoder_irq *irq; >>> + >>> + if (adj_mode) { >>> + phys_enc->cached_mode = *adj_mode; >>> + drm_mode_debug_printmodeline(adj_mode); >>> + DPU_DEBUG("caching mode:\n"); >>> + } >>> + >>> + irq = &phys_enc->irq[INTR_IDX_WB_DONE]; >>> + irq->irq_idx = 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, >>> + &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; >>> + >>> + 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 (phys_enc->hw_pp->merge_3d) >> >> This is a bit.. counterintuitive. I'd prefer hw_ctl->caps->features & >> BIT(DPU_ACTIVE_CTL). > Your suggestion should work. > > But consider it this way. > > Checking caps is telling us if the chipset supports merge3d which is > sufficient in this case but phys_enc->hw_pp->merge_3d is telling us > whether merge_3d is also being used in this RM session. > > So i just thought its a better check to cover both. hw_pp->merge_3d is set when PP has a MERGE_3D attached in hardware, no matter if it is used in the current pipeline or not. As I wrote, the code is counterintuitive. The comment talks about MDP 5.0, but the condition checks hw_pp->merge_3d, which might not be present for tons of other reasons (starting from somebody forgetting to list it in the hw_catalog). And in fact there is a followup question. Can we push this check (and a legacy reset sequence) into the dpu_encoder_helper_phys_cleanup() itself? Thus the dpu_encoder_phys will just call _helper_phys_cleanup() and let the helper care about hardware details. >>> + dpu_encoder_helper_phys_cleanup(phys_enc); >>> + >>> + phys_enc->enable_state = DPU_ENC_DISABLED; >>> +} >>> + >>> +/** >>> + * dpu_encoder_phys_wb_get_hw_resources - get hardware resources >>> + * @phys_enc: Pointer to physical encoder >>> + * @hw_res: Pointer to encoder resources >>> + */ >>> +static void dpu_encoder_phys_wb_get_hw_resources( >>> + struct dpu_encoder_phys *phys_enc, >>> + struct dpu_encoder_hw_resources *hw_res) >>> +{ >>> + if (!phys_enc) { >>> + DPU_ERROR("invalid encoder\n"); >>> + return; >>> + } >>> + >>> + hw_res->wbs[phys_enc->intf_idx - WB_0] = INTF_MODE_WB_LINE; >>> + DPU_DEBUG("[wb:%d] intf_mode=%d\n", phys_enc->intf_idx - WB_0, >>> + hw_res->wbs[phys_enc->intf_idx - WB_0]); >>> +} >>> + >>> +/** >>> + * 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 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; >>> + wb_enc->aspace = phys_enc->dpu_kms->base.aspace; >> >> Any particular reason to cache it in wb_enc? > > We can technically get this from phys_enc->dpu_kms->base.aspace so I can > drop this. > >> >>> + >>> + wb_cfg = &wb_enc->wb_cfg; >>> + >>> + memset(wb_cfg, 0, sizeof(struct dpu_hw_wb_cfg)); >>> + >>> + ret = msm_framebuffer_prepare(job->fb, wb_enc->aspace); >>> + 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(wb_enc->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); >>> + >>> + if (!job->fb) >>> + return; >>> + >>> + msm_framebuffer_cleanup(job->fb, wb_enc->aspace); >>> + // revisit this after everything else works >> >> Everything works now, doesn't it? > > Yeah, everything works. > i forgot to get rid of this comment :) > Had it in a bunch of other places too and missed one while cleaning up. > >> >>> + 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->mode_set = dpu_encoder_phys_wb_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->get_hw_resources = dpu_encoder_phys_wb_get_hw_resources; >>> + 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; >>> + >>> + struct dpu_encoder_irq *irq; >>> + 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 < INTR_IDX_MAX; i++) { >>> + irq = &phys_enc->irq[i]; >>> + INIT_LIST_HEAD(&irq->cb.list); >>> + irq->irq_idx = -EINVAL; >>> + irq->cb.arg = phys_enc; >>> + } >>> + >>> + irq = &phys_enc->irq[INTR_IDX_WB_DONE]; >>> + irq->name = "wb_done"; >>> + irq->intr_idx = INTR_IDX_WB_DONE; >>> + irq->cb.func = dpu_encoder_phys_wb_done_irq; >>> + >>> + 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); >>> +} >> >>
On 4/14/2022 5:24 PM, Dmitry Baryshkov wrote: > On 15/04/2022 01:16, Abhinav Kumar wrote: >> >> >> On 2/4/2022 3:19 PM, Dmitry Baryshkov wrote: >>> On 05/02/2022 00:17, Abhinav Kumar wrote: >>>> Introduce the dpu_encoder_phys_* for the writeback interface >>>> to handle writeback specific hardware programming. >>>> >>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>> >>> Mostly looks ok, see minor comments bellow. >>> >>>> --- >>>> drivers/gpu/drm/msm/Makefile | 1 + >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 36 +- >>>> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 813 >>>> +++++++++++++++++++++ >>>> 3 files changed, 849 insertions(+), 1 deletion(-) >>>> 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 c43ef35..3abaf84 100644 >>>> --- a/drivers/gpu/drm/msm/Makefile >>>> +++ b/drivers/gpu/drm/msm/Makefile >>>> @@ -53,6 +53,7 @@ msm-y := \ >>>> 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 7b3354d..80da0a9 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >>>> @@ -159,6 +159,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, >>>> @@ -166,6 +167,7 @@ enum dpu_intr_idx { >>>> INTR_IDX_UNDERRUN, >>>> INTR_IDX_CTL_START, >>>> INTR_IDX_RDPTR, >>>> + INTR_IDX_WB_DONE, >>>> INTR_IDX_MAX, >>>> }; >>>> @@ -196,7 +198,7 @@ struct dpu_encoder_irq { >>>> * @hw_ctl: Hardware interface to the ctl registers >>>> * @hw_pp: Hardware interface to the ping pong registers >>>> * @hw_intf: Hardware interface to the intf registers >>>> - * @hw_wb: Hardware interface to the wb registers >>>> + * @hw_wb: Hardware interface to the wb registers >>>> * @dpu_kms: Pointer to the dpu_kms top level >>>> * @cached_mode: DRM mode cached at mode_set time, acted on in >>>> enable >>>> * @enabled: Whether the encoder has enabled and running a >>>> mode >>>> @@ -250,6 +252,31 @@ 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 >>>> + * @cdp_cfg: chroma down prefetch block config for wb >>>> + * @aspace: address space to be used for wb framebuffer >>>> + * @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 dpu_hw_wb_cdp_cfg cdp_cfg; >>> >>> What about moving them to the stack rather storing them in the >>> structure? >> >> Yes cdp_cfg can be moved to the stack. Will do it in the next version. >> >> wb_cfg cannot because for drm_wb, prepare_wb_job has the job's fb >> attributes which are used to setup the wb. >> >> But we commit it only in the encoder_kickoff in >> dpu_encoder_phys_wb_setup_fb(). >> >> So its shared across two methods. >> >>> >>>> + struct msm_gem_address_space *aspace; >>>> + 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 >>>> @@ -317,6 +344,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..783f83e >>>> --- /dev/null >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c >>>> @@ -0,0 +1,813 @@ >>>> +// 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> >>> >>> the header is unused >> ack >>> >>>> + >>>> +#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) >>>> +{ >>>> + return true; >>>> +} >>> >>> A comment that there is only one physical enc for WB would be helpful >>> here. But maybe we should just pull a generic is_master into >>> dpu_encoder_phys.h (it would work here too). >> >> I am just following what we have today for phys_vid and phys_cmd. >> But yes, I can leave a comment for sure. >> >>> >>>> + >>>> +/** >>>> + * 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); >>>> +} >>>> + >>>> +//this can be moved to some common file? >>> >>> C99 comment and missing "FIXME: " :-D >> Yes I will remove this since i am addressing this now. >>> >>> Yes, let's pull it into dpu_hw_util.c >>> >>> And if you (by chance) would see a value in unifying other >>> QoS/CDP-related functions let's move them to the dpu_hw_util.c too. >> >> Yes I think I can move some utility functions like below to the util >> file. Will take care of it in next version. >> >>> >>>> +static u64 _dpu_encoder_phys_wb_get_qos_lut(struct dpu_qos_lut_tbl >>>> *tbl, >>>> + u32 total_fl) >>>> +{ >>>> + int i; >>>> + >>>> + if (!tbl || !tbl->nentry || !tbl->entries) >>>> + return 0; >>>> + >>>> + for (i = 0; i < tbl->nentry; i++) >>>> + if (total_fl <= tbl->entries[i].fl) >>>> + return tbl->entries[i].lut; >>>> + >>>> + /* if last fl is zero, use as default */ >>>> + if (!tbl->entries[i-1].fl) >>>> + return tbl->entries[i-1].lut; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +/** >>>> + * 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_encoder_phys_wb_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 || >>>> + !phys_enc->connector) { >>>> + DPU_ERROR("invalid encoder\n"); >>>> + return; >>>> + } >>>> + >>>> + hw_wb = phys_enc->hw_wb; >>>> + wb_cfg = &wb_enc->wb_cfg; >>>> + cdp_cfg = &wb_enc->cdp_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, true); >>>> + } 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, true); >>>> + } >>>> +} >>>> + >>>> +/** >>>> + * 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; >>>> + } >>>> + >>>> + 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_encoder_helper_register_irq(phys, INTR_IDX_WB_DONE); >>>> + if (ret) >>>> + atomic_dec_return(&wb_enc->wbirq_refcount); >>>> + } else if (!enable && >>>> + atomic_dec_return(&wb_enc->wbirq_refcount) == 0) { >>>> + dpu_encoder_helper_unregister_irq(phys, INTR_IDX_WB_DONE); >>>> + if (ret) >>>> + atomic_inc_return(&wb_enc->wbirq_refcount); >>>> + } >>>> +} >>>> + >>>> +/** >>>> + * dpu_encoder_phys_wb_mode_set - set display mode >>>> + * @phys_enc: Pointer to physical encoder >>>> + * @mode: Pointer to requested display mode >>>> + * @adj_mode: Pointer to adjusted display mode >>>> + */ >>>> +static void dpu_encoder_phys_wb_mode_set( >>>> + struct dpu_encoder_phys *phys_enc, >>>> + struct drm_display_mode *mode, >>>> + struct drm_display_mode *adj_mode) >>>> +{ >>>> + struct dpu_encoder_irq *irq; >>>> + >>>> + if (adj_mode) { >>>> + phys_enc->cached_mode = *adj_mode; >>>> + drm_mode_debug_printmodeline(adj_mode); >>>> + DPU_DEBUG("caching mode:\n"); >>>> + } >>>> + >>>> + irq = &phys_enc->irq[INTR_IDX_WB_DONE]; >>>> + irq->irq_idx = 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, >>>> + &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; >>>> + >>>> + 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 (phys_enc->hw_pp->merge_3d) >>> >>> This is a bit.. counterintuitive. I'd prefer hw_ctl->caps->features & >>> BIT(DPU_ACTIVE_CTL). >> Your suggestion should work. >> >> But consider it this way. >> >> Checking caps is telling us if the chipset supports merge3d which is >> sufficient in this case but phys_enc->hw_pp->merge_3d is telling us >> whether merge_3d is also being used in this RM session. >> >> So i just thought its a better check to cover both. > > hw_pp->merge_3d is set when PP has a MERGE_3D attached in hardware, no > matter if it is used in the current pipeline or not. > > As I wrote, the code is counterintuitive. The comment talks about MDP > 5.0, but the condition checks hw_pp->merge_3d, which might not be > present for tons of other reasons (starting from somebody forgetting to > list it in the hw_catalog). Alright, I will uset the hw_ctl->caps->features & BIT(DPU_ACTIVE_CTL) > > And in fact there is a followup question. Can we push this check (and a > legacy reset sequence) into the dpu_encoder_helper_phys_cleanup() > itself? Thus the dpu_encoder_phys will just call _helper_phys_cleanup() > and let the helper care about hardware details. So like I mentioned, at the moment, I am not fully confident of the testing of the sequence for all use-cases to use if for all interfaces (although downstream is already doing it ). I will certainly, keep testing this and if it looks fine, I can easily move this one level up in a follow-up change and move this out of phys_wb. Right now, its part of phys_wb because only wb uses it. > > >>>> + dpu_encoder_helper_phys_cleanup(phys_enc); >>>> + >>>> + phys_enc->enable_state = DPU_ENC_DISABLED; >>>> +} >>>> + >>>> +/** >>>> + * dpu_encoder_phys_wb_get_hw_resources - get hardware resources >>>> + * @phys_enc: Pointer to physical encoder >>>> + * @hw_res: Pointer to encoder resources >>>> + */ >>>> +static void dpu_encoder_phys_wb_get_hw_resources( >>>> + struct dpu_encoder_phys *phys_enc, >>>> + struct dpu_encoder_hw_resources *hw_res) >>>> +{ >>>> + if (!phys_enc) { >>>> + DPU_ERROR("invalid encoder\n"); >>>> + return; >>>> + } >>>> + >>>> + hw_res->wbs[phys_enc->intf_idx - WB_0] = INTF_MODE_WB_LINE; >>>> + DPU_DEBUG("[wb:%d] intf_mode=%d\n", phys_enc->intf_idx - WB_0, >>>> + hw_res->wbs[phys_enc->intf_idx - WB_0]); >>>> +} >>>> + >>>> +/** >>>> + * 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 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; >>>> + wb_enc->aspace = phys_enc->dpu_kms->base.aspace; >>> >>> Any particular reason to cache it in wb_enc? >> >> We can technically get this from phys_enc->dpu_kms->base.aspace so I >> can drop this. >> >>> >>>> + >>>> + wb_cfg = &wb_enc->wb_cfg; >>>> + >>>> + memset(wb_cfg, 0, sizeof(struct dpu_hw_wb_cfg)); >>>> + >>>> + ret = msm_framebuffer_prepare(job->fb, wb_enc->aspace); >>>> + 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(wb_enc->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); >>>> + >>>> + if (!job->fb) >>>> + return; >>>> + >>>> + msm_framebuffer_cleanup(job->fb, wb_enc->aspace); >>>> + // revisit this after everything else works >>> >>> Everything works now, doesn't it? >> >> Yeah, everything works. >> i forgot to get rid of this comment :) >> Had it in a bunch of other places too and missed one while cleaning up. >> >>> >>>> + 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->mode_set = dpu_encoder_phys_wb_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->get_hw_resources = dpu_encoder_phys_wb_get_hw_resources; >>>> + 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; >>>> + >>>> + struct dpu_encoder_irq *irq; >>>> + 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 < INTR_IDX_MAX; i++) { >>>> + irq = &phys_enc->irq[i]; >>>> + INIT_LIST_HEAD(&irq->cb.list); >>>> + irq->irq_idx = -EINVAL; >>>> + irq->cb.arg = phys_enc; >>>> + } >>>> + >>>> + irq = &phys_enc->irq[INTR_IDX_WB_DONE]; >>>> + irq->name = "wb_done"; >>>> + irq->intr_idx = INTR_IDX_WB_DONE; >>>> + irq->cb.func = dpu_encoder_phys_wb_done_irq; >>>> + >>>> + 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); >>>> +} >>> >>> > >
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index c43ef35..3abaf84 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -53,6 +53,7 @@ msm-y := \ 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 7b3354d..80da0a9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -159,6 +159,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, @@ -166,6 +167,7 @@ enum dpu_intr_idx { INTR_IDX_UNDERRUN, INTR_IDX_CTL_START, INTR_IDX_RDPTR, + INTR_IDX_WB_DONE, INTR_IDX_MAX, }; @@ -196,7 +198,7 @@ struct dpu_encoder_irq { * @hw_ctl: Hardware interface to the ctl registers * @hw_pp: Hardware interface to the ping pong registers * @hw_intf: Hardware interface to the intf registers - * @hw_wb: Hardware interface to the wb registers + * @hw_wb: Hardware interface to the wb registers * @dpu_kms: Pointer to the dpu_kms top level * @cached_mode: DRM mode cached at mode_set time, acted on in enable * @enabled: Whether the encoder has enabled and running a mode @@ -250,6 +252,31 @@ 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 + * @cdp_cfg: chroma down prefetch block config for wb + * @aspace: address space to be used for wb framebuffer + * @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 dpu_hw_wb_cdp_cfg cdp_cfg; + struct msm_gem_address_space *aspace; + 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 @@ -317,6 +344,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..783f83e --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -0,0 +1,813 @@ +// 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) +{ + 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); +} + +//this can be moved to some common file? +static u64 _dpu_encoder_phys_wb_get_qos_lut(struct dpu_qos_lut_tbl *tbl, + u32 total_fl) +{ + int i; + + if (!tbl || !tbl->nentry || !tbl->entries) + return 0; + + for (i = 0; i < tbl->nentry; i++) + if (total_fl <= tbl->entries[i].fl) + return tbl->entries[i].lut; + + /* if last fl is zero, use as default */ + if (!tbl->entries[i-1].fl) + return tbl->entries[i-1].lut; + + return 0; +} + +/** + * 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_encoder_phys_wb_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 || + !phys_enc->connector) { + DPU_ERROR("invalid encoder\n"); + return; + } + + hw_wb = phys_enc->hw_wb; + wb_cfg = &wb_enc->wb_cfg; + cdp_cfg = &wb_enc->cdp_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, true); + } 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, true); + } +} + +/** + * 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; + } + + 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_encoder_helper_register_irq(phys, INTR_IDX_WB_DONE); + if (ret) + atomic_dec_return(&wb_enc->wbirq_refcount); + } else if (!enable && + atomic_dec_return(&wb_enc->wbirq_refcount) == 0) { + dpu_encoder_helper_unregister_irq(phys, INTR_IDX_WB_DONE); + if (ret) + atomic_inc_return(&wb_enc->wbirq_refcount); + } +} + +/** + * dpu_encoder_phys_wb_mode_set - set display mode + * @phys_enc: Pointer to physical encoder + * @mode: Pointer to requested display mode + * @adj_mode: Pointer to adjusted display mode + */ +static void dpu_encoder_phys_wb_mode_set( + struct dpu_encoder_phys *phys_enc, + struct drm_display_mode *mode, + struct drm_display_mode *adj_mode) +{ + struct dpu_encoder_irq *irq; + + if (adj_mode) { + phys_enc->cached_mode = *adj_mode; + drm_mode_debug_printmodeline(adj_mode); + DPU_DEBUG("caching mode:\n"); + } + + irq = &phys_enc->irq[INTR_IDX_WB_DONE]; + irq->irq_idx = 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, + &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; + + 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 (phys_enc->hw_pp->merge_3d) + dpu_encoder_helper_phys_cleanup(phys_enc); + + phys_enc->enable_state = DPU_ENC_DISABLED; +} + +/** + * dpu_encoder_phys_wb_get_hw_resources - get hardware resources + * @phys_enc: Pointer to physical encoder + * @hw_res: Pointer to encoder resources + */ +static void dpu_encoder_phys_wb_get_hw_resources( + struct dpu_encoder_phys *phys_enc, + struct dpu_encoder_hw_resources *hw_res) +{ + if (!phys_enc) { + DPU_ERROR("invalid encoder\n"); + return; + } + + hw_res->wbs[phys_enc->intf_idx - WB_0] = INTF_MODE_WB_LINE; + DPU_DEBUG("[wb:%d] intf_mode=%d\n", phys_enc->intf_idx - WB_0, + hw_res->wbs[phys_enc->intf_idx - WB_0]); +} + +/** + * 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 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; + wb_enc->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, wb_enc->aspace); + 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(wb_enc->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); + + if (!job->fb) + return; + + msm_framebuffer_cleanup(job->fb, wb_enc->aspace); + // revisit this after everything else works + 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->mode_set = dpu_encoder_phys_wb_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->get_hw_resources = dpu_encoder_phys_wb_get_hw_resources; + 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; + + struct dpu_encoder_irq *irq; + 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 < INTR_IDX_MAX; i++) { + irq = &phys_enc->irq[i]; + INIT_LIST_HEAD(&irq->cb.list); + irq->irq_idx = -EINVAL; + irq->cb.arg = phys_enc; + } + + irq = &phys_enc->irq[INTR_IDX_WB_DONE]; + irq->name = "wb_done"; + irq->intr_idx = INTR_IDX_WB_DONE; + irq->cb.func = dpu_encoder_phys_wb_done_irq; + + 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); +}
Introduce the dpu_encoder_phys_* for the writeback interface to handle writeback specific hardware programming. Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> --- drivers/gpu/drm/msm/Makefile | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 36 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 813 +++++++++++++++++++++ 3 files changed, 849 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c