On Fri, 8 Dec 2023 at 18:34, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
>
>
>
> On 12/8/2023 3:54 AM, Dmitry Baryshkov wrote:
> > On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
> >>
> >> Reserve CDM blocks for writeback if the format of the output fb
> >> is YUV. At the moment, the reservation is done only for writeback
> >> but can easily be extended by relaxing the checks once other
> >> interfaces are ready to output YUV.
> >>
> >> changes in v2:
> >> - use needs_cdm from topology struct
> >> - drop fb related checks from atomic_mode_set()
> >
> > It looks like this should be squashed with the patch 11. The 'unbind
> > CDM' doesn't really make sense without this patch. We need to allocate
> > it first, before touching it.
> >
>
> The way I was thinking was that patch just completes the
> dpu_encoder_phys_cleanup() and yes it was intentionally kept ahead
> because that will not kick in till hw_cdm is assigned.
>
> Then, this patch only handles reserving/assignment of hw_cdm when needed.
>
> That was the motivation behind this split.
It leaves a leaf code that is not used at all. There is no need to
cleanup anything if it was not allocated. Please remove the split and
squash it with allocation.
>
> >>
> >> Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 +++++++++++++++++++++
> >> 1 file changed, 27 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> index 862912727925..a576e3e62429 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> @@ -16,6 +16,7 @@
> >> #include <drm/drm_crtc.h>
> >> #include <drm/drm_file.h>
> >> #include <drm/drm_probe_helper.h>
> >> +#include <drm/drm_framebuffer.h>
> >>
> >> #include "msm_drv.h"
> >> #include "dpu_kms.h"
> >> @@ -583,6 +584,7 @@ static int dpu_encoder_virt_atomic_check(
> >> struct drm_display_mode *adj_mode;
> >> struct msm_display_topology topology;
> >> struct dpu_global_state *global_state;
> >> + struct drm_framebuffer *fb;
> >> struct drm_dsc_config *dsc;
> >> int i = 0;
> >> int ret = 0;
> >> @@ -623,6 +625,22 @@ static int dpu_encoder_virt_atomic_check(
> >>
> >> topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, dsc);
> >>
> >> + /*
> >> + * Use CDM only for writeback at the moment as other interfaces cannot handle it.
> >> + * if writeback itself cannot handle cdm for some reason it will fail in its atomic_check()
> >> + * earlier.
> >> + */
> >> + if (dpu_enc->disp_info.intf_type == INTF_WB && conn_state->writeback_job) {
> >> + fb = conn_state->writeback_job->fb;
> >> +
> >> + if (fb && DPU_FORMAT_IS_YUV(to_dpu_format(msm_framebuffer_format(fb))))
> >> + topology.needs_cdm = true;
> >> + if (topology.needs_cdm && !dpu_enc->cur_master->hw_cdm)
> >> + crtc_state->mode_changed = true;
> >> + else if (!topology.needs_cdm && dpu_enc->cur_master->hw_cdm)
> >> + crtc_state->mode_changed = true;
> >> + }
> >> +
> >> /*
> >> * Release and Allocate resources on every modeset
> >> * Dont allocate when active is false.
> >> @@ -1063,6 +1081,15 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> >>
> >> dpu_enc->dsc_mask = dsc_mask;
> >>
> >> + if (dpu_enc->disp_info.intf_type == INTF_WB && conn_state->writeback_job) {
> >> + struct dpu_hw_blk *hw_cdm = NULL;
> >> +
> >> + dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> >> + drm_enc->base.id, DPU_HW_BLK_CDM,
> >> + &hw_cdm, 1);
> >> + dpu_enc->cur_master->hw_cdm = hw_cdm ? to_dpu_hw_cdm(hw_cdm) : NULL;
> >> + }
> >> +
> >> cstate = to_dpu_crtc_state(crtc_state);
> >>
> >> for (i = 0; i < num_lm; i++) {
> >> --
> >> 2.40.1
> >>
> >
> >
> > --
> > With best wishes
> >
> > Dmitry
--
With best wishes
Dmitry
