On 25/04/2022 03:32, Abhinav Kumar wrote:
Make changes to dpu_encoder to support virtual encoder needed to support writeback for dpu.
changes in v4:
- squash dpu_encoder pieces from [1]
[1] https://patchwork.freedesktop.org/patch/483099/?series=102964&rev=2
Signed-off-by: Abhinav Kumar quic_abhinavk@quicinc.com
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 94 +++++++++++++++++++----- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 7 ++ 2 files changed, 83 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 25c7eda..d1e92d89 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1013,9 +1013,18 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) phys->hw_intf = dpu_rm_get_intf(&dpu_kms->rm, phys->intf_idx);
if (!phys->hw_intf) {
if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX)phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx);if (!phys->hw_intf && !phys->hw_wb) { DPU_ERROR_ENC(dpu_enc,
"no intf block assigned at idx: %d\n", i);
"no intf or wb block assigned at idx: %d\n", i);return;}if (phys->hw_intf && phys->hw_wb) {DPU_ERROR_ENC(dpu_enc,"invalid phys both intf and wb block at idx: %d\n", i); return;@@ -1163,16 +1172,35 @@ static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg *catalog, { int i = 0;
- for (i = 0; i < catalog->intf_count; i++) {
if (catalog->intf[i].type == type&& catalog->intf[i].controller_id == controller_id) {return catalog->intf[i].id;
if (type != INTF_WB) {
for (i = 0; i < catalog->intf_count; i++) {if (catalog->intf[i].type == type&& catalog->intf[i].controller_id == controller_id) {return catalog->intf[i].id;}} }
return INTF_MAX; }
+static enum dpu_wb dpu_encoder_get_wb(struct dpu_mdss_cfg *catalog,
enum dpu_intf_type type, u32 controller_id)+{
- int i = 0;
- if (type != INTF_WB)
goto end;- for (i = 0; i < catalog->wb_count; i++) {
if (catalog->wb[i].id == controller_id)return catalog->wb[i].id;- }
+end:
- return WB_MAX;
+}
- static void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc, struct dpu_encoder_phys *phy_enc) {
@@ -1887,16 +1915,32 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
dpu_encoder_helper_reset_mixers(phys_enc);
- for (i = 0; i < dpu_enc->num_phys_encs; i++) {
if (dpu_enc->phys_encs[i] && phys_enc->hw_intf->ops.bind_pingpong_blk)phys_enc->hw_intf->ops.bind_pingpong_blk(dpu_enc->phys_encs[i]->hw_intf, false,dpu_enc->phys_encs[i]->hw_pp->idx);/* mark INTF flush as pending */if (phys_enc->hw_ctl->ops.update_pending_flush_intf)phys_enc->hw_ctl->ops.update_pending_flush_intf(phys_enc->hw_ctl,dpu_enc->phys_encs[i]->hw_intf->idx);
/*
* TODO: move the once-only operation like CTL flush/trigger* into dpu_encoder_virt_disable() and all operations which need* to be done per phys encoder into the phys_disable() op.*/if (phys_enc->hw_wb) {
/* disable the PP block */if (phys_enc->hw_wb->ops.bind_pingpong_blk)phys_enc->hw_wb->ops.bind_pingpong_blk(phys_enc->hw_wb, false,phys_enc->hw_pp->idx);/* mark WB flush as pending */if (phys_enc->hw_ctl->ops.update_pending_flush_wb)phys_enc->hw_ctl->ops.update_pending_flush_wb(ctl, phys_enc->hw_wb->idx);} else {
for (i = 0; i < dpu_enc->num_phys_encs; i++) {if (dpu_enc->phys_encs[i] && phys_enc->hw_intf->ops.bind_pingpong_blk)phys_enc->hw_intf->ops.bind_pingpong_blk(dpu_enc->phys_encs[i]->hw_intf, false,dpu_enc->phys_encs[i]->hw_pp->idx);/* mark INTF flush as pending */if (phys_enc->hw_ctl->ops.update_pending_flush_intf)phys_enc->hw_ctl->ops.update_pending_flush_intf(phys_enc->hw_ctl,dpu_enc->phys_encs[i]->hw_intf->idx);}}
/* reset the merge 3D HW block */
@@ -2112,6 +2156,9 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, case DRM_MODE_ENCODER_TMDS: intf_type = INTF_DP; break;
case DRM_MODE_ENCODER_VIRTUAL:
intf_type = INTF_WB;break;}
WARN_ON(disp_info->num_of_h_tiles < 1);
@@ -2149,8 +2196,19 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, phys_params.intf_idx = dpu_encoder_get_intf(dpu_kms->catalog, intf_type, controller_id);
if (phys_params.intf_idx == INTF_MAX) {DPU_ERROR_ENC(dpu_enc, "could not get intf: type %d, id %d\n",
phys_params.wb_idx = dpu_encoder_get_wb(dpu_kms->catalog,intf_type, controller_id);/** For boards which have no physical displays, having no interface* is fine because it can still be used with just writeback.* If we try without a display on a board which uses a DPU in which* writeback is not supported, then this will still fail as it will not* find any writeback in the catalog.*/if ((phys_params.intf_idx == INTF_MAX) &&(phys_params.wb_idx == WB_MAX)) {DPU_ERROR_ENC(dpu_enc, "could not get intf or wb: type %d, id %d\n",
I've commented the previous iteration by the mistake, but the comment still applies to this version too. Let's repeat it, so that we have all the comments in the single version.
I think the comment is misleading. It is a phys_params, so it must have etiher intf_idx or wb_idx, but not both of them, despite the board having the interfaces or WB.
So somthing like this might be more appropriate:
/* * The phys_params might represent either an INTF or a WB unit, but not * both of them at the same time. */
if ((phys_params.intf_idx == INTF_MAX) && (phys_params.wb_idx == WB_MAX)) { .... // error out }
if ((phys_params.intf_idx != INTF_MAX) && (phys_params.wb_idx == WB_MAX)) { ........ // error out }
Looks good otherwise.
intf_type, controller_id); ret = -EINVAL; }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 544a9a4..c84b8e8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -11,6 +11,7 @@
#include "dpu_kms.h" #include "dpu_hw_intf.h" +#include "dpu_hw_wb.h" #include "dpu_hw_pingpong.h" #include "dpu_hw_ctl.h" #include "dpu_hw_top.h" @@ -165,12 +166,14 @@ enum dpu_intr_idx {
- @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
- @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
- @split_role: Role to play in a split-panel configuration
- @intf_mode: Interface mode
- @intf_idx: Interface index on dpu hardware
- @wb_idx: Writeback index on dpu hardware
- @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes
- @enable_state: Enable state tracking
- @vblank_refcount: Reference count of vblank request
@@ -193,11 +196,13 @@ struct dpu_encoder_phys { struct dpu_hw_ctl *hw_ctl; struct dpu_hw_pingpong *hw_pp; struct dpu_hw_intf *hw_intf;
- struct dpu_hw_wb *hw_wb; struct dpu_kms *dpu_kms; struct drm_display_mode cached_mode; enum dpu_enc_split_role split_role; enum dpu_intf_mode intf_mode; enum dpu_intf intf_idx;
- enum dpu_wb wb_idx; spinlock_t *enc_spinlock; enum dpu_enc_enable_state enable_state; atomic_t vblank_refcount;
@@ -243,6 +248,7 @@ struct dpu_encoder_phys_cmd {
- @parent_ops: Callbacks exposed by the parent to the phys_enc
- @split_role: Role to play in a split-panel configuration
- @intf_idx: Interface index this phys_enc will control
*/ struct dpu_enc_phys_init_params {
- @wb_idx: Writeback index this phys_enc will control
- @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes
@@ -251,6 +257,7 @@ struct dpu_enc_phys_init_params { const struct dpu_encoder_virt_ops *parent_ops; enum dpu_enc_split_role split_role; enum dpu_intf intf_idx;
- enum dpu_wb wb_idx; spinlock_t *enc_spinlock; };