On 1/26/2024 4:39 PM, Paloma Arellano wrote:
On 1/25/2024 1:14 PM, Dmitry Baryshkov wrote:On 25/01/2024 21:38, Paloma Arellano wrote:Generalize dpu_encoder_helper_phys_setup_cdm to be compatible with DP. Signed-off-by: Paloma Arellano <quic_parellan@xxxxxxxxxxx> --- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 +-- .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 31 ++++++++++--------- 2 files changed, 18 insertions(+), 17 deletions(-)diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.hindex 993f263433314..37ac385727c3b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -153,6 +153,7 @@ enum dpu_intr_idx { * @hw_intf: Hardware interface to the intf registers * @hw_wb: Hardware interface to the wb registers * @hw_cdm: Hardware interface to the CDM registers+ * @cdm_cfg: CDM block config needed to store WB/DP block's CDM configurationPlease realign the description.Ack* @dpu_kms: Pointer to the dpu_kms top level* @cached_mode: DRM mode cached at mode_set time, acted on in enable * @vblank_ctl_lock: Vblank ctl mutex lock to protect vblank_refcount@@ -183,6 +184,7 @@ struct dpu_encoder_phys { struct dpu_hw_intf *hw_intf; struct dpu_hw_wb *hw_wb; struct dpu_hw_cdm *hw_cdm; + struct dpu_hw_cdm_cfg cdm_cfg;It might be slightly better to move it after all the pointers, so after the dpu_kms.Ackstruct dpu_kms *dpu_kms; struct drm_display_mode cached_mode; struct mutex vblank_ctl_lock;@@ -213,7 +215,6 @@ static inline int dpu_encoder_phys_inc_pending(struct dpu_encoder_phys *phys)* @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- * @cdm_cfg: cdm block config needed to store writeback block's CDM configuration* @wb_conn: backpointer to writeback connector * @wb_job: backpointer to current writeback job * @dest: dpu buffer layout for current writeback output buffer @@ -223,7 +224,6 @@ struct dpu_encoder_phys_wb { atomic_t wbirq_refcount; int wb_done_timeout_cnt; struct dpu_hw_wb_cfg wb_cfg; - struct dpu_hw_cdm_cfg cdm_cfg; struct drm_writeback_connector *wb_conn; struct drm_writeback_job *wb_job; struct dpu_hw_fmt_layout dest;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.cindex 4cd2d9e3131a4..072fc6950e496 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c@@ -269,28 +269,21 @@ static void dpu_encoder_phys_wb_setup_ctl(struct dpu_encoder_phys *phys_enc) * This API does not handle DPU_CHROMA_H1V2.* @phys_enc:Pointer to physical encoder */-static void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys *phys_enc) +static void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys *phys_enc,+ const struct dpu_format *dpu_fmt, + u32 output_type) { struct dpu_hw_cdm *hw_cdm; struct dpu_hw_cdm_cfg *cdm_cfg; struct dpu_hw_pingpong *hw_pp; - struct dpu_encoder_phys_wb *wb_enc; - const struct msm_format *format; - const struct dpu_format *dpu_fmt; - struct drm_writeback_job *wb_job; int ret; if (!phys_enc) return; - wb_enc = to_dpu_encoder_phys_wb(phys_enc); - cdm_cfg = &wb_enc->cdm_cfg; + cdm_cfg = &phys_enc->cdm_cfg; hw_pp = phys_enc->hw_pp; hw_cdm = phys_enc->hw_cdm; - wb_job = wb_enc->wb_job; - - format = msm_framebuffer_format(wb_enc->wb_job->fb);- dpu_fmt = dpu_get_dpu_format_ext(format->pixel_format, wb_job->fb->modifier);if (!hw_cdm) return;@@ -306,10 +299,10 @@ static void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys *phys_enc)memset(cdm_cfg, 0, sizeof(struct dpu_hw_cdm_cfg)); - cdm_cfg->output_width = wb_job->fb->width; - cdm_cfg->output_height = wb_job->fb->height; + cdm_cfg->output_width = phys_enc->cached_mode.hdisplay; + cdm_cfg->output_height = phys_enc->cached_mode.vdisplay;This is a semantic change. Instead of passing the FB size, this passes the mode dimensions. They are not guaranteed to be the same, especially for the WB case.
The WB job is storing the output FB of WB. I cannot think of a use-case where this cannot match the current mode programmed to the WB encoder.
Yes, if it was the drm_plane's FB, then it cannot be guaranteed as the plane can scale the contents but here thats not the case. Here its the output FB of WB.
cdm_cfg->output_fmt = dpu_fmt; - cdm_cfg->output_type = CDM_CDWN_OUTPUT_WB; + cdm_cfg->output_type = output_type; cdm_cfg->output_bit_depth = DPU_FORMAT_IS_DX(dpu_fmt) ? CDM_CDWN_OUTPUT_10BIT : CDM_CDWN_OUTPUT_8BIT; cdm_cfg->csc_cfg = &dpu_csc10_rgb2yuv_601l; @@ -462,6 +455,14 @@ static void dpu_encoder_phys_wb_setup( struct dpu_hw_wb *hw_wb = phys_enc->hw_wb; struct drm_display_mode mode = phys_enc->cached_mode; struct drm_framebuffer *fb = NULL;+ struct dpu_encoder_phys_wb *wb_enc = to_dpu_encoder_phys_wb(phys_enc);+ struct drm_writeback_job *wb_job; + const struct msm_format *format; + const struct dpu_format *dpu_fmt; + + wb_job = wb_enc->wb_job; + format = msm_framebuffer_format(wb_enc->wb_job->fb);+ dpu_fmt = dpu_get_dpu_format_ext(format->pixel_format, wb_job->fb->modifier);DPU_DEBUG("[mode_set:%d, \"%s\",%d,%d]\n", hw_wb->idx - WB_0, mode.name, @@ -475,7 +476,7 @@ static void dpu_encoder_phys_wb_setup( dpu_encoder_phys_wb_setup_fb(phys_enc, fb); - dpu_encoder_helper_phys_setup_cdm(phys_enc);+ dpu_encoder_helper_phys_setup_cdm(phys_enc, dpu_fmt, CDM_CDWN_OUTPUT_WB);dpu_encoder_phys_wb_setup_ctl(phys_enc); }
