| Message ID | 20190720134709.15186-1-noralf@tronnes.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/tinydrm: Move mipi_dbi | expand |
Hi Noralf. Good to see a long journey end with a very nice result. I only entered the DRM world for the last parts of the journey, but nevertheless impressed by all the nice refactoring done. On Sat, Jul 20, 2019 at 03:47:03PM +0200, Noralf Trønnes wrote: > This series ticks off the last tinydrm todo entry and moves out mipi_dbi > to be a core helper. > > It splits struct mipi_dbi into an interface part and a display pipeline > part (upload framebuffer over SPI). I also took the opportunity to > rename the ambiguous 'mipi' variable name to 'dbi'. This lines up with > the use of the 'dsi' variable name in the MIPI DSI helper. > > Note: > This depends on series: drm/tinydrm: Remove tinydrm.ko > > Series is also available here: > https://github.com/notro/linux/tree/move_mipi_dbi > > Noralf. > > Noralf Trønnes (6): > drm/tinydrm/mipi-dbi: Move cmdlock mutex init > drm/tinydrm: Rename variable mipi -> dbi > drm/tinydrm: Rename remaining variable mipi -> dbidev > drm/tinydrm: Split struct mipi_dbi in two > drm/tinydrm: Move mipi-dbi > MAINTAINERS: Remove tinydrm entry I have read all patches - looks good. In "Split struct mipi_dbi in two" the documentation for the struct members is moved inside the struct - this makes it more readbale and is good. This could have been an independent patch, or at least mentioned in the changelog. You can for all patches add: Reviewed-by: Sam Ravnborg <sam@ravnborg.org> Sam
Hi Sam, Den 20.07.2019 22.29, skrev Sam Ravnborg: > Hi Noralf. > > Good to see a long journey end with a very nice result. > I only entered the DRM world for the last parts of the journey, > but nevertheless impressed by all the nice refactoring done. > Thanks, it's taken a while, but now it's quite easy to write small DRM drivers. > On Sat, Jul 20, 2019 at 03:47:03PM +0200, Noralf Trønnes wrote: >> This series ticks off the last tinydrm todo entry and moves out mipi_dbi >> to be a core helper. >> >> It splits struct mipi_dbi into an interface part and a display pipeline >> part (upload framebuffer over SPI). I also took the opportunity to >> rename the ambiguous 'mipi' variable name to 'dbi'. This lines up with >> the use of the 'dsi' variable name in the MIPI DSI helper. >> >> Note: >> This depends on series: drm/tinydrm: Remove tinydrm.ko >> >> Series is also available here: >> https://github.com/notro/linux/tree/move_mipi_dbi >> >> Noralf. >> >> Noralf Trønnes (6): >> drm/tinydrm/mipi-dbi: Move cmdlock mutex init >> drm/tinydrm: Rename variable mipi -> dbi >> drm/tinydrm: Rename remaining variable mipi -> dbidev >> drm/tinydrm: Split struct mipi_dbi in two >> drm/tinydrm: Move mipi-dbi >> MAINTAINERS: Remove tinydrm entry > > I have read all patches - looks good. > In "Split struct mipi_dbi in two" the documentation for the struct > members is moved inside the struct - this makes it more readbale and is > good. > This could have been an independent patch, or at least mentioned in the > changelog. > > You can for all patches add: > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > The kbuild test robot alerted told me about some missing dependency so I'm spinning a new version. Thanks for the review Sam! Noralf. > Sam >
Noralf Trønnes <noralf@tronnes.org> writes: > This series ticks off the last tinydrm todo entry and moves out mipi_dbi > to be a core helper. > > It splits struct mipi_dbi into an interface part and a display pipeline > part (upload framebuffer over SPI). I also took the opportunity to > rename the ambiguous 'mipi' variable name to 'dbi'. This lines up with > the use of the 'dsi' variable name in the MIPI DSI helper. > > Note: > This depends on series: drm/tinydrm: Remove tinydrm.ko > > Series is also available here: > https://github.com/notro/linux/tree/move_mipi_dbi Congratulations on making it to this stage. This looks like a fine conclusion to tinydrm. Acked-by: Eric Anholt <eric@anholt.net>
Den 22.07.2019 20.06, skrev Eric Anholt: > Noralf Trønnes <noralf@tronnes.org> writes: > >> This series ticks off the last tinydrm todo entry and moves out mipi_dbi >> to be a core helper. >> >> It splits struct mipi_dbi into an interface part and a display pipeline >> part (upload framebuffer over SPI). I also took the opportunity to >> rename the ambiguous 'mipi' variable name to 'dbi'. This lines up with >> the use of the 'dsi' variable name in the MIPI DSI helper. >> >> Note: >> This depends on series: drm/tinydrm: Remove tinydrm.ko >> >> Series is also available here: >> https://github.com/notro/linux/tree/move_mipi_dbi > > Congratulations on making it to this stage. This looks like a fine > conclusion to tinydrm. > Thanks, it took a while, but really nice to finally get here. There are only two patches left to do now, one to remove the menuconfig so the drivers are visible by default and people can start putting their tiny drivers here, and secondly to change the folder name to 'tiny' as Daniel wants it. > Acked-by: Eric Anholt <eric@anholt.net> > Thanks, care to take a look at version 2 of the series? I had to add 3 patches to deal with a kconfig dependency that I had missed out on. Noralf.
On Mon, Jul 22, 2019 at 11:06:15AM -0700, Eric Anholt wrote: > Noralf Trønnes <noralf@tronnes.org> writes: > > > This series ticks off the last tinydrm todo entry and moves out mipi_dbi > > to be a core helper. > > > > It splits struct mipi_dbi into an interface part and a display pipeline > > part (upload framebuffer over SPI). I also took the opportunity to > > rename the ambiguous 'mipi' variable name to 'dbi'. This lines up with > > the use of the 'dsi' variable name in the MIPI DSI helper. > > > > Note: > > This depends on series: drm/tinydrm: Remove tinydrm.ko > > > > Series is also available here: > > https://github.com/notro/linux/tree/move_mipi_dbi > > Congratulations on making it to this stage. This looks like a fine > conclusion to tinydrm. > > Acked-by: Eric Anholt <eric@anholt.net> Yeah let me heap on the congrats too, this has ben a long but really impressive journey to watch! -Daniel
Den 23.07.2019 09.10, skrev Daniel Vetter: > On Mon, Jul 22, 2019 at 11:06:15AM -0700, Eric Anholt wrote: >> Noralf Trønnes <noralf@tronnes.org> writes: >> >>> This series ticks off the last tinydrm todo entry and moves out mipi_dbi >>> to be a core helper. >>> >>> It splits struct mipi_dbi into an interface part and a display pipeline >>> part (upload framebuffer over SPI). I also took the opportunity to >>> rename the ambiguous 'mipi' variable name to 'dbi'. This lines up with >>> the use of the 'dsi' variable name in the MIPI DSI helper. >>> >>> Note: >>> This depends on series: drm/tinydrm: Remove tinydrm.ko >>> >>> Series is also available here: >>> https://github.com/notro/linux/tree/move_mipi_dbi >> >> Congratulations on making it to this stage. This looks like a fine >> conclusion to tinydrm. >> >> Acked-by: Eric Anholt <eric@anholt.net> > > Yeah let me heap on the congrats too, this has ben a long but really > impressive journey to watch! Looking back it's suprising to me that I continued to work on this. After Linus took a dump on tinydrm I wasn't much interested in doing any further work on Linux, there are lots of other interesting projects to work on. Then you cc'ed me on a patch telling me that someone was using the simple display helper I made, and before I knew it I was knee deep in refactoring work. One big take away for me has been how much better my code becomes after going through a review process. Some times, looking back, I wonder - did I actually write that nice piece of code? And the real answer I guess is that I did the typing and often had the idea, but many times the fleshed out solution was not something that I came up with. Noralf. > -Daniel >
On Tue, Jul 23, 2019 at 03:41:11PM +0200, Noralf Trønnes wrote: > Den 23.07.2019 09.10, skrev Daniel Vetter: > > On Mon, Jul 22, 2019 at 11:06:15AM -0700, Eric Anholt wrote: > >> Noralf Trønnes <noralf@tronnes.org> writes: > >> > >>> This series ticks off the last tinydrm todo entry and moves out mipi_dbi > >>> to be a core helper. > >>> > >>> It splits struct mipi_dbi into an interface part and a display pipeline > >>> part (upload framebuffer over SPI). I also took the opportunity to > >>> rename the ambiguous 'mipi' variable name to 'dbi'. This lines up with > >>> the use of the 'dsi' variable name in the MIPI DSI helper. > >>> > >>> Note: > >>> This depends on series: drm/tinydrm: Remove tinydrm.ko > >>> > >>> Series is also available here: > >>> https://github.com/notro/linux/tree/move_mipi_dbi > >> > >> Congratulations on making it to this stage. This looks like a fine > >> conclusion to tinydrm. > >> > >> Acked-by: Eric Anholt <eric@anholt.net> > > > > Yeah let me heap on the congrats too, this has ben a long but really > > impressive journey to watch! > > Looking back it's suprising to me that I continued to work on this. > After Linus took a dump on tinydrm I wasn't much interested in doing any > further work on Linux, there are lots of other interesting projects to > work on. Then you cc'ed me on a patch telling me that someone was using > the simple display helper I made, and before I knew it I was knee deep > in refactoring work. Historical aside: That dump from Linus was one of the reasons we finally decided to enact a code of conduct and enforce it against everyone. We just can't afford to lose great people just because some big guns have a hard time controlling their temper. Aside from it's just nicer to work together with some respect. > One big take away for me has been how much better my code becomes after > going through a review process. Some times, looking back, I wonder - did > I actually write that nice piece of code? And the real answer I guess is > that I did the typing and often had the idea, but many times the fleshed > out solution was not something that I came up with. For me great review (both for as reviewer and as author) is when everyone learns something. Personally I definitely learned a lot from all the small driver work that's been going on the past few years - completely different world compared to drivers mearsured in 100kloc units :-) Cheers, Daniel
This series ticks off the last tinydrm todo entry and moves out mipi_dbi to be a core helper. It splits struct mipi_dbi into an interface part and a display pipeline part (upload framebuffer over SPI). I also took the opportunity to rename the ambiguous 'mipi' variable name to 'dbi'. This lines up with the use of the 'dsi' variable name in the MIPI DSI helper. Note: This depends on series: drm/tinydrm: Remove tinydrm.ko Series is also available here: https://github.com/notro/linux/tree/move_mipi_dbi Noralf. Noralf Trønnes (6): drm/tinydrm/mipi-dbi: Move cmdlock mutex init drm/tinydrm: Rename variable mipi -> dbi drm/tinydrm: Rename remaining variable mipi -> dbidev drm/tinydrm: Split struct mipi_dbi in two drm/tinydrm: Move mipi-dbi MAINTAINERS: Remove tinydrm entry Documentation/gpu/drivers.rst | 1 - Documentation/gpu/drm-kms-helpers.rst | 12 + Documentation/gpu/tinydrm.rst | 18 - Documentation/gpu/todo.rst | 13 - MAINTAINERS | 13 +- drivers/gpu/drm/Kconfig | 4 + drivers/gpu/drm/Makefile | 1 + .../{tinydrm/mipi-dbi.c => drm_mipi_dbi.c} | 324 +++++++++--------- drivers/gpu/drm/tinydrm/Kconfig | 15 +- drivers/gpu/drm/tinydrm/Makefile | 4 - drivers/gpu/drm/tinydrm/hx8357d.c | 61 ++-- drivers/gpu/drm/tinydrm/ili9225.c | 171 ++++----- drivers/gpu/drm/tinydrm/ili9341.c | 83 ++--- drivers/gpu/drm/tinydrm/mi0283qt.c | 89 ++--- drivers/gpu/drm/tinydrm/st7586.c | 104 +++--- drivers/gpu/drm/tinydrm/st7735r.c | 77 +++-- include/drm/drm_mipi_dbi.h | 188 ++++++++++ include/drm/tinydrm/mipi-dbi.h | 135 -------- 18 files changed, 682 insertions(+), 631 deletions(-) delete mode 100644 Documentation/gpu/tinydrm.rst rename drivers/gpu/drm/{tinydrm/mipi-dbi.c => drm_mipi_dbi.c} (78%) create mode 100644 include/drm/drm_mipi_dbi.h delete mode 100644 include/drm/tinydrm/mipi-dbi.h