Hi Noralf, On Sat, Oct 01, 2022 at 02:52:06PM +0200, Noralf Trønnes wrote: > Den 29.09.2022 18.31, skrev Maxime Ripard: > > Multiple drivers (meson, vc4, sun4i) define analog TV 525-lines and > > 625-lines modes in their drivers. > > > > Since those modes are fairly standard, and that we'll need to use them > > in more places in the future, it makes sense to move their definition > > into the core framework. > > > > However, analog display usually have fairly loose timings requirements, > > the only discrete parameters being the total number of lines and pixel > > clock frequency. Thus, we created a function that will create a display > > mode from the standard, the pixel frequency and the active area. > > > > Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx> > > > > --- > > > > Changes in v4: > > - Reworded the line length check comment > > - Switch to HZ_PER_KHZ in tests > > - Use previous timing to fill our mode > > - Move the number of lines check earlier > > --- > > drivers/gpu/drm/drm_modes.c | 474 +++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/tests/Makefile | 1 + > > drivers/gpu/drm/tests/drm_modes_test.c | 144 ++++++++++ > > include/drm/drm_modes.h | 17 ++ > > 4 files changed, 636 insertions(+) > > > > I haven't followed the discussion on this patch, but it seems rather > excessive to add over 600 lines of code (including tests) to add 2 fixed > modes. And it's very difficult to see from the code what the actual > display mode timings really are, which would be useful for other > developers down the road wanting to use them. > > Why not just hardcode the modes? Yeah, I have kind of the same feeling tbh, but it was asked back on the v1 to ease the transition of old fbdev drivers, since they will need such a function: https://lore.kernel.org/dri-devel/CAMuHMdUrwzPYjA0wdR7ADj5Ov6+m03JbnY8fBYzRYyWDuNm5=g@xxxxxxxxxxxxxx/ Maxime
Attachment:
signature.asc
Description: PGP signature
