Hi, On Wed, 5 May 2021 at 11:17, Sankeerth Billakanti <sbillaka@xxxxxxxxxxxxxx> wrote: > > These patches add support for the next generation eDP driver on SnapDragon > with dpu support. The existing eDP driver cannot support the new eDP > hardware. So, to maintain backward compatibility, the older eDP driver is > moved to v200 folder and the new generation eDP driver is added in > the v510 folder. What exactly does this version correspond to? I assume that v510 corresponds to sdmshrike/sc8180x. Is it right? Is it really so specific, or just v2/v5 would be enough? Not to mention that this is the MDP/ version, while other blocks tend to use block-specific versions/ids. Also, how much does it differ from the current DP core supported via drivers/gpu/drm/msm/dp ? First two patches did not make it to the linux-msm, so I can not comment on each of the lines. However just my few cents (other reviewers might disagree though): - I see little benefit in renaming the folders just for the sake of renaming. You can put your code in drivers/gpu/drm/msm/edp-v510, if you really insist on that. Note that for all other (even incompatible) hardware types we still use single level of folders. - Also I see that significant parts of code (e.g. AUX, bridge, connector, maybe more) are just c&p of old edp code pieces. Please share the code instead of duplicating it. - Please consider updating register definitions in xml form and then providing both changed xml files (to mesa project (?)) and generated headers into the kernel. - Please consider using clk_bulk_* functions instead of using dss_module_power. I'm going to send a patchset reworking current users to use the generic clk_bulk_* function family. - In generic, this eDP clock handling seems to match closely DP clocks handling (with all the name comparison, etc). Consider moving this to a generic piece of code - PHY seems to be a version of QMP PHY. Please use it, like it was done for the DP itself. There is support for combined USB+DP PHYs (both v3 and v4), so it should be possible to extend that for eDP. > These are baseline changes with which we can enable display. The new eDP > controller can also support additional features such as backlight control, > PSR etc. which will be enabled in subsequent patch series. > > Summary of changes: > DPU driver interface to the new eDP v510 display driver. > New generation eDP controller and phy driver implementation. > A common interface to choose enable the required eDP driver. > > Sankeerth Billakanti (3): > drm/msm/edp: support multiple generations of edp hardware > drm/msm/edp: add support for next gen edp > drm/msm/disp/dpu1: add support for edp encoder > > drivers/gpu/drm/msm/Makefile | 19 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 33 + > drivers/gpu/drm/msm/edp/edp.c | 198 --- > drivers/gpu/drm/msm/edp/edp.h | 78 - > drivers/gpu/drm/msm/edp/edp.xml.h | 380 ----- > drivers/gpu/drm/msm/edp/edp_aux.c | 264 ---- > drivers/gpu/drm/msm/edp/edp_bridge.c | 111 -- > drivers/gpu/drm/msm/edp/edp_common.c | 38 + > drivers/gpu/drm/msm/edp/edp_common.h | 47 + > drivers/gpu/drm/msm/edp/edp_connector.c | 132 -- > drivers/gpu/drm/msm/edp/edp_ctrl.c | 1375 ------------------ > drivers/gpu/drm/msm/edp/edp_phy.c | 98 -- > drivers/gpu/drm/msm/edp/v200/edp.xml.h | 380 +++++ > drivers/gpu/drm/msm/edp/v200/edp_v200.c | 210 +++ > drivers/gpu/drm/msm/edp/v200/edp_v200.h | 70 + > drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c | 264 ++++ > drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c | 111 ++ > drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c | 132 ++ > drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c | 1375 ++++++++++++++++++ > drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c | 98 ++ > drivers/gpu/drm/msm/edp/v510/edp_v510.c | 220 +++ > drivers/gpu/drm/msm/edp/v510/edp_v510.h | 151 ++ > drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c | 268 ++++ > drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c | 111 ++ > drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c | 117 ++ > drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c | 1583 +++++++++++++++++++++ > drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c | 641 +++++++++ > drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h | 339 +++++ > 29 files changed, 6207 insertions(+), 2643 deletions(-) > delete mode 100644 drivers/gpu/drm/msm/edp/edp.c > delete mode 100644 drivers/gpu/drm/msm/edp/edp.h > delete mode 100644 drivers/gpu/drm/msm/edp/edp.xml.h > delete mode 100644 drivers/gpu/drm/msm/edp/edp_aux.c > delete mode 100644 drivers/gpu/drm/msm/edp/edp_bridge.c > create mode 100644 drivers/gpu/drm/msm/edp/edp_common.c > create mode 100644 drivers/gpu/drm/msm/edp/edp_common.h > delete mode 100644 drivers/gpu/drm/msm/edp/edp_connector.c > delete mode 100644 drivers/gpu/drm/msm/edp/edp_ctrl.c > delete mode 100644 drivers/gpu/drm/msm/edp/edp_phy.c > create mode 100644 drivers/gpu/drm/msm/edp/v200/edp.xml.h > create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.c > create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200.h > create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_aux.c > create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_bridge.c > create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_connector.c > create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_ctrl.c > create mode 100644 drivers/gpu/drm/msm/edp/v200/edp_v200_phy.c > create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.c > create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510.h > create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_aux.c > create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_bridge.c > create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_connector.c > create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_ctrl.c > create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_phy.c > create mode 100644 drivers/gpu/drm/msm/edp/v510/edp_v510_reg.h > > -- > The Qualcomm Innovatin Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project > -- With best wishes Dmitry
