Re: [PATCH v1 00/14] add display port DSC feature
Hi Dmitry / Marijn I have seen your review comments and agree there is much work to be done to get this in a better shape. We will post a better V2 to address the concerns. Would appreciate some patience till then. Thanks for your support in reviews as always Abhinav On 1/23/2023 10:24 AM, Kuogee Hsieh wrote: This patch add DSC related supporting functions into to both dp controller and dpu enccoder Kuogee Hsieh (14): drm/msm/dp: add dpcd read of both dsc and fec capability drm/msm/dp: add dsc factor into calculation of supported bpp drm/msm/dp: add configure mainlink_levels base on lane number drm/msm/dp: correct configure Colorimetry Indicator Field at MISC0 drm/msm/dp: upgrade tu calculation base on newest algorithm drm/msm/dp: add display compression related struct drm/msm/dp: add dsc helper functions drm/msm/dp: add dsc supporting functions to DP controller drm/msm/dsi: export struct msm_compression_info to dpu encoder drm/msm/disp/dpu: add supports of DSC encoder v1.2 engine drm/msm/disp/dpu1: add supports of new flush mechanism drm/msm/disp/dpu1: revise timing engine programming to work for DSC drm/msm/disp/dpu1: add dsc supporting functions to dpu encoder drm/msm/disp/dpu1: add sc7280 dsc block and sub block drivers/gpu/drm/msm/Makefile | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c | 537 + drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h | 25 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 341 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 7 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 43 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 50 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 74 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 43 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 21 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 23 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 23 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 371 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 132 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h| 3 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 6 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 10 +- drivers/gpu/drm/msm/dp/dp_catalog.c| 176 - drivers/gpu/drm/msm/dp/dp_catalog.h| 97 ++- drivers/gpu/drm/msm/dp/dp_ctrl.c | 839 ++--- drivers/gpu/drm/msm/dp/dp_display.c| 61 +- drivers/gpu/drm/msm/dp/dp_link.c | 29 +- drivers/gpu/drm/msm/dp/dp_panel.c | 749 +- drivers/gpu/drm/msm/dp/dp_panel.h | 67 +- drivers/gpu/drm/msm/dp/dp_reg.h| 40 +- drivers/gpu/drm/msm/dsi/dsi.c | 3 +- drivers/gpu/drm/msm/dsi/dsi.h | 3 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 14 +- drivers/gpu/drm/msm/msm_drv.h | 113 ++- 32 files changed, 3429 insertions(+), 497 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
Re: [PATCH v1 00/14] add display port DSC feature
On 23/01/2023 20:24, Kuogee Hsieh wrote: This patch add DSC related supporting functions into to both dp controller and dpu enccoder Kuogee Hsieh (14): drm/msm/dp: add dpcd read of both dsc and fec capability drm/msm/dp: add dsc factor into calculation of supported bpp drm/msm/dp: add configure mainlink_levels base on lane number drm/msm/dp: correct configure Colorimetry Indicator Field at MISC0 drm/msm/dp: upgrade tu calculation base on newest algorithm drm/msm/dp: add display compression related struct drm/msm/dp: add dsc helper functions drm/msm/dp: add dsc supporting functions to DP controller drm/msm/dsi: export struct msm_compression_info to dpu encoder drm/msm/disp/dpu: add supports of DSC encoder v1.2 engine drm/msm/disp/dpu1: add supports of new flush mechanism drm/msm/disp/dpu1: revise timing engine programming to work for DSC drm/msm/disp/dpu1: add dsc supporting functions to dpu encoder drm/msm/disp/dpu1: add sc7280 dsc block and sub block Some generic notes regarding the series. I understand that the the series is complex, but following points might ease both your work and the review proces. First, atomicity. If your commit message says 'do this and that', it is highly likely that the patch should be split into smaller parts. Second, please pay attention to the history. If some part of the code or the data structure was removed, you have to justify bringing it back. This is extremely important in your case, as significant parts of the code come from the vendor code, thut it is easy to step on the same rake again. And if the previous removal was incorrect, please describe why. If we went through 10 revisions of a patch a year ago, it's not worth sending again a patch that closely remedies one of early iterations. It doesn't stand a chance of getting through. Next. Obvious item. ./scripts/checkpatch.pl should be your friend. It is not. Last, but not least. Please follow the mailing list. Less than a week ago one of reviews pointed out that commit messages like 'this patch does this and that' are not really welcomed. By sending the same kind of commit messages, you stand a high chance of receiveing the same response. Please go through the recommendations in Documentation/process/submitting-patches.rst. drivers/gpu/drm/msm/Makefile | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c | 537 + drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h | 25 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 341 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 7 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 43 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 50 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 74 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 43 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 21 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 23 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 23 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 371 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 132 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h| 3 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 6 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 10 +- drivers/gpu/drm/msm/dp/dp_catalog.c| 176 - drivers/gpu/drm/msm/dp/dp_catalog.h| 97 ++- drivers/gpu/drm/msm/dp/dp_ctrl.c | 839 ++--- drivers/gpu/drm/msm/dp/dp_display.c| 61 +- drivers/gpu/drm/msm/dp/dp_link.c | 29 +- drivers/gpu/drm/msm/dp/dp_panel.c | 749 +- drivers/gpu/drm/msm/dp/dp_panel.h | 67 +- drivers/gpu/drm/msm/dp/dp_reg.h| 40 +- drivers/gpu/drm/msm/dsi/dsi.c | 3 +- drivers/gpu/drm/msm/dsi/dsi.h | 3 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 14 +- drivers/gpu/drm/msm/msm_drv.h | 113 ++- 32 files changed, 3429 insertions(+), 497 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c -- With best wishes Dmitry
Re: [PATCH v1 00/14] add display port DSC feature
DisplayPort is a name, and I think you should spell it as such in both the cover letter title and individual patch descriptions (capital D and P, no space in between). On 2023-01-23 10:24:20, Kuogee Hsieh wrote: > This patch add DSC related supporting functions into to both dp controller > and dpu enccoder > > Kuogee Hsieh (14): > drm/msm/dp: add dpcd read of both dsc and fec capability > drm/msm/dp: add dsc factor into calculation of supported bpp > drm/msm/dp: add configure mainlink_levels base on lane number > drm/msm/dp: correct configure Colorimetry Indicator Field at MISC0 > drm/msm/dp: upgrade tu calculation base on newest algorithm > drm/msm/dp: add display compression related struct > drm/msm/dp: add dsc helper functions > drm/msm/dp: add dsc supporting functions to DP controller > drm/msm/dsi: export struct msm_compression_info to dpu encoder > drm/msm/disp/dpu: add supports of DSC encoder v1.2 engine > drm/msm/disp/dpu1: add supports of new flush mechanism > drm/msm/disp/dpu1: revise timing engine programming to work for DSC > drm/msm/disp/dpu1: add dsc supporting functions to dpu encoder > drm/msm/disp/dpu1: add sc7280 dsc block and sub block For DSC, capitalize it everywhere instead of the current free-form lower and uppercase mixup in patch titles. Still asking around for the subsystem tag, I've seen: drm/msm/dpu drm/msm/dpu1 drm/msm/disp/dpu drm/msm/disp/dpu1 And you're already mixing two of them. Aside that, thanks for sending this series! Been looking forward to DSC 1.2 for a while, but for DSI! - Marijn > drivers/gpu/drm/msm/Makefile | 2 + > drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c | 537 + > drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h | 25 + > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 341 +++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 4 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 7 +- > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 43 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 50 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 74 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 43 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 21 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 23 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 23 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 371 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 132 ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 10 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h| 3 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 6 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 10 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 10 +- > drivers/gpu/drm/msm/dp/dp_catalog.c| 176 - > drivers/gpu/drm/msm/dp/dp_catalog.h| 97 ++- > drivers/gpu/drm/msm/dp/dp_ctrl.c | 839 > ++--- > drivers/gpu/drm/msm/dp/dp_display.c| 61 +- > drivers/gpu/drm/msm/dp/dp_link.c | 29 +- > drivers/gpu/drm/msm/dp/dp_panel.c | 749 +- > drivers/gpu/drm/msm/dp/dp_panel.h | 67 +- > drivers/gpu/drm/msm/dp/dp_reg.h| 40 +- > drivers/gpu/drm/msm/dsi/dsi.c | 3 +- > drivers/gpu/drm/msm/dsi/dsi.h | 3 +- > drivers/gpu/drm/msm/dsi/dsi_host.c | 14 +- > drivers/gpu/drm/msm/msm_drv.h | 113 ++- > 32 files changed, 3429 insertions(+), 497 deletions(-) > create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c > create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h > create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
[PATCH v1 00/14] add display port DSC feature
This patch add DSC related supporting functions into to both dp controller and dpu enccoder Kuogee Hsieh (14): drm/msm/dp: add dpcd read of both dsc and fec capability drm/msm/dp: add dsc factor into calculation of supported bpp drm/msm/dp: add configure mainlink_levels base on lane number drm/msm/dp: correct configure Colorimetry Indicator Field at MISC0 drm/msm/dp: upgrade tu calculation base on newest algorithm drm/msm/dp: add display compression related struct drm/msm/dp: add dsc helper functions drm/msm/dp: add dsc supporting functions to DP controller drm/msm/dsi: export struct msm_compression_info to dpu encoder drm/msm/disp/dpu: add supports of DSC encoder v1.2 engine drm/msm/disp/dpu1: add supports of new flush mechanism drm/msm/disp/dpu1: revise timing engine programming to work for DSC drm/msm/disp/dpu1: add dsc supporting functions to dpu encoder drm/msm/disp/dpu1: add sc7280 dsc block and sub block drivers/gpu/drm/msm/Makefile | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c | 537 + drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h | 25 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 341 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 7 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 43 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 50 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 74 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 43 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 21 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 23 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 23 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 371 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 132 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h| 3 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 6 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 10 +- drivers/gpu/drm/msm/dp/dp_catalog.c| 176 - drivers/gpu/drm/msm/dp/dp_catalog.h| 97 ++- drivers/gpu/drm/msm/dp/dp_ctrl.c | 839 ++--- drivers/gpu/drm/msm/dp/dp_display.c| 61 +- drivers/gpu/drm/msm/dp/dp_link.c | 29 +- drivers/gpu/drm/msm/dp/dp_panel.c | 749 +- drivers/gpu/drm/msm/dp/dp_panel.h | 67 +- drivers/gpu/drm/msm/dp/dp_reg.h| 40 +- drivers/gpu/drm/msm/dsi/dsi.c | 3 +- drivers/gpu/drm/msm/dsi/dsi.h | 3 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 14 +- drivers/gpu/drm/msm/msm_drv.h | 113 ++- 32 files changed, 3429 insertions(+), 497 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
