[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <bb4d24c7-71d4-25ce-886f-81bc77adf67c@quicinc.com>
Date: Thu, 30 Nov 2023 16:45:27 -0800
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: <freedreno@...ts.freedesktop.org>, Rob Clark <robdclark@...il.com>,
"Sean Paul" <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
"David Airlie" <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
<dri-devel@...ts.freedesktop.org>, <quic_jesszhan@...cinc.com>,
<quic_parellan@...cinc.com>, <quic_khsieh@...cinc.com>,
<linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 13/16] drm/msm/dpu: reserve cdm blocks for writeback in
case of YUV output
On 8/30/2023 5:23 PM, Dmitry Baryshkov wrote:
> On Thu, 31 Aug 2023 at 01:50, Abhinav Kumar <quic_abhinavk@...cinc.com> 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.
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 35 ++++++++++++++++++++-
>> 1 file changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index 1b1e07292a9e..7a3d179bdfba 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"
>> @@ -615,9 +616,11 @@ 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;
>> + bool needs_cdm = false;
>>
>> if (!drm_enc || !crtc_state || !conn_state) {
>> DPU_ERROR("invalid arg(s), drm_enc %d, crtc/conn state %d/%d\n",
>> @@ -655,6 +658,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))))
>> + needs_cdm = true;
>> + if (needs_cdm && !dpu_enc->cur_master->hw_cdm)
>> + crtc_state->mode_changed = true;
>> + else if (!needs_cdm && dpu_enc->cur_master->hw_cdm)
>> + crtc_state->mode_changed = true;
>> + }
>
> What would be the (estimated) check for DP?
>
Originally we were planning a lot more but now we are going to start
with the mode being drm_mode_is_420_only and use CDM for that.
>> +
>> /*
>> * Release and Allocate resources on every modeset
>> * Dont allocate when active is false.
>> @@ -664,7 +683,7 @@ static int dpu_encoder_virt_atomic_check(
>>
>> if (!crtc_state->active_changed || crtc_state->enable)
>> ret = dpu_rm_reserve(&dpu_kms->rm, global_state,
>> - drm_enc, crtc_state, topology, false);
>> + drm_enc, crtc_state, topology, needs_cdm);
>> }
>>
>> trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags);
>> @@ -1126,6 +1145,20 @@ 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;
>> + struct drm_framebuffer *fb;
>> +
>> + fb = conn_state->writeback_job->fb;
>> +
>> + if (fb && DPU_FORMAT_IS_YUV(to_dpu_format(msm_framebuffer_format(fb)))) {
>
> You can drop all fb-related conditions here. If we have CDM, we know
> that we asked for it. If we do not, it's because we do not need it.
>
hmmm .... let me do some testing with this and if it works I will make
this change as well. Thanks.
>> + 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
>>
>
>
Powered by blists - more mailing lists