| Message ID | 20241206-no_3dmux-v1-1-72ad2677a323@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm/dpu: filter out too wide modes if no 3dmux is present | expand |
On Fri, Dec 06, 2024 at 12:00:53PM -0800, Abhinav Kumar wrote: > On chipsets such as QCS615, there is no 3dmux present. In such > a case, a layer exceeding the max_mixer_width cannot be split, > hence cannot be supported. > > Filter out the modes which exceed the max_mixer_width when there > is no 3dmux present. Also, add a check in the dpu_crtc_atomic_check() > to return failure for such modes. > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > --- > Note: this was only compile tested, so its pending validation on QCS615 > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 9f6ffd344693ecfb633095772a31ada5613345dc..e6e5540aae83be7c20d8ae29115b8fdd42056e55 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -732,6 +732,13 @@ static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc, > struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); > int i; > > + /* if we cannot merge 2 LMs (no 3d mux) better to fail earlier > + * before even checking the width after the split > + */ > + if (!dpu_kms->catalog->caps->has_3d_merge > + && adj_mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) > + return -E2BIG; Is it the same as checking that there are LMs which support DPU_MIXER_SOURCESPLIT ? > + > for (i = 0; i < cstate->num_mixers; i++) { > struct drm_rect *r = &cstate->lm_bounds[i]; > r->x1 = crtc_split_width * i; > @@ -1251,6 +1258,12 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc, > { > struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); > > + /* if there is no 3d_mux block we cannot merge LMs so we cannot > + * split the large layer into 2 LMs, filter out such modes > + */ > + if (!dpu_kms->catalog->caps->has_3d_merge > + && mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) > + return MODE_BAD; This can be more specific, like MODE_BAD_HVALUE. > /* > * max crtc width is equal to the max mixer width * 2 and max height is 4K > */ > > --- > base-commit: af2ea8ab7a546b430726183458da0a173d331272 > change-id: 20241206-no_3dmux-521a55ea0669 > > Best regards, > -- > Abhinav Kumar <quic_abhinavk@quicinc.com> >
On 12/7/2024 9:29 PM, Dmitry Baryshkov wrote: > On Fri, Dec 06, 2024 at 12:00:53PM -0800, Abhinav Kumar wrote: >> On chipsets such as QCS615, there is no 3dmux present. In such >> a case, a layer exceeding the max_mixer_width cannot be split, >> hence cannot be supported. >> >> Filter out the modes which exceed the max_mixer_width when there >> is no 3dmux present. Also, add a check in the dpu_crtc_atomic_check() >> to return failure for such modes. >> >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >> --- >> Note: this was only compile tested, so its pending validation on QCS615 >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> index 9f6ffd344693ecfb633095772a31ada5613345dc..e6e5540aae83be7c20d8ae29115b8fdd42056e55 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >> @@ -732,6 +732,13 @@ static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc, >> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); >> int i; >> >> + /* if we cannot merge 2 LMs (no 3d mux) better to fail earlier >> + * before even checking the width after the split >> + */ >> + if (!dpu_kms->catalog->caps->has_3d_merge >> + && adj_mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) >> + return -E2BIG; > > Is it the same as checking that there are LMs which support > DPU_MIXER_SOURCESPLIT ? > DPU_MIXER_SOURCESPLIT tells whether we can have more than one SSPP in the same blend stage. 494 if (test_bit(DPU_MIXER_SOURCESPLIT, 495 &ctx->mixer_hw_caps->features)) 496 pipes_per_stage = PIPES_PER_STAGE; 497 else 498 pipes_per_stage = 1; That is different from this one. Here we are checking if we can actually blend two LM outputs using the 3dmux (so its post blend). >> + >> for (i = 0; i < cstate->num_mixers; i++) { >> struct drm_rect *r = &cstate->lm_bounds[i]; >> r->x1 = crtc_split_width * i; >> @@ -1251,6 +1258,12 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc, >> { >> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); >> >> + /* if there is no 3d_mux block we cannot merge LMs so we cannot >> + * split the large layer into 2 LMs, filter out such modes >> + */ >> + if (!dpu_kms->catalog->caps->has_3d_merge >> + && mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) >> + return MODE_BAD; > > This can be more specific, like MODE_BAD_HVALUE. > Yes for sure, will fix this up. >> /* >> * max crtc width is equal to the max mixer width * 2 and max height is 4K >> */ >> >> --- >> base-commit: af2ea8ab7a546b430726183458da0a173d331272 >> change-id: 20241206-no_3dmux-521a55ea0669 >> >> Best regards, >> -- >> Abhinav Kumar <quic_abhinavk@quicinc.com> >> >
Hi Abhinav, On Mon, 9 Dec 2024 at 22:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 12/7/2024 9:29 PM, Dmitry Baryshkov wrote: > > On Fri, Dec 06, 2024 at 12:00:53PM -0800, Abhinav Kumar wrote: > >> On chipsets such as QCS615, there is no 3dmux present. In such > >> a case, a layer exceeding the max_mixer_width cannot be split, > >> hence cannot be supported. > >> > >> Filter out the modes which exceed the max_mixer_width when there > >> is no 3dmux present. Also, add a check in the dpu_crtc_atomic_check() > >> to return failure for such modes. > >> > >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > >> --- > >> Note: this was only compile tested, so its pending validation on QCS615 > >> --- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 +++++++++++++ > >> 1 file changed, 13 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > >> index 9f6ffd344693ecfb633095772a31ada5613345dc..e6e5540aae83be7c20d8ae29115b8fdd42056e55 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > >> @@ -732,6 +732,13 @@ static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc, > >> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); > >> int i; > >> > >> + /* if we cannot merge 2 LMs (no 3d mux) better to fail earlier > >> + * before even checking the width after the split > >> + */ > >> + if (!dpu_kms->catalog->caps->has_3d_merge > >> + && adj_mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) > >> + return -E2BIG; > > > > Is it the same as checking that there are LMs which support > > DPU_MIXER_SOURCESPLIT ? > > > > DPU_MIXER_SOURCESPLIT tells whether we can have more than one SSPP in > the same blend stage. Do we have a feature bit that corresponds to the ability to use 2 LMs? I mean, there are other *split topologies, not necessarily the 3DMux ones. E.g. PPSPLIT. > > 494 if (test_bit(DPU_MIXER_SOURCESPLIT, > 495 &ctx->mixer_hw_caps->features)) > 496 pipes_per_stage = PIPES_PER_STAGE; > 497 else > 498 pipes_per_stage = 1; > > That is different from this one. Here we are checking if we can actually > blend two LM outputs using the 3dmux (so its post blend). > > >> + > >> for (i = 0; i < cstate->num_mixers; i++) { > >> struct drm_rect *r = &cstate->lm_bounds[i]; > >> r->x1 = crtc_split_width * i; > >> @@ -1251,6 +1258,12 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc, > >> { > >> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); > >> > >> + /* if there is no 3d_mux block we cannot merge LMs so we cannot > >> + * split the large layer into 2 LMs, filter out such modes > >> + */ > >> + if (!dpu_kms->catalog->caps->has_3d_merge > >> + && mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) > >> + return MODE_BAD; > > > > This can be more specific, like MODE_BAD_HVALUE. > > > > Yes for sure, will fix this up. > > >> /* > >> * max crtc width is equal to the max mixer width * 2 and max height is 4K > >> */ > >> > >> --- > >> base-commit: af2ea8ab7a546b430726183458da0a173d331272 > >> change-id: 20241206-no_3dmux-521a55ea0669 > >> > >> Best regards, > >> -- > >> Abhinav Kumar <quic_abhinavk@quicinc.com> > >> > >
Hi Dmitry On 12/9/2024 2:16 PM, Dmitry Baryshkov wrote: > Hi Abhinav, > > On Mon, 9 Dec 2024 at 22:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> >> >> On 12/7/2024 9:29 PM, Dmitry Baryshkov wrote: >>> On Fri, Dec 06, 2024 at 12:00:53PM -0800, Abhinav Kumar wrote: >>>> On chipsets such as QCS615, there is no 3dmux present. In such >>>> a case, a layer exceeding the max_mixer_width cannot be split, >>>> hence cannot be supported. >>>> >>>> Filter out the modes which exceed the max_mixer_width when there >>>> is no 3dmux present. Also, add a check in the dpu_crtc_atomic_check() >>>> to return failure for such modes. >>>> >>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>>> --- >>>> Note: this was only compile tested, so its pending validation on QCS615 >>>> --- >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 +++++++++++++ >>>> 1 file changed, 13 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >>>> index 9f6ffd344693ecfb633095772a31ada5613345dc..e6e5540aae83be7c20d8ae29115b8fdd42056e55 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c >>>> @@ -732,6 +732,13 @@ static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc, >>>> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); >>>> int i; >>>> >>>> + /* if we cannot merge 2 LMs (no 3d mux) better to fail earlier >>>> + * before even checking the width after the split >>>> + */ >>>> + if (!dpu_kms->catalog->caps->has_3d_merge >>>> + && adj_mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) >>>> + return -E2BIG; >>> >>> Is it the same as checking that there are LMs which support >>> DPU_MIXER_SOURCESPLIT ? >>> >> >> DPU_MIXER_SOURCESPLIT tells whether we can have more than one SSPP in >> the same blend stage. > > Do we have a feature bit that corresponds to the ability to use 2 LMs? > I mean, there are other *split topologies, not necessarily the 3DMux > ones. E.g. PPSPLIT. > A layer can always be split across LMs. There is not really any feature bit for this as it can always be done in pretty much all DPU chipsets. Here the issue is we are not able to merge because there are no 3d mux blocks and hence we cannot split. We need to merge because, the same display is requiring multiple LMs. PP split will be a single LM going to two PPs and that going to two INTFs. Hence the way to look at this patch would be we are avoiding split because we cannot merge and not that we cannot split. >> >> 494 if (test_bit(DPU_MIXER_SOURCESPLIT, >> 495 &ctx->mixer_hw_caps->features)) >> 496 pipes_per_stage = PIPES_PER_STAGE; >> 497 else >> 498 pipes_per_stage = 1; >> >> That is different from this one. Here we are checking if we can actually >> blend two LM outputs using the 3dmux (so its post blend). >> >>>> + >>>> for (i = 0; i < cstate->num_mixers; i++) { >>>> struct drm_rect *r = &cstate->lm_bounds[i]; >>>> r->x1 = crtc_split_width * i; >>>> @@ -1251,6 +1258,12 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc, >>>> { >>>> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); >>>> >>>> + /* if there is no 3d_mux block we cannot merge LMs so we cannot >>>> + * split the large layer into 2 LMs, filter out such modes >>>> + */ >>>> + if (!dpu_kms->catalog->caps->has_3d_merge >>>> + && mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) >>>> + return MODE_BAD; >>> >>> This can be more specific, like MODE_BAD_HVALUE. >>> >> >> Yes for sure, will fix this up. >> >>>> /* >>>> * max crtc width is equal to the max mixer width * 2 and max height is 4K >>>> */ >>>> >>>> --- >>>> base-commit: af2ea8ab7a546b430726183458da0a173d331272 >>>> change-id: 20241206-no_3dmux-521a55ea0669 >>>> >>>> Best regards, >>>> -- >>>> Abhinav Kumar <quic_abhinavk@quicinc.com> >>>> >>> > > >
Hi Abhinav, On Tue, 10 Dec 2024 at 00:30, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > Hi Dmitry > > On 12/9/2024 2:16 PM, Dmitry Baryshkov wrote: > > Hi Abhinav, > > > > On Mon, 9 Dec 2024 at 22:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >> > >> > >> > >> On 12/7/2024 9:29 PM, Dmitry Baryshkov wrote: > >>> On Fri, Dec 06, 2024 at 12:00:53PM -0800, Abhinav Kumar wrote: > >>>> On chipsets such as QCS615, there is no 3dmux present. In such > >>>> a case, a layer exceeding the max_mixer_width cannot be split, > >>>> hence cannot be supported. > >>>> > >>>> Filter out the modes which exceed the max_mixer_width when there > >>>> is no 3dmux present. Also, add a check in the dpu_crtc_atomic_check() > >>>> to return failure for such modes. > >>>> > >>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > >>>> --- > >>>> Note: this was only compile tested, so its pending validation on QCS615 > >>>> --- > >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 +++++++++++++ > >>>> 1 file changed, 13 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > >>>> index 9f6ffd344693ecfb633095772a31ada5613345dc..e6e5540aae83be7c20d8ae29115b8fdd42056e55 100644 > >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > >>>> @@ -732,6 +732,13 @@ static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc, > >>>> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); > >>>> int i; > >>>> > >>>> + /* if we cannot merge 2 LMs (no 3d mux) better to fail earlier > >>>> + * before even checking the width after the split > >>>> + */ > >>>> + if (!dpu_kms->catalog->caps->has_3d_merge > >>>> + && adj_mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) > >>>> + return -E2BIG; > >>> > >>> Is it the same as checking that there are LMs which support > >>> DPU_MIXER_SOURCESPLIT ? > >>> > >> > >> DPU_MIXER_SOURCESPLIT tells whether we can have more than one SSPP in > >> the same blend stage. > > > > Do we have a feature bit that corresponds to the ability to use 2 LMs? > > I mean, there are other *split topologies, not necessarily the 3DMux > > ones. E.g. PPSPLIT. > > > > A layer can always be split across LMs. There is not really any feature > bit for this as it can always be done in pretty much all DPU chipsets. > > Here the issue is we are not able to merge because there are no 3d mux > blocks and hence we cannot split. > > We need to merge because, the same display is requiring multiple LMs. > > PP split will be a single LM going to two PPs and that going to two INTFs. > > Hence the way to look at this patch would be we are avoiding split > because we cannot merge and not that we cannot split. Ack, thanks a lot for the explanation! > > >> > >> 494 if (test_bit(DPU_MIXER_SOURCESPLIT, > >> 495 &ctx->mixer_hw_caps->features)) > >> 496 pipes_per_stage = PIPES_PER_STAGE; > >> 497 else > >> 498 pipes_per_stage = 1; > >> > >> That is different from this one. Here we are checking if we can actually > >> blend two LM outputs using the 3dmux (so its post blend). > >> > >>>> + > >>>> for (i = 0; i < cstate->num_mixers; i++) { > >>>> struct drm_rect *r = &cstate->lm_bounds[i]; > >>>> r->x1 = crtc_split_width * i; > >>>> @@ -1251,6 +1258,12 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc, > >>>> { > >>>> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); > >>>> > >>>> + /* if there is no 3d_mux block we cannot merge LMs so we cannot > >>>> + * split the large layer into 2 LMs, filter out such modes > >>>> + */ > >>>> + if (!dpu_kms->catalog->caps->has_3d_merge > >>>> + && mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) > >>>> + return MODE_BAD; > >>> > >>> This can be more specific, like MODE_BAD_HVALUE. > >>> > >> > >> Yes for sure, will fix this up. > >> > >>>> /* > >>>> * max crtc width is equal to the max mixer width * 2 and max height is 4K > >>>> */ > >>>> > >>>> --- > >>>> base-commit: af2ea8ab7a546b430726183458da0a173d331272 > >>>> change-id: 20241206-no_3dmux-521a55ea0669 > >>>> > >>>> Best regards, > >>>> -- > >>>> Abhinav Kumar <quic_abhinavk@quicinc.com> > >>>> > >>> > > > > > >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 9f6ffd344693ecfb633095772a31ada5613345dc..e6e5540aae83be7c20d8ae29115b8fdd42056e55 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -732,6 +732,13 @@ static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc, struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); int i; + /* if we cannot merge 2 LMs (no 3d mux) better to fail earlier + * before even checking the width after the split + */ + if (!dpu_kms->catalog->caps->has_3d_merge + && adj_mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) + return -E2BIG; + for (i = 0; i < cstate->num_mixers; i++) { struct drm_rect *r = &cstate->lm_bounds[i]; r->x1 = crtc_split_width * i; @@ -1251,6 +1258,12 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc, { struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); + /* if there is no 3d_mux block we cannot merge LMs so we cannot + * split the large layer into 2 LMs, filter out such modes + */ + if (!dpu_kms->catalog->caps->has_3d_merge + && mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width) + return MODE_BAD; /* * max crtc width is equal to the max mixer width * 2 and max height is 4K */
On chipsets such as QCS615, there is no 3dmux present. In such a case, a layer exceeding the max_mixer_width cannot be split, hence cannot be supported. Filter out the modes which exceed the max_mixer_width when there is no 3dmux present. Also, add a check in the dpu_crtc_atomic_check() to return failure for such modes. Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> --- Note: this was only compile tested, so its pending validation on QCS615 --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) --- base-commit: af2ea8ab7a546b430726183458da0a173d331272 change-id: 20241206-no_3dmux-521a55ea0669 Best regards,