| Message ID | 20240501154251.3302887-1-dianders@chromium.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/mipi-dsi: Reduce bloat and add funcs for cleaner init seqs | expand |
Hi, On 01/05/2024 17:41, Douglas Anderson wrote: > The consensus of many DRM folks is that we want to move away from DSI > drivers defining tables of init commands. Instead, we want to move to > init functions that can use common DRM functions. The issue thus far > has been that using the macros mipi_dsi_generic_write_seq() and > mipi_dsi_dcs_write_seq() bloats the driver using them. > > While trying to solve bloat, we realized that the majority of the it > was easy to solve. This series solves the bloat for existing drivers > by moving the printout outside of the macro. > > During discussion of my v1 patch to fix the bloat [1], we also decided > that we really want to change the way that drivers deal with init > sequences to make it clearer. In addition to being cleaner, a side > effect of moving drivers to the new style reduces bloat _even more_. > > This series also contains a few minor fixes / cleanups that I found > along the way. > > This series converts four drivers over to the new > mipi_dsi_dcs_write_seq_multi() function. Not all conversions have been > tested, but hopefully they are straightforward enough. I'd appreciate > testing. > > NOTE: In v3 I tried to incorporate the feedback from v2. I also > converted the other two panels I could find that used table-based > initialization. > > [1] https://lore.kernel.org/r/20240424172017.1.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid > > Changes in v3: > - ("mipi_dsi_*_write functions don't need to ratelimit...") moved earlier. > - Add a TODO item for cleaning up the deprecated macros/functions. > - Fix spacing of init function. > - Inline kerneldoc comments for struct mipi_dsi_multi_context. > - Rebased upon patch to remove ratelimit of prints. > - Remove an unneeded error print. > - Squash boe-tv101wum-nl6 lowercase patch into main patch > - Use %zd in print instead of casting errors to int. > - drm/panel: ili9882t: Don't use a table for initting panels > - drm/panel: innolux-p079zca: Don't use a table for initting panels > > Changes in v2: > - Add some comments to the macros about printing and returning. > - Change the way err value is handled in prep for next patch. > - Modify commit message now that this is part of a series. > - Rebased upon patches to avoid theoretical int overflow. > - drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_dcs_write_seq() > - drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_generic_write_seq() > - drm/mipi-dsi: Introduce mipi_dsi_*_write_seq_multi() > - drm/mipi-dsi: mipi_dsi_*_write functions don't need to ratelimit prints > - drm/panel: boe-tv101wum-nl6: Convert hex to lowercase > - drm/panel: boe-tv101wum-nl6: Don't use a table for initting commands > - drm/panel: novatek-nt36672e: Switch to mipi_dsi_dcs_write_seq_multi() > > Douglas Anderson (9): > drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_dcs_write_seq() > drm/mipi-dsi: Fix theoretical int overflow in > mipi_dsi_generic_write_seq() > drm/mipi-dsi: mipi_dsi_*_write functions don't need to ratelimit > prints > drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq() > drm/mipi-dsi: Introduce mipi_dsi_*_write_seq_multi() > drm/panel: novatek-nt36672e: Switch to mipi_dsi_dcs_write_seq_multi() > drm/panel: boe-tv101wum-nl6: Don't use a table for initting panels > drm/panel: ili9882t: Don't use a table for initting panels > drm/panel: innolux-p079zca: Don't use a table for initting panels Thanks Doug! I think we all agree on the core changes, now I think we can wait a few weeks and try to get some test feedbacks on the indirectly and directly affected panels, drm-misc-next won't be merged into linux-next until v6.10-rc1 anyway so we have some time to test on our boards. Neil > > Documentation/gpu/todo.rst | 18 + > drivers/gpu/drm/drm_mipi_dsi.c | 112 + > .../gpu/drm/panel/panel-boe-tv101wum-nl6.c | 2792 +++++++++-------- > drivers/gpu/drm/panel/panel-ilitek-ili9882t.c | 794 +++-- > drivers/gpu/drm/panel/panel-innolux-p079zca.c | 284 +- > .../gpu/drm/panel/panel-novatek-nt36672e.c | 576 ++-- > include/drm/drm_mipi_dsi.h | 101 +- > 7 files changed, 2451 insertions(+), 2226 deletions(-) >
Hi, On Thu, May 2, 2024 at 12:48 AM Neil Armstrong <neil.armstrong@linaro.org> wrote: > > Hi, > > On 01/05/2024 17:41, Douglas Anderson wrote: > > The consensus of many DRM folks is that we want to move away from DSI > > drivers defining tables of init commands. Instead, we want to move to > > init functions that can use common DRM functions. The issue thus far > > has been that using the macros mipi_dsi_generic_write_seq() and > > mipi_dsi_dcs_write_seq() bloats the driver using them. > > > > While trying to solve bloat, we realized that the majority of the it > > was easy to solve. This series solves the bloat for existing drivers > > by moving the printout outside of the macro. > > > > During discussion of my v1 patch to fix the bloat [1], we also decided > > that we really want to change the way that drivers deal with init > > sequences to make it clearer. In addition to being cleaner, a side > > effect of moving drivers to the new style reduces bloat _even more_. > > > > This series also contains a few minor fixes / cleanups that I found > > along the way. > > > > This series converts four drivers over to the new > > mipi_dsi_dcs_write_seq_multi() function. Not all conversions have been > > tested, but hopefully they are straightforward enough. I'd appreciate > > testing. > > > > NOTE: In v3 I tried to incorporate the feedback from v2. I also > > converted the other two panels I could find that used table-based > > initialization. > > > > [1] https://lore.kernel.org/r/20240424172017.1.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid > > > > Changes in v3: > > - ("mipi_dsi_*_write functions don't need to ratelimit...") moved earlier. > > - Add a TODO item for cleaning up the deprecated macros/functions. > > - Fix spacing of init function. > > - Inline kerneldoc comments for struct mipi_dsi_multi_context. > > - Rebased upon patch to remove ratelimit of prints. > > - Remove an unneeded error print. > > - Squash boe-tv101wum-nl6 lowercase patch into main patch > > - Use %zd in print instead of casting errors to int. > > - drm/panel: ili9882t: Don't use a table for initting panels > > - drm/panel: innolux-p079zca: Don't use a table for initting panels > > > > Changes in v2: > > - Add some comments to the macros about printing and returning. > > - Change the way err value is handled in prep for next patch. > > - Modify commit message now that this is part of a series. > > - Rebased upon patches to avoid theoretical int overflow. > > - drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_dcs_write_seq() > > - drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_generic_write_seq() > > - drm/mipi-dsi: Introduce mipi_dsi_*_write_seq_multi() > > - drm/mipi-dsi: mipi_dsi_*_write functions don't need to ratelimit prints > > - drm/panel: boe-tv101wum-nl6: Convert hex to lowercase > > - drm/panel: boe-tv101wum-nl6: Don't use a table for initting commands > > - drm/panel: novatek-nt36672e: Switch to mipi_dsi_dcs_write_seq_multi() > > > > Douglas Anderson (9): > > drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_dcs_write_seq() > > drm/mipi-dsi: Fix theoretical int overflow in > > mipi_dsi_generic_write_seq() > > drm/mipi-dsi: mipi_dsi_*_write functions don't need to ratelimit > > prints > > drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq() > > drm/mipi-dsi: Introduce mipi_dsi_*_write_seq_multi() > > drm/panel: novatek-nt36672e: Switch to mipi_dsi_dcs_write_seq_multi() > > drm/panel: boe-tv101wum-nl6: Don't use a table for initting panels > > drm/panel: ili9882t: Don't use a table for initting panels > > drm/panel: innolux-p079zca: Don't use a table for initting panels > > Thanks Doug! > > I think we all agree on the core changes, now I think we can wait a few weeks > and try to get some test feedbacks on the indirectly and directly affected > panels, drm-misc-next won't be merged into linux-next until v6.10-rc1 anyway > so we have some time to test on our boards. Great! Just to be clear, are you suggesting that we leave these patches on the lists for a few weeks before landing in drm-misc-next, or are you suggesting that it's safe to land them in drm-misc-next because it won't make it to linuxnext for a while anyway? I assume the former (AKA leave it on the lists for a while) but just want to be clear. ;-) There's nothing terribly urgent about these patches except that they are blocking Cong Yang's patch series [0] and lvzhaoxiong's patch series [1]. I think it would be fine for them to send out their patch series with mine marked as a dependency so we can finish reviewing their series and then when mine lands theirs will be good to go too. Maybe we can aim for 2 weeks of stewing on the list if there are no issues during that time? I know landing in drm-misc during this time won't help get into mainline faster, but with ChromeOS's "upstream first" policy it saves us a bunch of headache if we can land things in our tree from a maintainer tree with stable git hashes (like drm-misc-next) instead of landing from a mailing list. Certainly that doesn't mean we want to rush patches in before they're ready, but I just want to say that there is still some benefit in getting the patches landed sooner rather than later. :-) [0] https://lore.kernel.org/r/20240424023010.2099949-1-yangcong5@huaqin.corp-partner.google.com [1] https://lore.kernel.org/r/20240418081548.12160-3-lvzhaoxiong@huaqin.corp-partner.google.com
On 02/05/2024 16:27, Doug Anderson wrote: > Hi, > > On Thu, May 2, 2024 at 12:48 AM Neil Armstrong > <neil.armstrong@linaro.org> wrote: >> >> Hi, >> >> On 01/05/2024 17:41, Douglas Anderson wrote: >>> The consensus of many DRM folks is that we want to move away from DSI >>> drivers defining tables of init commands. Instead, we want to move to >>> init functions that can use common DRM functions. The issue thus far >>> has been that using the macros mipi_dsi_generic_write_seq() and >>> mipi_dsi_dcs_write_seq() bloats the driver using them. >>> >>> While trying to solve bloat, we realized that the majority of the it >>> was easy to solve. This series solves the bloat for existing drivers >>> by moving the printout outside of the macro. >>> >>> During discussion of my v1 patch to fix the bloat [1], we also decided >>> that we really want to change the way that drivers deal with init >>> sequences to make it clearer. In addition to being cleaner, a side >>> effect of moving drivers to the new style reduces bloat _even more_. >>> >>> This series also contains a few minor fixes / cleanups that I found >>> along the way. >>> >>> This series converts four drivers over to the new >>> mipi_dsi_dcs_write_seq_multi() function. Not all conversions have been >>> tested, but hopefully they are straightforward enough. I'd appreciate >>> testing. >>> >>> NOTE: In v3 I tried to incorporate the feedback from v2. I also >>> converted the other two panels I could find that used table-based >>> initialization. >>> >>> [1] https://lore.kernel.org/r/20240424172017.1.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid >>> >>> Changes in v3: >>> - ("mipi_dsi_*_write functions don't need to ratelimit...") moved earlier. >>> - Add a TODO item for cleaning up the deprecated macros/functions. >>> - Fix spacing of init function. >>> - Inline kerneldoc comments for struct mipi_dsi_multi_context. >>> - Rebased upon patch to remove ratelimit of prints. >>> - Remove an unneeded error print. >>> - Squash boe-tv101wum-nl6 lowercase patch into main patch >>> - Use %zd in print instead of casting errors to int. >>> - drm/panel: ili9882t: Don't use a table for initting panels >>> - drm/panel: innolux-p079zca: Don't use a table for initting panels >>> >>> Changes in v2: >>> - Add some comments to the macros about printing and returning. >>> - Change the way err value is handled in prep for next patch. >>> - Modify commit message now that this is part of a series. >>> - Rebased upon patches to avoid theoretical int overflow. >>> - drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_dcs_write_seq() >>> - drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_generic_write_seq() >>> - drm/mipi-dsi: Introduce mipi_dsi_*_write_seq_multi() >>> - drm/mipi-dsi: mipi_dsi_*_write functions don't need to ratelimit prints >>> - drm/panel: boe-tv101wum-nl6: Convert hex to lowercase >>> - drm/panel: boe-tv101wum-nl6: Don't use a table for initting commands >>> - drm/panel: novatek-nt36672e: Switch to mipi_dsi_dcs_write_seq_multi() >>> >>> Douglas Anderson (9): >>> drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_dcs_write_seq() >>> drm/mipi-dsi: Fix theoretical int overflow in >>> mipi_dsi_generic_write_seq() >>> drm/mipi-dsi: mipi_dsi_*_write functions don't need to ratelimit >>> prints >>> drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq() >>> drm/mipi-dsi: Introduce mipi_dsi_*_write_seq_multi() >>> drm/panel: novatek-nt36672e: Switch to mipi_dsi_dcs_write_seq_multi() >>> drm/panel: boe-tv101wum-nl6: Don't use a table for initting panels >>> drm/panel: ili9882t: Don't use a table for initting panels >>> drm/panel: innolux-p079zca: Don't use a table for initting panels >> >> Thanks Doug! >> >> I think we all agree on the core changes, now I think we can wait a few weeks >> and try to get some test feedbacks on the indirectly and directly affected >> panels, drm-misc-next won't be merged into linux-next until v6.10-rc1 anyway >> so we have some time to test on our boards. > > Great! > > Just to be clear, are you suggesting that we leave these patches on > the lists for a few weeks before landing in drm-misc-next, or are you > suggesting that it's safe to land them in drm-misc-next because it > won't make it to linuxnext for a while anyway? I assume the former > (AKA leave it on the lists for a while) but just want to be clear. ;-) Yes you assume right > > There's nothing terribly urgent about these patches except that they > are blocking Cong Yang's patch series [0] and lvzhaoxiong's patch > series [1]. I think it would be fine for them to send out their patch > series with mine marked as a dependency so we can finish reviewing > their series and then when mine lands theirs will be good to go too. > > Maybe we can aim for 2 weeks of stewing on the list if there are no > issues during that time? I know landing in drm-misc during this time > won't help get into mainline faster, but with ChromeOS's "upstream > first" policy it saves us a bunch of headache if we can land things in > our tree from a maintainer tree with stable git hashes (like > drm-misc-next) instead of landing from a mailing list. Certainly that > doesn't mean we want to rush patches in before they're ready, but I > just want to say that there is still some benefit in getting the > patches landed sooner rather than later. :-) Yes let's wait 2 weeks and apply them Neil > > [0] https://lore.kernel.org/r/20240424023010.2099949-1-yangcong5@huaqin.corp-partner.google.com > [1] https://lore.kernel.org/r/20240418081548.12160-3-lvzhaoxiong@huaqin.corp-partner.google.com
The consensus of many DRM folks is that we want to move away from DSI drivers defining tables of init commands. Instead, we want to move to init functions that can use common DRM functions. The issue thus far has been that using the macros mipi_dsi_generic_write_seq() and mipi_dsi_dcs_write_seq() bloats the driver using them. While trying to solve bloat, we realized that the majority of the it was easy to solve. This series solves the bloat for existing drivers by moving the printout outside of the macro. During discussion of my v1 patch to fix the bloat [1], we also decided that we really want to change the way that drivers deal with init sequences to make it clearer. In addition to being cleaner, a side effect of moving drivers to the new style reduces bloat _even more_. This series also contains a few minor fixes / cleanups that I found along the way. This series converts four drivers over to the new mipi_dsi_dcs_write_seq_multi() function. Not all conversions have been tested, but hopefully they are straightforward enough. I'd appreciate testing. NOTE: In v3 I tried to incorporate the feedback from v2. I also converted the other two panels I could find that used table-based initialization. [1] https://lore.kernel.org/r/20240424172017.1.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid Changes in v3: - ("mipi_dsi_*_write functions don't need to ratelimit...") moved earlier. - Add a TODO item for cleaning up the deprecated macros/functions. - Fix spacing of init function. - Inline kerneldoc comments for struct mipi_dsi_multi_context. - Rebased upon patch to remove ratelimit of prints. - Remove an unneeded error print. - Squash boe-tv101wum-nl6 lowercase patch into main patch - Use %zd in print instead of casting errors to int. - drm/panel: ili9882t: Don't use a table for initting panels - drm/panel: innolux-p079zca: Don't use a table for initting panels Changes in v2: - Add some comments to the macros about printing and returning. - Change the way err value is handled in prep for next patch. - Modify commit message now that this is part of a series. - Rebased upon patches to avoid theoretical int overflow. - drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_dcs_write_seq() - drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_generic_write_seq() - drm/mipi-dsi: Introduce mipi_dsi_*_write_seq_multi() - drm/mipi-dsi: mipi_dsi_*_write functions don't need to ratelimit prints - drm/panel: boe-tv101wum-nl6: Convert hex to lowercase - drm/panel: boe-tv101wum-nl6: Don't use a table for initting commands - drm/panel: novatek-nt36672e: Switch to mipi_dsi_dcs_write_seq_multi() Douglas Anderson (9): drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_dcs_write_seq() drm/mipi-dsi: Fix theoretical int overflow in mipi_dsi_generic_write_seq() drm/mipi-dsi: mipi_dsi_*_write functions don't need to ratelimit prints drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq() drm/mipi-dsi: Introduce mipi_dsi_*_write_seq_multi() drm/panel: novatek-nt36672e: Switch to mipi_dsi_dcs_write_seq_multi() drm/panel: boe-tv101wum-nl6: Don't use a table for initting panels drm/panel: ili9882t: Don't use a table for initting panels drm/panel: innolux-p079zca: Don't use a table for initting panels Documentation/gpu/todo.rst | 18 + drivers/gpu/drm/drm_mipi_dsi.c | 112 + .../gpu/drm/panel/panel-boe-tv101wum-nl6.c | 2792 +++++++++-------- drivers/gpu/drm/panel/panel-ilitek-ili9882t.c | 794 +++-- drivers/gpu/drm/panel/panel-innolux-p079zca.c | 284 +- .../gpu/drm/panel/panel-novatek-nt36672e.c | 576 ++-- include/drm/drm_mipi_dsi.h | 101 +- 7 files changed, 2451 insertions(+), 2226 deletions(-)