| Message ID | 1655335395-16159-1-git-send-email-quic_abhinavk@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm/dpu: set preferred mode for writeback connector | expand |
On 16/06/2022 02:23, Abhinav Kumar wrote: > After [1] was merged to IGT, we use either the first supported > mode in the list OR the preferred mode to determine the primary > plane to use for the sub-test due to the IGT API [2]. > > Since writeback does not set any preferred mode, this was > selecting 4k as that was the first entry in the list. > > We use maxlinewidth to add the list of supported modes for > the writeback connector which is the right thing to do, however > since we do not have dual-SSPP support yet for DPU, this fails > the bandwidth check in dpu_core_perf_crtc_check(). > > Till we have dual-SSPP support, workaround this mismatch between > the list of supported modes and maxlinewidth limited modes by > marking 640x480 as the preferred mode for DPU writeback because > kms_writeback tests 640x480 mode only [3]. Telling that we support modes up to 4k, failing to set 4k mode and then using the preferred mode to force IGT to lower resolution sounds like a hack. As adding wide dual-SSPP support will take some time. I'd suggest dropping support for 4k modes for now and using DEFAULT_DPU_LINE_WIDTH instead (or hw_wb->caps->maxlinewidth). A comment in the source code that the check should be removed/modified once dual-SSPP is supported would be helpful. > [1]: https://patchwork.freedesktop.org/patch/486441/ > [2]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/lib/igt_kms.c#L1562 > [3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L68 > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Any Fixes tags? > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c > index 399115e4e217..104cc59d6466 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c > @@ -10,9 +10,14 @@ static int dpu_wb_conn_get_modes(struct drm_connector *connector) > struct drm_device *dev = connector->dev; > struct msm_drm_private *priv = dev->dev_private; > struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); > + int count; > > - return drm_add_modes_noedid(connector, dpu_kms->catalog->caps->max_linewidth, > + count = drm_add_modes_noedid(connector, dpu_kms->catalog->caps->max_linewidth, > dev->mode_config.max_height); > + > + drm_set_preferred_mode(connector, 640, 480); > + > + return count; > } > > static const struct drm_connector_funcs dpu_wb_conn_funcs = {
On 6/16/2022 1:36 AM, Dmitry Baryshkov wrote: > On 16/06/2022 02:23, Abhinav Kumar wrote: >> After [1] was merged to IGT, we use either the first supported >> mode in the list OR the preferred mode to determine the primary >> plane to use for the sub-test due to the IGT API [2]. >> >> Since writeback does not set any preferred mode, this was >> selecting 4k as that was the first entry in the list. >> >> We use maxlinewidth to add the list of supported modes for >> the writeback connector which is the right thing to do, however >> since we do not have dual-SSPP support yet for DPU, this fails >> the bandwidth check in dpu_core_perf_crtc_check(). >> >> Till we have dual-SSPP support, workaround this mismatch between >> the list of supported modes and maxlinewidth limited modes by >> marking 640x480 as the preferred mode for DPU writeback because >> kms_writeback tests 640x480 mode only [3]. > > Telling that we support modes up to 4k, failing to set 4k mode and then > using the preferred mode to force IGT to lower resolution sounds like a > hack. > > As adding wide dual-SSPP support will take some time. I'd suggest > dropping support for 4k modes for now and using DEFAULT_DPU_LINE_WIDTH > instead (or hw_wb->caps->maxlinewidth). A comment in the source code > that the check should be removed/modified once dual-SSPP is supported > would be helpful. > Yes, I am planning to drop this one and use max_mixerwidth instead as i posted on IRC. > >> [1]: https://patchwork.freedesktop.org/patch/486441/ >> [2]: >> https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/lib/igt_kms.c#L1562 >> >> [3]: >> https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L68 >> >> >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > > Any Fixes tags? Yes, will add it in the new patch. > >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c >> index 399115e4e217..104cc59d6466 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c >> @@ -10,9 +10,14 @@ static int dpu_wb_conn_get_modes(struct >> drm_connector *connector) >> struct drm_device *dev = connector->dev; >> struct msm_drm_private *priv = dev->dev_private; >> struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); >> + int count; >> - return drm_add_modes_noedid(connector, >> dpu_kms->catalog->caps->max_linewidth, >> + count = drm_add_modes_noedid(connector, >> dpu_kms->catalog->caps->max_linewidth, >> dev->mode_config.max_height); >> + >> + drm_set_preferred_mode(connector, 640, 480); >> + >> + return count; >> } >> static const struct drm_connector_funcs dpu_wb_conn_funcs = { > >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c index 399115e4e217..104cc59d6466 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c @@ -10,9 +10,14 @@ static int dpu_wb_conn_get_modes(struct drm_connector *connector) struct drm_device *dev = connector->dev; struct msm_drm_private *priv = dev->dev_private; struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); + int count; - return drm_add_modes_noedid(connector, dpu_kms->catalog->caps->max_linewidth, + count = drm_add_modes_noedid(connector, dpu_kms->catalog->caps->max_linewidth, dev->mode_config.max_height); + + drm_set_preferred_mode(connector, 640, 480); + + return count; } static const struct drm_connector_funcs dpu_wb_conn_funcs = {
After [1] was merged to IGT, we use either the first supported mode in the list OR the preferred mode to determine the primary plane to use for the sub-test due to the IGT API [2]. Since writeback does not set any preferred mode, this was selecting 4k as that was the first entry in the list. We use maxlinewidth to add the list of supported modes for the writeback connector which is the right thing to do, however since we do not have dual-SSPP support yet for DPU, this fails the bandwidth check in dpu_core_perf_crtc_check(). Till we have dual-SSPP support, workaround this mismatch between the list of supported modes and maxlinewidth limited modes by marking 640x480 as the preferred mode for DPU writeback because kms_writeback tests 640x480 mode only [3]. [1]: https://patchwork.freedesktop.org/patch/486441/ [2]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/lib/igt_kms.c#L1562 [3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L68 Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)