Hi,

On Sat, Oct 15, 2022 at 05:04:50PM +0200, Noralf Trønnes wrote:

Den 13.10.2022 10.36, skrev Maxime Ripard:

quoted

On Sat, Oct 01, 2022 at 02:52:06PM +0200, Noralf Trønnes wrote:

quoted

Den 29.09.2022 18.31, skrev Maxime Ripard:

quoted

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 <redacted>

---

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@mail.gmail.com/ (local)

If that's the case I suggest you just hardcode them for now and leave
the complexity to the developer doing the actual conversion of the fbdev
driver. Who knows when that will happen, but that person will have your
well documented and discussed work to fall back on.

I'd rather not, tbh. We've collectively spent weeks figuring this out,
reviewing it and so on, I very much want to avoid doing this all over
again if it's going to be useful at some point.

Jani also wanted to expose a function and not a raw mode, so this patch
also addresses that:
https://lore.kernel.org/dri-devel/8735eeg31e.fsf@intel.com/ (local)

Maxime

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel