Messages in this thread | | | Date | Sat, 19 Feb 2022 00:21:20 +0300 | Subject | Re: [REPOST PATCH v4 08/13] drm/msm/disp/dpu1: Don't use DSC with mode_3d | From | Dmitry Baryshkov <> |
| |
On 18/02/2022 23:46, Abhinav Kumar wrote: > > > On 2/16/2022 11:12 PM, Dmitry Baryshkov wrote: >> On 17/02/2022 09:33, Abhinav Kumar wrote: >>> >>> >>> On 2/16/2022 10:10 PM, Vinod Koul wrote: >>>> On 16-02-22, 19:11, Abhinav Kumar wrote: >>>>> >>>>> >>>>> On 2/10/2022 2:34 AM, Vinod Koul wrote: >>>>>> We cannot enable mode_3d when we are using the DSC. So pass >>>>>> configuration to detect DSC is enabled and not enable mode_3d >>>>>> when we are using DSC >>>>>> >>>>>> We add a helper dpu_encoder_helper_get_dsc() to detect dsc >>>>>> enabled and pass this to .setup_intf_cfg() >>>>>> >>>>>> Signed-off-by: Vinod Koul <vkoul@kernel.org> >>>>> >>>>> We should not use 3D mux only when we use DSC merge topology. >>>>> I agree that today we use only 2-2-1 topology for DSC which means >>>>> its using >>>>> DSC merge. >>>>> >>>>> But generalizing that 3D mux should not be used for DSC is not right. >>>>> >>>>> You can detect DSC merge by checking if there are two encoders and one >>>>> interface in the topology and if so, you can disable 3D mux. >>>> >>>> Right now with DSC we disable that as suggested by Dmitry last time. >>>> Whenever we introduce merge we should revisit this, for now this should >>>> suffice >>>> >>> >>> Sorry I didnt follow. >>> >>> The topology which you are supporting today IS DSC merge 2-2-1. I >>> didnt get what you mean by "whenever we introduce". >>> >>> I didnt follow Dmitry's comment either. >>> >>> "anybody adding support for SDE_RM_TOPOLOGY_DUALPIPE_3DMERGE_DSC >>> handle this." >>> >>> 3D mux shouldnt be used when DSC merge is used. >>> >>> The topology Dmitry is referring to will not use DSC merge but you >>> are using it here and thats why you had to make this patch in the >>> first place. So I am not sure why would someone who uses 3D merge >>> topology worry about DSC merge. Your patch is the one which deals >>> with the topology in question. >>> >>> What I am suggesting is a small but necessary improvement to this patch. >> >> It seems that we can replace this patch by changing >> dpu_encoder_helper_get_3d_blend_mode() to contain the following >> condition (instead of the one present there). Does the following seem >> correct to you: >> >> static inline enum dpu_3d_blend_mode >> dpu_encoder_helper_get_3d_blend_mode( >> struct dpu_encoder_phys *phys_enc) >> { >> struct dpu_crtc_state *dpu_cstate; >> >> if (!phys_enc || phys_enc->enable_state == DPU_ENC_DISABLING) >> return BLEND_3D_NONE; >> >> dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state); >> >> + /* Use merge_3d unless DSCMERGE topology is used */ >> if (phys_enc->split_role == ENC_ROLE_SOLO && >> + hweight(dpu_encoder_helper_get_dsc(phys_enc)) != 1 &&
Yes, the correct should be: hweight(...) == 2
>> dpu_cstate->num_mixers == CRTC_DUAL_MIXERS) >> return BLEND_3D_H_ROW_INT; >> >> return BLEND_3D_NONE; >> } > > This will not be enough. To detect whether DSC merge is enabled you need > to query the topology. The above condition only checks if DSC is enabled > not DSC merge. > > So the above function can be modified to use a helper like below instead > of the hweight. > > bool dpu_encoder_get_dsc_merge_info(struct dpu_encoder_virt *dpu_enc) > { > struct msm_display_topology topology = {0}; > > topology = dpu_encoder_get_topology(...); > > if (topology.num_dsc > topology.num_intf)
num_intf is 1 or 2. If it's one, the split_role is SOLO hweight would return a num of bits in the DSC mask. It's 0, 1 or 2. So, if the split_role is SOLO and hweight is 2, we get exactly your condition.
Does that sound correct?
> return true; > else > return false; > } > > if (!dpu_encoder_get_dsc_merge_info() && other conditions listed above) > return BLEND_3D_H_ROW_INT; > else > BLEND_3D_NONE; >> >> >>> >>> All that you have to do is in query whether DSC merge is used from >>> the topology. You can do it in multiple ways: >>> >>> 1) Either query this from the encoder >>> 2) Store a bool "dsc_merge" in the intf_cfg >>> >> >>
-- With best wishes Dmitry
| |