On Thu, 01 Aug 2024, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
> For cloned outputs, don't pick a default resolution of 1024x768 as
> most hardware can do better. Instead look through the modes of all
> connectors to find a common mode for all of them.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> ---
> drivers/gpu/drm/drm_client_modeset.c | 54 +++++++++++++++++-----------
> 1 file changed, 34 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 31af5cf37a09..67b422dc8e7f 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -266,7 +266,7 @@ static bool drm_client_target_cloned(struct drm_device *dev,
> {
> int count, i, j;
> bool can_clone = false;
> - struct drm_display_mode *dmt_mode, *mode;
> + struct drm_display_mode *mode, *common_mode = NULL;
>
> /* only contemplate cloning in the single crtc case */
> if (dev->mode_config.num_crtc > 1)
> @@ -309,35 +309,49 @@ static bool drm_client_target_cloned(struct drm_device *dev,
> return true;
> }
>
> - /* try and find a 1024x768 mode on each connector */
> - can_clone = true;
> - dmt_mode = drm_mode_find_dmt(dev, 1024, 768, 60, false);
> -
> - if (!dmt_mode)
> - goto fail;
> + /* try and find a mode common among connectors */
>
> + can_clone = false;
> for (i = 0; i < connector_count; i++) {
> if (!enabled[i])
> continue;
>
> - list_for_each_entry(mode, &connectors[i]->modes, head) {
> - if (drm_mode_match(mode, dmt_mode,
> - DRM_MODE_MATCH_TIMINGS |
> - DRM_MODE_MATCH_CLOCK |
> - DRM_MODE_MATCH_FLAGS |
> - DRM_MODE_MATCH_3D_FLAGS))
> - modes[i] = mode;
> + list_for_each_entry(common_mode, &connectors[i]->modes, head) {
> + can_clone = true;
> +
> + for (j = 1; j < connector_count; j++) {
Should this start from i instead of 1?
Anyway, I have a hard time wrapping my head around this whole thing. I
think it would greatly benefit from a helper function to search for a
mode from an array of connectors.
BR,
Jani.
> + if (!enabled[i])
> + continue;
> +
> + can_clone = false;
> + list_for_each_entry(mode, &connectors[j]->modes, head) {
> + can_clone = drm_mode_match(common_mode, mode,
> + DRM_MODE_MATCH_TIMINGS |
> + DRM_MODE_MATCH_CLOCK |
> + DRM_MODE_MATCH_FLAGS |
> + DRM_MODE_MATCH_3D_FLAGS);
> + if (can_clone)
> + break; // found common mode on connector
> + }
> + if (!can_clone)
> + break; // try next common mode
> + }
> + if (can_clone)
> + break; // found common mode among all connectors
> }
> - if (!modes[i])
> - can_clone = false;
> + break;
> }
> - kfree(dmt_mode);
> -
> if (can_clone) {
> - drm_dbg_kms(dev, "can clone using 1024x768\n");
> + for (i = 0; i < connector_count; i++) {
> + if (!enabled[i])
> + continue;
> + modes[i] = common_mode;
> +
> + }
> + drm_dbg_kms(dev, "can clone using" DRM_MODE_FMT "\n", DRM_MODE_ARG(common_mode));
> return true;
> }
> -fail:
> +
> drm_info(dev, "kms: can't enable cloning when we probably wanted to.\n");
> return false;
> }
--
Jani Nikula, Intel
