| Message ID | 1683061382-32651-3-git-send-email-quic_khsieh@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add DSC 1.2 dpu supports | expand |
On 03/05/2023 00:02, Kuogee Hsieh wrote: > Legacy DPU requires PP block to be involved during DSC setting up. > This patch adds DDPU_PINGPONG_DSC feature bit to indicate that both DPU_PINGPONG_DSC > dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_enable() pingpong ops > functions are required to complete DSC data path set up and start > DSC engine. Nit: as these ops were already present, I'd say that the lack of the flag means that these operations are not supported and must not be called for DSC setup/teardown. Nevertheless: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Reported-by : Marijn Suijten <marijn.suijten@somainline.org> > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 9 ++++++--- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > index 71584cd..c07a6b6 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > @@ -144,6 +144,7 @@ enum { > * @DPU_PINGPONG_SPLIT PP block supports split fifo > * @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo > * @DPU_PINGPONG_DITHER, Dither blocks > + * @DPU_PINGPONG_DSC, PP ops functions required for DSC > * @DPU_PINGPONG_MAX > */ > enum { > @@ -152,6 +153,7 @@ enum { > DPU_PINGPONG_SPLIT, > DPU_PINGPONG_SLAVE, > DPU_PINGPONG_DITHER, > + DPU_PINGPONG_DSC, > DPU_PINGPONG_MAX > }; > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c > index 3822e06..f255a04 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c > @@ -264,9 +264,12 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c, > c->ops.get_autorefresh = dpu_hw_pp_get_autorefresh_config; > c->ops.poll_timeout_wr_ptr = dpu_hw_pp_poll_timeout_wr_ptr; > c->ops.get_line_count = dpu_hw_pp_get_line_count; > - c->ops.setup_dsc = dpu_hw_pp_setup_dsc; > - c->ops.enable_dsc = dpu_hw_pp_dsc_enable; > - c->ops.disable_dsc = dpu_hw_pp_dsc_disable; > + > + if (features & BIT(DPU_PINGPONG_DSC)) { > + c->ops.setup_dsc = dpu_hw_pp_setup_dsc; > + c->ops.enable_dsc = dpu_hw_pp_dsc_enable; > + c->ops.disable_dsc = dpu_hw_pp_dsc_disable; > + } > > if (test_bit(DPU_PINGPONG_DITHER, &features)) > c->ops.setup_dither = dpu_hw_pp_setup_dither;
On 2023-05-02 14:02:57, Kuogee Hsieh wrote: > Legacy DPU requires PP block to be involved during DSC setting up. This patch should clarify that "legacy" means DPU < 7.0.0, as we found out in [1] that PINGPONG has no more register remaining in 7.x. In addition, it seems that the DCE enable register/flag moved to INTF, as added by Jessica in [2]. Perhaps those patches should be part of this series too, and this patch should mention that it was moved? [1]: https://lore.kernel.org/linux-arm-msm/20230411-dpu-intf-te-v4-7-27ce1a5ab5c6@somainline.org/ [2]: https://lore.kernel.org/linux-arm-msm/20230405-add-dsc-support-v1-0-6bc6f03ae735@quicinc.com/T/#t > This patch adds DDPU_PINGPONG_DSC feature bit to indicate that both > dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_enable() pingpong ops > functions are required to complete DSC data path set up and start datapath setup* As already suggested by Dmitry's review in a different way, this patch doesn't "indicate that both ops are required to complete DSC datapath setup", this patch removes those callbacks from PP since DPU 7.0.0 as the registers are no longer present (and have been moved to the INTF in some form). The *implementation* is good though, and I'd r-b it after addressing the nits - thanks! > DSC engine. > > Reported-by : Marijn Suijten <marijn.suijten@somainline.org> drop the space before :. > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 9 ++++++--- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > index 71584cd..c07a6b6 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > @@ -144,6 +144,7 @@ enum { > * @DPU_PINGPONG_SPLIT PP block supports split fifo > * @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo > * @DPU_PINGPONG_DITHER, Dither blocks > + * @DPU_PINGPONG_DSC, PP ops functions required for DSC Mixing tab indentation, and the comma shouldn't be there. - Marijn > * @DPU_PINGPONG_MAX > */ > enum { > @@ -152,6 +153,7 @@ enum { > DPU_PINGPONG_SPLIT, > DPU_PINGPONG_SLAVE, > DPU_PINGPONG_DITHER, > + DPU_PINGPONG_DSC, > DPU_PINGPONG_MAX > }; > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c > index 3822e06..f255a04 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c > @@ -264,9 +264,12 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c, > c->ops.get_autorefresh = dpu_hw_pp_get_autorefresh_config; > c->ops.poll_timeout_wr_ptr = dpu_hw_pp_poll_timeout_wr_ptr; > c->ops.get_line_count = dpu_hw_pp_get_line_count; > - c->ops.setup_dsc = dpu_hw_pp_setup_dsc; > - c->ops.enable_dsc = dpu_hw_pp_dsc_enable; > - c->ops.disable_dsc = dpu_hw_pp_dsc_disable; > + > + if (features & BIT(DPU_PINGPONG_DSC)) { > + c->ops.setup_dsc = dpu_hw_pp_setup_dsc; > + c->ops.enable_dsc = dpu_hw_pp_dsc_enable; > + c->ops.disable_dsc = dpu_hw_pp_dsc_disable; > + } > > if (test_bit(DPU_PINGPONG_DITHER, &features)) > c->ops.setup_dither = dpu_hw_pp_setup_dither; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index 71584cd..c07a6b6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -144,6 +144,7 @@ enum { * @DPU_PINGPONG_SPLIT PP block supports split fifo * @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo * @DPU_PINGPONG_DITHER, Dither blocks + * @DPU_PINGPONG_DSC, PP ops functions required for DSC * @DPU_PINGPONG_MAX */ enum { @@ -152,6 +153,7 @@ enum { DPU_PINGPONG_SPLIT, DPU_PINGPONG_SLAVE, DPU_PINGPONG_DITHER, + DPU_PINGPONG_DSC, DPU_PINGPONG_MAX }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c index 3822e06..f255a04 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c @@ -264,9 +264,12 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c, c->ops.get_autorefresh = dpu_hw_pp_get_autorefresh_config; c->ops.poll_timeout_wr_ptr = dpu_hw_pp_poll_timeout_wr_ptr; c->ops.get_line_count = dpu_hw_pp_get_line_count; - c->ops.setup_dsc = dpu_hw_pp_setup_dsc; - c->ops.enable_dsc = dpu_hw_pp_dsc_enable; - c->ops.disable_dsc = dpu_hw_pp_dsc_disable; + + if (features & BIT(DPU_PINGPONG_DSC)) { + c->ops.setup_dsc = dpu_hw_pp_setup_dsc; + c->ops.enable_dsc = dpu_hw_pp_dsc_enable; + c->ops.disable_dsc = dpu_hw_pp_dsc_disable; + } if (test_bit(DPU_PINGPONG_DITHER, &features)) c->ops.setup_dither = dpu_hw_pp_setup_dither;
Legacy DPU requires PP block to be involved during DSC setting up. This patch adds DDPU_PINGPONG_DSC feature bit to indicate that both dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_enable() pingpong ops functions are required to complete DSC data path set up and start DSC engine. Reported-by : Marijn Suijten <marijn.suijten@somainline.org> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-)