| Message ID | 1655235140-16424-1-git-send-email-quic_abhinavk@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display() | expand |
On 14/06/2022 22:32, Abhinav Kumar wrote: > intf and wb resources are not dependent on the rm global > state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). > > Move the allocation of intf and wb resources to dpu_encoder_setup_display() > so that we can utilize the hw caps even during atomic_check() phase. > > Since dpu_encoder_setup_display() already has protection against > setting invalid intf_idx and wb_idx, these checks can now > be dropped as well. > > Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++++++------------------ > 1 file changed, 7 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 3a462e327e0e..e991d4ba8a40 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, > phys->hw_pp = dpu_enc->hw_pp[i]; > phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); > > - 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->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 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; > - } Please retain these checks in dpu_encoder_setup_display(). It checks that we really have got the intf or wb. For example one might have specified the INTF that leads to INTF_NONE interface. Or non-existing/not supported WB. > - > phys->cached_mode = crtc_state->adjusted_mode; > if (phys->ops.atomic_mode_set) > phys->ops.atomic_mode_set(phys, crtc_state, conn_state); > @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, > struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; > atomic_set(&phys->vsync_cnt, 0); > atomic_set(&phys->underrun_cnt, 0); > + > + 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->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) > + phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); > } > + > mutex_unlock(&dpu_enc->enc_lock); > > return ret;
On 14/06/2022 22:32, Abhinav Kumar wrote: > intf and wb resources are not dependent on the rm global > state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). > > Move the allocation of intf and wb resources to dpu_encoder_setup_display() > so that we can utilize the hw caps even during atomic_check() phase. > > Since dpu_encoder_setup_display() already has protection against > setting invalid intf_idx and wb_idx, these checks can now > be dropped as well. > I'm going to pick up the last two patches, so you'd have to resend just the first one. > Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++++++------------------ > 1 file changed, 7 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 3a462e327e0e..e991d4ba8a40 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, > phys->hw_pp = dpu_enc->hw_pp[i]; > phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); > > - 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->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 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; > - } > - > phys->cached_mode = crtc_state->adjusted_mode; > if (phys->ops.atomic_mode_set) > phys->ops.atomic_mode_set(phys, crtc_state, conn_state); > @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, > struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; > atomic_set(&phys->vsync_cnt, 0); > atomic_set(&phys->underrun_cnt, 0); > + > + 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->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) > + phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); > } > + > mutex_unlock(&dpu_enc->enc_lock); > > return ret;
On 6/15/2022 5:36 AM, Dmitry Baryshkov wrote: > On 14/06/2022 22:32, Abhinav Kumar wrote: >> intf and wb resources are not dependent on the rm global >> state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). >> >> Move the allocation of intf and wb resources to >> dpu_encoder_setup_display() >> so that we can utilize the hw caps even during atomic_check() phase. >> >> Since dpu_encoder_setup_display() already has protection against >> setting invalid intf_idx and wb_idx, these checks can now >> be dropped as well. >> >> Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual >> encoder") >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 >> +++++++------------------ >> 1 file changed, 7 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 3a462e327e0e..e991d4ba8a40 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> @@ -1048,24 +1048,6 @@ static void >> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, >> phys->hw_pp = dpu_enc->hw_pp[i]; >> phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); >> - 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->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 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; >> - } > > Please retain these checks in dpu_encoder_setup_display(). > It checks that we really have got the intf or wb. For example one might > have specified the INTF that leads to INTF_NONE interface. Or > non-existing/not supported WB. Right, so the reason I omitted that was dpu_encoder_setup_display() already has these checks: https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2273 Please check lines 2273-2284. Only if all those checks succeeded we call dpu_encoder_virt_add_phys_encs which increments num_phys_encs. Thats why I dropped those. Let me know if you have more questions. > >> - >> phys->cached_mode = crtc_state->adjusted_mode; >> if (phys->ops.atomic_mode_set) >> phys->ops.atomic_mode_set(phys, crtc_state, conn_state); >> @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct >> dpu_encoder_virt *dpu_enc, >> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; >> atomic_set(&phys->vsync_cnt, 0); >> atomic_set(&phys->underrun_cnt, 0); >> + >> + 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->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) >> + phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); >> } >> + >> mutex_unlock(&dpu_enc->enc_lock); >> return ret; > >
On 15/06/2022 19:40, Abhinav Kumar wrote: > > > On 6/15/2022 5:36 AM, Dmitry Baryshkov wrote: >> On 14/06/2022 22:32, Abhinav Kumar wrote: >>> intf and wb resources are not dependent on the rm global >>> state so need not be allocated during >>> dpu_encoder_virt_atomic_mode_set(). >>> >>> Move the allocation of intf and wb resources to >>> dpu_encoder_setup_display() >>> so that we can utilize the hw caps even during atomic_check() phase. >>> >>> Since dpu_encoder_setup_display() already has protection against >>> setting invalid intf_idx and wb_idx, these checks can now >>> be dropped as well. >>> >>> Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual >>> encoder") >>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>> --- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 >>> +++++++------------------ >>> 1 file changed, 7 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 3a462e327e0e..e991d4ba8a40 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> @@ -1048,24 +1048,6 @@ static void >>> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, >>> phys->hw_pp = dpu_enc->hw_pp[i]; >>> phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); >>> - 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->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 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; >>> - } >> >> Please retain these checks in dpu_encoder_setup_display(). >> It checks that we really have got the intf or wb. For example one >> might have specified the INTF that leads to INTF_NONE interface. Or >> non-existing/not supported WB. > > Right, so the reason I omitted that was dpu_encoder_setup_display() > already has these checks: > > https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2273 > > > Please check lines 2273-2284. > > Only if all those checks succeeded we call > dpu_encoder_virt_add_phys_encs which increments num_phys_encs. As I wrote, it checks indices from phys_params, but not the acquired hardware instances. > > Thats why I dropped those. > > Let me know if you have more questions. > >> >>> - >>> phys->cached_mode = crtc_state->adjusted_mode; >>> if (phys->ops.atomic_mode_set) >>> phys->ops.atomic_mode_set(phys, crtc_state, conn_state); >>> @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct >>> dpu_encoder_virt *dpu_enc, >>> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; >>> atomic_set(&phys->vsync_cnt, 0); >>> atomic_set(&phys->underrun_cnt, 0); >>> + >>> + 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->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) >>> + phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); >>> } >>> + >>> mutex_unlock(&dpu_enc->enc_lock); >>> return ret; >> >>
On 6/15/2022 10:04 AM, Dmitry Baryshkov wrote: > On 15/06/2022 19:40, Abhinav Kumar wrote: >> >> >> On 6/15/2022 5:36 AM, Dmitry Baryshkov wrote: >>> On 14/06/2022 22:32, Abhinav Kumar wrote: >>>> intf and wb resources are not dependent on the rm global >>>> state so need not be allocated during >>>> dpu_encoder_virt_atomic_mode_set(). >>>> >>>> Move the allocation of intf and wb resources to >>>> dpu_encoder_setup_display() >>>> so that we can utilize the hw caps even during atomic_check() phase. >>>> >>>> Since dpu_encoder_setup_display() already has protection against >>>> setting invalid intf_idx and wb_idx, these checks can now >>>> be dropped as well. >>>> >>>> Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual >>>> encoder") >>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>>> --- >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 >>>> +++++++------------------ >>>> 1 file changed, 7 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 3a462e327e0e..e991d4ba8a40 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>> @@ -1048,24 +1048,6 @@ static void >>>> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, >>>> phys->hw_pp = dpu_enc->hw_pp[i]; >>>> phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); >>>> - 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->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 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; >>>> - } >>> >>> Please retain these checks in dpu_encoder_setup_display(). >>> It checks that we really have got the intf or wb. For example one >>> might have specified the INTF that leads to INTF_NONE interface. Or >>> non-existing/not supported WB. >> >> Right, so the reason I omitted that was dpu_encoder_setup_display() >> already has these checks: >> >> https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2273 >> >> >> Please check lines 2273-2284. >> >> Only if all those checks succeeded we call >> dpu_encoder_virt_add_phys_encs which increments num_phys_encs. > > As I wrote, it checks indices from phys_params, but not the acquired > hardware instances. Right but today, both the get_intf() and get_wb() just return the intf/wb corresponding to the index. So as long as the index is valid how will checking hw_wb or hw_intf be different? static inline struct dpu_hw_intf *dpu_rm_get_intf(struct dpu_rm *rm, enum dpu_intf intf_idx) { return rm->hw_intf[intf_idx - INTF_0]; } /** * dpu_rm_get_wb - Return a struct dpu_hw_wb instance given it's index. * @rm: DPU Resource Manager handle * @wb_idx: WB index */ static inline struct dpu_hw_wb *dpu_rm_get_wb(struct dpu_rm *rm, enum dpu_wb wb_idx) { return rm->hw_wb[wb_idx - WB_0]; } > >> >> Thats why I dropped those. >> >> Let me know if you have more questions. >> >>> >>>> - >>>> phys->cached_mode = crtc_state->adjusted_mode; >>>> if (phys->ops.atomic_mode_set) >>>> phys->ops.atomic_mode_set(phys, crtc_state, conn_state); >>>> @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct >>>> dpu_encoder_virt *dpu_enc, >>>> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; >>>> atomic_set(&phys->vsync_cnt, 0); >>>> atomic_set(&phys->underrun_cnt, 0); >>>> + >>>> + 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->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) >>>> + phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); >>>> } >>>> + >>>> mutex_unlock(&dpu_enc->enc_lock); >>>> return ret; >>> >>> > >
On 15/06/2022 20:11, Abhinav Kumar wrote: > > > On 6/15/2022 10:04 AM, Dmitry Baryshkov wrote: >> On 15/06/2022 19:40, Abhinav Kumar wrote: >>> >>> >>> On 6/15/2022 5:36 AM, Dmitry Baryshkov wrote: >>>> On 14/06/2022 22:32, Abhinav Kumar wrote: >>>>> intf and wb resources are not dependent on the rm global >>>>> state so need not be allocated during >>>>> dpu_encoder_virt_atomic_mode_set(). >>>>> >>>>> Move the allocation of intf and wb resources to >>>>> dpu_encoder_setup_display() >>>>> so that we can utilize the hw caps even during atomic_check() phase. >>>>> >>>>> Since dpu_encoder_setup_display() already has protection against >>>>> setting invalid intf_idx and wb_idx, these checks can now >>>>> be dropped as well. >>>>> >>>>> Fixes: e02a559a720f ("make changes to dpu_encoder to support >>>>> virtual encoder") >>>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>>>> --- >>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 >>>>> +++++++------------------ >>>>> 1 file changed, 7 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 3a462e327e0e..e991d4ba8a40 100644 >>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>>> @@ -1048,24 +1048,6 @@ static void >>>>> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, >>>>> phys->hw_pp = dpu_enc->hw_pp[i]; >>>>> phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); >>>>> - 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->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 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; >>>>> - } >>>> >>>> Please retain these checks in dpu_encoder_setup_display(). >>>> It checks that we really have got the intf or wb. For example one >>>> might have specified the INTF that leads to INTF_NONE interface. Or >>>> non-existing/not supported WB. >>> >>> Right, so the reason I omitted that was dpu_encoder_setup_display() >>> already has these checks: >>> >>> https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2273 >>> >>> >>> Please check lines 2273-2284. >>> >>> Only if all those checks succeeded we call >>> dpu_encoder_virt_add_phys_encs which increments num_phys_encs. >> >> As I wrote, it checks indices from phys_params, but not the acquired >> hardware instances. > > Right but today, both the get_intf() and get_wb() just return the > intf/wb corresponding to the index. So as long as the index is valid how > will checking hw_wb or hw_intf be different? > > static inline struct dpu_hw_intf *dpu_rm_get_intf(struct dpu_rm *rm, > enum dpu_intf intf_idx) > { > return rm->hw_intf[intf_idx - INTF_0]; > } > > /** > * dpu_rm_get_wb - Return a struct dpu_hw_wb instance given it's index. > * @rm: DPU Resource Manager handle > * @wb_idx: WB index > */ > static inline struct dpu_hw_wb *dpu_rm_get_wb(struct dpu_rm *rm, enum > dpu_wb wb_idx) > { > return rm->hw_wb[wb_idx - WB_0]; > } WB_0 is valid, but dpu_rm_get_wb(WB_0) will return NULL. INTF_0 is valid, but dpu_rm_get_intf(INTF_0) on qcm2290 will return NULL. Etc. >> >>> >>> Thats why I dropped those. >>> >>> Let me know if you have more questions. >>> >>>> >>>>> - >>>>> phys->cached_mode = crtc_state->adjusted_mode; >>>>> if (phys->ops.atomic_mode_set) >>>>> phys->ops.atomic_mode_set(phys, crtc_state, conn_state); >>>>> @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct >>>>> dpu_encoder_virt *dpu_enc, >>>>> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; >>>>> atomic_set(&phys->vsync_cnt, 0); >>>>> atomic_set(&phys->underrun_cnt, 0); >>>>> + >>>>> + 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->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) >>>>> + phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); >>>>> } >>>>> + >>>>> mutex_unlock(&dpu_enc->enc_lock); >>>>> return ret; >>>> >>>> >> >>
On 6/15/2022 11:34 AM, Dmitry Baryshkov wrote: > On 15/06/2022 20:11, Abhinav Kumar wrote: >> >> >> On 6/15/2022 10:04 AM, Dmitry Baryshkov wrote: >>> On 15/06/2022 19:40, Abhinav Kumar wrote: >>>> >>>> >>>> On 6/15/2022 5:36 AM, Dmitry Baryshkov wrote: >>>>> On 14/06/2022 22:32, Abhinav Kumar wrote: >>>>>> intf and wb resources are not dependent on the rm global >>>>>> state so need not be allocated during >>>>>> dpu_encoder_virt_atomic_mode_set(). >>>>>> >>>>>> Move the allocation of intf and wb resources to >>>>>> dpu_encoder_setup_display() >>>>>> so that we can utilize the hw caps even during atomic_check() phase. >>>>>> >>>>>> Since dpu_encoder_setup_display() already has protection against >>>>>> setting invalid intf_idx and wb_idx, these checks can now >>>>>> be dropped as well. >>>>>> >>>>>> Fixes: e02a559a720f ("make changes to dpu_encoder to support >>>>>> virtual encoder") >>>>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>>>>> --- >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 >>>>>> +++++++------------------ >>>>>> 1 file changed, 7 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 3a462e327e0e..e991d4ba8a40 100644 >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>>>>> @@ -1048,24 +1048,6 @@ static void >>>>>> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, >>>>>> phys->hw_pp = dpu_enc->hw_pp[i]; >>>>>> phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); >>>>>> - 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->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 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; >>>>>> - } >>>>> >>>>> Please retain these checks in dpu_encoder_setup_display(). >>>>> It checks that we really have got the intf or wb. For example one >>>>> might have specified the INTF that leads to INTF_NONE interface. Or >>>>> non-existing/not supported WB. >>>> >>>> Right, so the reason I omitted that was dpu_encoder_setup_display() >>>> already has these checks: >>>> >>>> https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2273 >>>> >>>> >>>> Please check lines 2273-2284. >>>> >>>> Only if all those checks succeeded we call >>>> dpu_encoder_virt_add_phys_encs which increments num_phys_encs. >>> >>> As I wrote, it checks indices from phys_params, but not the acquired >>> hardware instances. >> >> Right but today, both the get_intf() and get_wb() just return the >> intf/wb corresponding to the index. So as long as the index is valid >> how will checking hw_wb or hw_intf be different? >> >> static inline struct dpu_hw_intf *dpu_rm_get_intf(struct dpu_rm *rm, >> enum dpu_intf intf_idx) >> { >> return rm->hw_intf[intf_idx - INTF_0]; >> } >> >> /** >> * dpu_rm_get_wb - Return a struct dpu_hw_wb instance given it's index. >> * @rm: DPU Resource Manager handle >> * @wb_idx: WB index >> */ >> static inline struct dpu_hw_wb *dpu_rm_get_wb(struct dpu_rm *rm, enum >> dpu_wb wb_idx) >> { >> return rm->hw_wb[wb_idx - WB_0]; >> } > > WB_0 is valid, but dpu_rm_get_wb(WB_0) will return NULL. > INTF_0 is valid, but dpu_rm_get_intf(INTF_0) on qcm2290 will return NULL. > > Etc. Ah okay got it, let me add them back in v2. > >>> >>>> >>>> Thats why I dropped those. >>>> >>>> Let me know if you have more questions. >>>> >>>>> >>>>>> - >>>>>> phys->cached_mode = crtc_state->adjusted_mode; >>>>>> if (phys->ops.atomic_mode_set) >>>>>> phys->ops.atomic_mode_set(phys, crtc_state, >>>>>> conn_state); >>>>>> @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct >>>>>> dpu_encoder_virt *dpu_enc, >>>>>> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; >>>>>> atomic_set(&phys->vsync_cnt, 0); >>>>>> atomic_set(&phys->underrun_cnt, 0); >>>>>> + >>>>>> + 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->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) >>>>>> + phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); >>>>>> } >>>>>> + >>>>>> mutex_unlock(&dpu_enc->enc_lock); >>>>>> return ret; >>>>> >>>>> >>> >>> > >
On 14/06/2022 22:32, Abhinav Kumar wrote: > intf and wb resources are not dependent on the rm global > state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). > > Move the allocation of intf and wb resources to dpu_encoder_setup_display() > so that we can utilize the hw caps even during atomic_check() phase. > > Since dpu_encoder_setup_display() already has protection against > setting invalid intf_idx and wb_idx, these checks can now > be dropped as well. > > Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") Please adjust the Fixes tags in all three commits. I didn't notice this beforehand and Stephen has complained. > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++++++------------------ > 1 file changed, 7 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 3a462e327e0e..e991d4ba8a40 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, > phys->hw_pp = dpu_enc->hw_pp[i]; > phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); > > - 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->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 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; > - } > - > phys->cached_mode = crtc_state->adjusted_mode; > if (phys->ops.atomic_mode_set) > phys->ops.atomic_mode_set(phys, crtc_state, conn_state); > @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, > struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; > atomic_set(&phys->vsync_cnt, 0); > atomic_set(&phys->underrun_cnt, 0); > + > + 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->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) > + phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); > } > + > mutex_unlock(&dpu_enc->enc_lock); > > return ret;
Hi Dmitry On 6/15/2022 10:55 PM, Dmitry Baryshkov wrote: > On 14/06/2022 22:32, Abhinav Kumar wrote: >> intf and wb resources are not dependent on the rm global >> state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). >> >> Move the allocation of intf and wb resources to >> dpu_encoder_setup_display() >> so that we can utilize the hw caps even during atomic_check() phase. >> >> Since dpu_encoder_setup_display() already has protection against >> setting invalid intf_idx and wb_idx, these checks can now >> be dropped as well. >> >> Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual >> encoder") > > Please adjust the Fixes tags in all three commits. I didn't notice this > beforehand and Stephen has complained. > Is something wrong with the tag? Format and hash looked right to me. >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 >> +++++++------------------ >> 1 file changed, 7 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 3a462e327e0e..e991d4ba8a40 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> @@ -1048,24 +1048,6 @@ static void >> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, >> phys->hw_pp = dpu_enc->hw_pp[i]; >> phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); >> - 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->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 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; >> - } >> - >> phys->cached_mode = crtc_state->adjusted_mode; >> if (phys->ops.atomic_mode_set) >> phys->ops.atomic_mode_set(phys, crtc_state, conn_state); >> @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct >> dpu_encoder_virt *dpu_enc, >> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; >> atomic_set(&phys->vsync_cnt, 0); >> atomic_set(&phys->underrun_cnt, 0); >> + >> + 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->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) >> + phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); >> } >> + >> mutex_unlock(&dpu_enc->enc_lock); >> return ret; > >
Quoting Abhinav Kumar (2022-06-15 22:59:25) > Hi Dmitry > > On 6/15/2022 10:55 PM, Dmitry Baryshkov wrote: > > On 14/06/2022 22:32, Abhinav Kumar wrote: > >> intf and wb resources are not dependent on the rm global > >> state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). > >> > >> Move the allocation of intf and wb resources to > >> dpu_encoder_setup_display() > >> so that we can utilize the hw caps even during atomic_check() phase. > >> > >> Since dpu_encoder_setup_display() already has protection against > >> setting invalid intf_idx and wb_idx, these checks can now > >> be dropped as well. > >> > >> Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual > >> encoder") > > > > Please adjust the Fixes tags in all three commits. I didn't notice this > > beforehand and Stephen has complained. I think Stephen is Stephen Rothwell. > > > Is something wrong with the tag? Format and hash looked right to me. $ git config pretty.fixes Fixes: %h ("%s") $ git help fixes 'fixes' is aliased to 'show -s --pretty=fixes' $ git fixes e02a559a720f Fixes: e02a559a720f ("drm/msm/dpu: make changes to dpu_encoder to support virtual encoder") it's missing the drm/msm/dpu prefix.
On Thu, 16 Jun 2022 at 09:18, Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Abhinav Kumar (2022-06-15 22:59:25) > > Hi Dmitry > > > > On 6/15/2022 10:55 PM, Dmitry Baryshkov wrote: > > > On 14/06/2022 22:32, Abhinav Kumar wrote: > > >> intf and wb resources are not dependent on the rm global > > >> state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). > > >> > > >> Move the allocation of intf and wb resources to > > >> dpu_encoder_setup_display() > > >> so that we can utilize the hw caps even during atomic_check() phase. > > >> > > >> Since dpu_encoder_setup_display() already has protection against > > >> setting invalid intf_idx and wb_idx, these checks can now > > >> be dropped as well. > > >> > > >> Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual > > >> encoder") > > > > > > Please adjust the Fixes tags in all three commits. I didn't notice this > > > beforehand and Stephen has complained. > > I think Stephen is Stephen Rothwell. Ugh, yes. Please excuse me. My brain didn't kick in to notice the name aliasing issue. > > Is something wrong with the tag? Format and hash looked right to me. > > $ git config pretty.fixes > Fixes: %h ("%s") > $ git help fixes > 'fixes' is aliased to 'show -s --pretty=fixes' > $ git fixes e02a559a720f > Fixes: e02a559a720f ("drm/msm/dpu: make changes to dpu_encoder to > support virtual encoder") > > it's missing the drm/msm/dpu prefix. I have more or less the same setup using a longer format and using the git-log instead of git-show. This way I can just do a git fixes drivers/gpu/drm/msm and spot the commit in question. [pretty] fixes = %C(auto)commit %H%Creset%nFixes: %h (\"%s\")%nAuthor: %aN <%aE>%nDate: %aD%nComitter-Date: %cD%n%n%w(0,4,4)%b
On 6/15/2022 11:21 PM, Dmitry Baryshkov wrote: > On Thu, 16 Jun 2022 at 09:18, Stephen Boyd <swboyd@chromium.org> wrote: >> >> Quoting Abhinav Kumar (2022-06-15 22:59:25) >>> Hi Dmitry >>> >>> On 6/15/2022 10:55 PM, Dmitry Baryshkov wrote: >>>> On 14/06/2022 22:32, Abhinav Kumar wrote: >>>>> intf and wb resources are not dependent on the rm global >>>>> state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). >>>>> >>>>> Move the allocation of intf and wb resources to >>>>> dpu_encoder_setup_display() >>>>> so that we can utilize the hw caps even during atomic_check() phase. >>>>> >>>>> Since dpu_encoder_setup_display() already has protection against >>>>> setting invalid intf_idx and wb_idx, these checks can now >>>>> be dropped as well. >>>>> >>>>> Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual >>>>> encoder") >>>> >>>> Please adjust the Fixes tags in all three commits. I didn't notice this >>>> beforehand and Stephen has complained. >> >> I think Stephen is Stephen Rothwell. > > Ugh, yes. Please excuse me. My brain didn't kick in to notice the name > aliasing issue. > >>> Is something wrong with the tag? Format and hash looked right to me. >> >> $ git config pretty.fixes >> Fixes: %h ("%s") >> $ git help fixes >> 'fixes' is aliased to 'show -s --pretty=fixes' >> $ git fixes e02a559a720f >> Fixes: e02a559a720f ("drm/msm/dpu: make changes to dpu_encoder to >> support virtual encoder") >> >> it's missing the drm/msm/dpu prefix. > > I have more or less the same setup using a longer format and using the > git-log instead of git-show. This way I can just do a git fixes > drivers/gpu/drm/msm and spot the commit in question. > > [pretty] > fixes = %C(auto)commit %H%Creset%nFixes: %h (\"%s\")%nAuthor: > %aN <%aE>%nDate: %aD%nComitter-Date: %cD%n%n%w(0,4,4)%b Thank you Stephen and Dmitry. That was a silly mistake to miss the prefix. Will fix it. >
Quoting Dmitry Baryshkov (2022-06-15 23:21:56) > > I have more or less the same setup using a longer format and using the > git-log instead of git-show. This way I can just do a git fixes > drivers/gpu/drm/msm and spot the commit in question. I've long desired to have this be part of checkpatch.pl, but nobody has done it yet. Maybe figuring out if the commit exists is harder than I think.
On 16/06/2022 09:28, Stephen Boyd wrote: > Quoting Dmitry Baryshkov (2022-06-15 23:21:56) >> >> I have more or less the same setup using a longer format and using the >> git-log instead of git-show. This way I can just do a git fixes >> drivers/gpu/drm/msm and spot the commit in question. > > I've long desired to have this be part of checkpatch.pl, but nobody has > done it yet. Maybe figuring out if the commit exists is harder than I > think. The issue is that the commit might exist, but might be not the ancestor of the commit in question. And when we do checkpatch, we have no way to check the lineage.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3a462e327e0e..e991d4ba8a40 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1048,24 +1048,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, phys->hw_pp = dpu_enc->hw_pp[i]; phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]); - 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->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 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; - } - phys->cached_mode = crtc_state->adjusted_mode; if (phys->ops.atomic_mode_set) phys->ops.atomic_mode_set(phys, crtc_state, conn_state); @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; atomic_set(&phys->vsync_cnt, 0); atomic_set(&phys->underrun_cnt, 0); + + 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->wb_idx >= WB_0 && phys->wb_idx < WB_MAX) + phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx); } + mutex_unlock(&dpu_enc->enc_lock); return ret;
intf and wb resources are not dependent on the rm global state so need not be allocated during dpu_encoder_virt_atomic_mode_set(). Move the allocation of intf and wb resources to dpu_encoder_setup_display() so that we can utilize the hw caps even during atomic_check() phase. Since dpu_encoder_setup_display() already has protection against setting invalid intf_idx and wb_idx, these checks can now be dropped as well. Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual encoder") Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-)