| Message ID | 20221221231943.1961117-7-marijn.suijten@somainline.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: DSC Electric Boogaloo for sm8[12]50 | expand |
On 22/12/2022 01:19, Marijn Suijten wrote: > Downstream calls this num_enc yet the DSC patches introduced a new > num_dsc struct member, leaving num_enc effectively unused. > > Fixes: 7e9cc175b159 ("drm/msm/disp/dpu1: Add support for DSC in topology") > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 9 ++++----- > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 4 ++-- > drivers/gpu/drm/msm/msm_drv.h | 2 -- > 3 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 9c6817b5a194..a158cd502d38 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -579,19 +579,18 @@ static struct msm_display_topology dpu_encoder_get_topology( > topology.num_dspp = topology.num_lm; > } > > - topology.num_enc = 0; > topology.num_intf = intf_count; > > if (dpu_enc->dsc) { > - /* In case of Display Stream Compression (DSC), we would use > - * 2 encoders, 2 layer mixers and 1 interface > + /* > + * In case of Display Stream Compression (DSC), we would use > + * 2 DSC encoders, 2 layer mixers and 1 interface > * this is power optimal and can drive up to (including) 4k > * screens > */ > - topology.num_enc = 2; > topology.num_dsc = 2; > - topology.num_intf = 1; > topology.num_lm = 2; > + topology.num_intf = 1; Unless there is a reason, please move num_intf assignment back while preparing v3. With that fixed: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > } > > return topology; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > index dcbf03d2940a..5e7aa0f3a31c 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > @@ -548,8 +548,8 @@ static int _dpu_rm_populate_requirements( > { > reqs->topology = req_topology; > > - DRM_DEBUG_KMS("num_lm: %d num_enc: %d num_intf: %d\n", > - reqs->topology.num_lm, reqs->topology.num_enc, > + DRM_DEBUG_KMS("num_lm: %d num_dsc: %d num_intf: %d\n", > + reqs->topology.num_lm, reqs->topology.num_dsc, > reqs->topology.num_intf); > > return 0; > diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h > index d4e0ef608950..74626a271f46 100644 > --- a/drivers/gpu/drm/msm/msm_drv.h > +++ b/drivers/gpu/drm/msm/msm_drv.h > @@ -82,14 +82,12 @@ enum msm_event_wait { > /** > * struct msm_display_topology - defines a display topology pipeline > * @num_lm: number of layer mixers used > - * @num_enc: number of compression encoder blocks used > * @num_intf: number of interfaces the panel is mounted on > * @num_dspp: number of dspp blocks used > * @num_dsc: number of Display Stream Compression (DSC) blocks used > */ > struct msm_display_topology { > u32 num_lm; > - u32 num_enc; > u32 num_intf; > u32 num_dspp; > u32 num_dsc;
On 2023-01-09 01:31:57, Dmitry Baryshkov wrote: > On 22/12/2022 01:19, Marijn Suijten wrote: > > Downstream calls this num_enc yet the DSC patches introduced a new > > num_dsc struct member, leaving num_enc effectively unused. > > > > Fixes: 7e9cc175b159 ("drm/msm/disp/dpu1: Add support for DSC in topology") > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 9 ++++----- > > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 4 ++-- > > drivers/gpu/drm/msm/msm_drv.h | 2 -- > > 3 files changed, 6 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > index 9c6817b5a194..a158cd502d38 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > @@ -579,19 +579,18 @@ static struct msm_display_topology dpu_encoder_get_topology( > > topology.num_dspp = topology.num_lm; > > } > > > > - topology.num_enc = 0; > > topology.num_intf = intf_count; > > > > if (dpu_enc->dsc) { > > - /* In case of Display Stream Compression (DSC), we would use > > - * 2 encoders, 2 layer mixers and 1 interface > > + /* > > + * In case of Display Stream Compression (DSC), we would use > > + * 2 DSC encoders, 2 layer mixers and 1 interface > > * this is power optimal and can drive up to (including) 4k > > * screens > > */ > > - topology.num_enc = 2; > > topology.num_dsc = 2; > > - topology.num_intf = 1; > > topology.num_lm = 2; > > + topology.num_intf = 1; > > Unless there is a reason, please move num_intf assignment back while > preparing v3. The assignment was reordered to match the order described in the comment right above, such that this reads more naturally. Not sure if it's worth sending that as a separate fix, or drop it entirely. > With that fixed: > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> <snip>
On Mon, 9 Jan 2023 at 10:21, Marijn Suijten <marijn.suijten@somainline.org> wrote: > > On 2023-01-09 01:31:57, Dmitry Baryshkov wrote: > > On 22/12/2022 01:19, Marijn Suijten wrote: > > > Downstream calls this num_enc yet the DSC patches introduced a new > > > num_dsc struct member, leaving num_enc effectively unused. > > > > > > Fixes: 7e9cc175b159 ("drm/msm/disp/dpu1: Add support for DSC in topology") > > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > > > --- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 9 ++++----- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 4 ++-- > > > drivers/gpu/drm/msm/msm_drv.h | 2 -- > > > 3 files changed, 6 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > index 9c6817b5a194..a158cd502d38 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > @@ -579,19 +579,18 @@ static struct msm_display_topology dpu_encoder_get_topology( > > > topology.num_dspp = topology.num_lm; > > > } > > > > > > - topology.num_enc = 0; > > > topology.num_intf = intf_count; > > > > > > if (dpu_enc->dsc) { > > > - /* In case of Display Stream Compression (DSC), we would use > > > - * 2 encoders, 2 layer mixers and 1 interface > > > + /* > > > + * In case of Display Stream Compression (DSC), we would use > > > + * 2 DSC encoders, 2 layer mixers and 1 interface > > > * this is power optimal and can drive up to (including) 4k > > > * screens > > > */ > > > - topology.num_enc = 2; > > > topology.num_dsc = 2; > > > - topology.num_intf = 1; > > > topology.num_lm = 2; > > > + topology.num_intf = 1; > > > > Unless there is a reason, please move num_intf assignment back while > > preparing v3. > > The assignment was reordered to match the order described in the comment > right above, such that this reads more naturally. Not sure if it's > worth sending that as a separate fix, or drop it entirely. I see. Sounds logical then. Let's keep it as is. > > > With that fixed: > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > <snip>
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 9c6817b5a194..a158cd502d38 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -579,19 +579,18 @@ static struct msm_display_topology dpu_encoder_get_topology( topology.num_dspp = topology.num_lm; } - topology.num_enc = 0; topology.num_intf = intf_count; if (dpu_enc->dsc) { - /* In case of Display Stream Compression (DSC), we would use - * 2 encoders, 2 layer mixers and 1 interface + /* + * In case of Display Stream Compression (DSC), we would use + * 2 DSC encoders, 2 layer mixers and 1 interface * this is power optimal and can drive up to (including) 4k * screens */ - topology.num_enc = 2; topology.num_dsc = 2; - topology.num_intf = 1; topology.num_lm = 2; + topology.num_intf = 1; } return topology; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c index dcbf03d2940a..5e7aa0f3a31c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c @@ -548,8 +548,8 @@ static int _dpu_rm_populate_requirements( { reqs->topology = req_topology; - DRM_DEBUG_KMS("num_lm: %d num_enc: %d num_intf: %d\n", - reqs->topology.num_lm, reqs->topology.num_enc, + DRM_DEBUG_KMS("num_lm: %d num_dsc: %d num_intf: %d\n", + reqs->topology.num_lm, reqs->topology.num_dsc, reqs->topology.num_intf); return 0; diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index d4e0ef608950..74626a271f46 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -82,14 +82,12 @@ enum msm_event_wait { /** * struct msm_display_topology - defines a display topology pipeline * @num_lm: number of layer mixers used - * @num_enc: number of compression encoder blocks used * @num_intf: number of interfaces the panel is mounted on * @num_dspp: number of dspp blocks used * @num_dsc: number of Display Stream Compression (DSC) blocks used */ struct msm_display_topology { u32 num_lm; - u32 num_enc; u32 num_intf; u32 num_dspp; u32 num_dsc;
Downstream calls this num_enc yet the DSC patches introduced a new num_dsc struct member, leaving num_enc effectively unused. Fixes: 7e9cc175b159 ("drm/msm/disp/dpu1: Add support for DSC in topology") Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 9 ++++----- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 4 ++-- drivers/gpu/drm/msm/msm_drv.h | 2 -- 3 files changed, 6 insertions(+), 9 deletions(-)