From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC666C43334 for ; Fri, 1 Jul 2022 20:02:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231937AbiGAUCB (ORCPT ); Fri, 1 Jul 2022 16:02:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38978 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231985AbiGAUB5 (ORCPT ); Fri, 1 Jul 2022 16:01:57 -0400 Received: from mail-qv1-f42.google.com (mail-qv1-f42.google.com [209.85.219.42]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE2864F199 for ; Fri, 1 Jul 2022 13:01:42 -0700 (PDT) Received: by mail-qv1-f42.google.com with SMTP id cu16so4851610qvb.7 for ; Fri, 01 Jul 2022 13:01:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=p2stC06lnxIJ04peLci0uJckjsb+NhjEnb2d6jyIQDI=; b=CYzana6E3yLyvyx9gHa1zQA5qQrRGAnlkKzcSgSsKMlQvVY7xw0mFjLjZOx1APLVZH ggndnhflliqAywot74ZLIsjBomrlFLmOah3feoYFI2ehW8RYAvCrN+SAdK1c0qwsRcS3 M74Ju5VAsc5nRvqse5aN5fCzmCfwBxWaD8fPE8d1GVwjgtAFTDd0R49yXXOxZRG2lZpX qrhKicY0RgbgP/7bYb+NIofzOpu+3e3om9pH8lrfCrxcVzjom2O48pYbDxn8CcddZ9YB lO8UY368qNkcBZN2LpdUWKO0AiY3JkLkcvdg+GkgpwyHTCwtSk/ysc8xG6eidVMSluB8 Ly7w== X-Gm-Message-State: AJIora+yrX9pNrvdm6VXf/HeTjFy2o/rlN75GNWNCCohU6/45sCbJPjQ VsKxUKuXAIIFASQ+6g83jcC7Klkn6XrpHA== X-Google-Smtp-Source: AGRyM1ubvqoI8uShr/Vyf9aFLH4PXfBdp0XMnAfBl+S5SMsNTxwOHpWNNavNCmxBK3nQkHuB+X2GvA== X-Received: by 2002:a05:6214:965:b0:470:5190:3350 with SMTP id do5-20020a056214096500b0047051903350mr18695490qvb.53.1656705700844; Fri, 01 Jul 2022 13:01:40 -0700 (PDT) Received: from mail-yb1-f174.google.com (mail-yb1-f174.google.com. [209.85.219.174]) by smtp.gmail.com with ESMTPSA id v26-20020ac873da000000b00304dec6452csm15091931qtp.78.2022.07.01.13.01.39 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 01 Jul 2022 13:01:40 -0700 (PDT) Received: by mail-yb1-f174.google.com with SMTP id d5so5823915yba.5 for ; Fri, 01 Jul 2022 13:01:39 -0700 (PDT) X-Received: by 2002:a25:3288:0:b0:66c:8a91:74bb with SMTP id y130-20020a253288000000b0066c8a9174bbmr16964347yby.89.1656705699367; Fri, 01 Jul 2022 13:01:39 -0700 (PDT) MIME-Version: 1.0 References: <20191014140416.28517-1-tzimmermann@suse.de> <20191014140416.28517-10-tzimmermann@suse.de> <8b79b40b-9786-9ddf-0dc9-89efbab38c7f@suse.de> In-Reply-To: From: Geert Uytterhoeven Date: Fri, 1 Jul 2022 22:01:27 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 09/15] drm/fbconv: Mode-setting pipeline enable / disable To: Thomas Zimmermann Cc: David Airlie , Daniel Vetter , Maarten Lankhorst , Maxime Ripard , Sean Paul , ajax@redhat.com, =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Mathieu Malaterre , michel@daenzer.net, Jonathan Corbet , Greg KH , DRI Development , Linux Fbdev development list Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-fbdev@vger.kernel.org Hi Thomas, On Mon, May 30, 2022 at 10:34 AM Geert Uytterhoeven wrote: > On Mon, May 30, 2022 at 9:47 AM Thomas Zimmermann wrote: > > Am 28.05.22 um 22:17 schrieb Geert Uytterhoeven: > > > On Mon, Oct 14, 2019 at 4:05 PM Thomas Zimmermann wrote: > > >> The display mode is set by converting the DRM display mode to an > > >> fb_info state and handling it to the fbdev driver's fb_setvar() > > >> function. This also requires a color depth, which we take from the > > >> value of struct drm_mode_config.preferred_depth > > >> > > >> Signed-off-by: Thomas Zimmermann > > > > > >> --- a/drivers/gpu/drm/drm_fbconv_helper.c > > >> +++ b/drivers/gpu/drm/drm_fbconv_helper.c > > >> @@ -919,6 +919,24 @@ static int drm_fbconv_update_fb_var_screeninfo_from_framebuffer( > > >> return 0; > > >> } > > >> > > >> +static int drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( > > >> + struct fb_var_screeninfo *fb_var, struct drm_simple_display_pipe *pipe) > > >> +{ > > >> + struct drm_plane *primary = pipe->crtc.primary; > > >> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); > > >> + > > >> + if (primary && primary->state && primary->state->fb) > > >> + return drm_fbconv_update_fb_var_screeninfo_from_framebuffer( > > >> + fb_var, primary->state->fb, > > >> + modeset->fb_info->fix.smem_len); > > >> + > > >> + fb_var->xres_virtual = fb_var->xres; > > >> + fb_var->yres_virtual = fb_var->yres; > > >> + fb_var->bits_per_pixel = modeset->dev->mode_config.preferred_depth; > > > > > > This looks wrong to me: IMHO bits_per_pixel should be derived from > > > the fourcc format of the _new_ mode to be set... > > > > Indeed, this appears to be wrong. > > OK. > > > > > > > > >> + > > >> + return 0; > > >> +} > > >> + > > >> /** > > >> * drm_fbconv_simple_display_pipe_mode_valid - default implementation for > > >> * struct drm_simple_display_pipe_funcs.mode_valid > > >> @@ -950,6 +968,28 @@ bool drm_fbconv_simple_display_pipe_mode_fixup( > > >> struct drm_crtc *crtc, const struct drm_display_mode *mode, > > >> struct drm_display_mode *adjusted_mode) > > >> { > > >> + struct drm_simple_display_pipe *pipe = > > >> + container_of(crtc, struct drm_simple_display_pipe, crtc); > > >> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); > > >> + struct fb_var_screeninfo fb_var; > > >> + int ret; > > >> + > > >> + if (!modeset->fb_info->fbops->fb_check_var) > > >> + return true; > > >> + > > >> + drm_fbconv_init_fb_var_screeninfo_from_mode(&fb_var, mode); > > >> + > > >> + ret = drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( > > >> + &fb_var, &modeset->pipe); > > >> + if (ret) > > >> + return true; > > >> + > > >> + ret = modeset->fb_info->fbops->fb_check_var(&fb_var, modeset->fb_info); > > > > > > ... hence this fails if the requested mode is valid with the new > > > fourcc format, but invalid with the old (but preferred) depth. > > > E.g. due to bandwidth limitations, a high-resolution mode is valid > > > with a low color depth, while a high color depth is limited to lower > > > resolutions. > > > Unfortunately we do not know the new fourcc format here, as both > > > drm_simple_display_pipe_funcs.mode_{valid,fixup}() are passed > > > the mode (from drm_mode_set.mode), but not the new format (from > > > drm_mode_set.fb->format). > > > > > > Am I missing something? Is the new format available in some other way? > > > > We can always get the format from the new plane state of > > modeset->pipe->plane. We'd have this in the atomic_check call. And it > > appears that drm_fbconv_simple_display_pipe_check() is a better place > > for this code anyway. > > Thanks, I'll give that a try! Getting the format from the new plane state of pipe->plane doesn't work, as pipe->plane.state->fb = NULL. But it is indeed available in the drm_simple_display_pipe_funcs.check() callback, so that seems to work... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 434BCC433EF for ; Fri, 1 Jul 2022 20:01:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 51C0B12BDCC; Fri, 1 Jul 2022 20:01:42 +0000 (UTC) Received: from mail-qk1-f180.google.com (mail-qk1-f180.google.com [209.85.222.180]) by gabe.freedesktop.org (Postfix) with ESMTPS id DE24412BDCC for ; Fri, 1 Jul 2022 20:01:41 +0000 (UTC) Received: by mail-qk1-f180.google.com with SMTP id b125so2577299qkg.11 for ; Fri, 01 Jul 2022 13:01:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=p2stC06lnxIJ04peLci0uJckjsb+NhjEnb2d6jyIQDI=; b=QvD0DekK+2rEVBsL3YOPgrCeE98qOZejqvuODqrWq8GEjsoD17sZIIAfMNNiA3zcZx 6OudL4ni7yYo+ofbdR+1f/jMuDeA72QPGz7VIperu8hhqz0fC214Fuk98QL+0B0uxg5/ svKOp45RHJ8EiXvtsoZYkKwN+sJ5GQJTgXGUfCfwm4i+J2ydCIqud51+DNzfPTWGk6s1 BGAkLJNn9KLKWtc7wM+XcTjGES/QryLbfezDhM1HMmZ3KXhPIy4zye4KBFzgsu7fq1Dx fZijXZCgS6AL4MEVfuVEojF5Vo9AYq0xymU/GUwoTMGWvEnjA68Td4KERtj5gUrCKiXG FwKQ== X-Gm-Message-State: AJIora9tXwnSdoXyIQG698eiPT1zzsYRyPD9Hnr1Si23H0tT/tqOJM8I gSkIAZkQG2i9+d8HGHH92CJIA1X3jQ+vdw== X-Google-Smtp-Source: AGRyM1tyJ7HCE2hWYJQut8aoUvC69okfBHJ96j1j1ZSbRYdaBuVLvB5dyW8CK0KciTWydhHSwaNBOQ== X-Received: by 2002:ae9:efd5:0:b0:6ae:f7fe:4502 with SMTP id d204-20020ae9efd5000000b006aef7fe4502mr11819387qkg.421.1656705700546; Fri, 01 Jul 2022 13:01:40 -0700 (PDT) Received: from mail-yb1-f170.google.com (mail-yb1-f170.google.com. [209.85.219.170]) by smtp.gmail.com with ESMTPSA id az8-20020a05620a170800b006b14b303b37sm5732918qkb.102.2022.07.01.13.01.39 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 01 Jul 2022 13:01:39 -0700 (PDT) Received: by mail-yb1-f170.google.com with SMTP id i7so5777862ybe.11 for ; Fri, 01 Jul 2022 13:01:39 -0700 (PDT) X-Received: by 2002:a25:3288:0:b0:66c:8a91:74bb with SMTP id y130-20020a253288000000b0066c8a9174bbmr16964347yby.89.1656705699367; Fri, 01 Jul 2022 13:01:39 -0700 (PDT) MIME-Version: 1.0 References: <20191014140416.28517-1-tzimmermann@suse.de> <20191014140416.28517-10-tzimmermann@suse.de> <8b79b40b-9786-9ddf-0dc9-89efbab38c7f@suse.de> In-Reply-To: From: Geert Uytterhoeven Date: Fri, 1 Jul 2022 22:01:27 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 09/15] drm/fbconv: Mode-setting pipeline enable / disable To: Thomas Zimmermann Content-Type: text/plain; charset="UTF-8" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Linux Fbdev development list , Jonathan Corbet , David Airlie , Greg KH , michel@daenzer.net, Mathieu Malaterre , DRI Development , Sean Paul Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Thomas, On Mon, May 30, 2022 at 10:34 AM Geert Uytterhoeven wrote: > On Mon, May 30, 2022 at 9:47 AM Thomas Zimmermann wrote: > > Am 28.05.22 um 22:17 schrieb Geert Uytterhoeven: > > > On Mon, Oct 14, 2019 at 4:05 PM Thomas Zimmermann wrote: > > >> The display mode is set by converting the DRM display mode to an > > >> fb_info state and handling it to the fbdev driver's fb_setvar() > > >> function. This also requires a color depth, which we take from the > > >> value of struct drm_mode_config.preferred_depth > > >> > > >> Signed-off-by: Thomas Zimmermann > > > > > >> --- a/drivers/gpu/drm/drm_fbconv_helper.c > > >> +++ b/drivers/gpu/drm/drm_fbconv_helper.c > > >> @@ -919,6 +919,24 @@ static int drm_fbconv_update_fb_var_screeninfo_from_framebuffer( > > >> return 0; > > >> } > > >> > > >> +static int drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( > > >> + struct fb_var_screeninfo *fb_var, struct drm_simple_display_pipe *pipe) > > >> +{ > > >> + struct drm_plane *primary = pipe->crtc.primary; > > >> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); > > >> + > > >> + if (primary && primary->state && primary->state->fb) > > >> + return drm_fbconv_update_fb_var_screeninfo_from_framebuffer( > > >> + fb_var, primary->state->fb, > > >> + modeset->fb_info->fix.smem_len); > > >> + > > >> + fb_var->xres_virtual = fb_var->xres; > > >> + fb_var->yres_virtual = fb_var->yres; > > >> + fb_var->bits_per_pixel = modeset->dev->mode_config.preferred_depth; > > > > > > This looks wrong to me: IMHO bits_per_pixel should be derived from > > > the fourcc format of the _new_ mode to be set... > > > > Indeed, this appears to be wrong. > > OK. > > > > > > > > >> + > > >> + return 0; > > >> +} > > >> + > > >> /** > > >> * drm_fbconv_simple_display_pipe_mode_valid - default implementation for > > >> * struct drm_simple_display_pipe_funcs.mode_valid > > >> @@ -950,6 +968,28 @@ bool drm_fbconv_simple_display_pipe_mode_fixup( > > >> struct drm_crtc *crtc, const struct drm_display_mode *mode, > > >> struct drm_display_mode *adjusted_mode) > > >> { > > >> + struct drm_simple_display_pipe *pipe = > > >> + container_of(crtc, struct drm_simple_display_pipe, crtc); > > >> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); > > >> + struct fb_var_screeninfo fb_var; > > >> + int ret; > > >> + > > >> + if (!modeset->fb_info->fbops->fb_check_var) > > >> + return true; > > >> + > > >> + drm_fbconv_init_fb_var_screeninfo_from_mode(&fb_var, mode); > > >> + > > >> + ret = drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( > > >> + &fb_var, &modeset->pipe); > > >> + if (ret) > > >> + return true; > > >> + > > >> + ret = modeset->fb_info->fbops->fb_check_var(&fb_var, modeset->fb_info); > > > > > > ... hence this fails if the requested mode is valid with the new > > > fourcc format, but invalid with the old (but preferred) depth. > > > E.g. due to bandwidth limitations, a high-resolution mode is valid > > > with a low color depth, while a high color depth is limited to lower > > > resolutions. > > > Unfortunately we do not know the new fourcc format here, as both > > > drm_simple_display_pipe_funcs.mode_{valid,fixup}() are passed > > > the mode (from drm_mode_set.mode), but not the new format (from > > > drm_mode_set.fb->format). > > > > > > Am I missing something? Is the new format available in some other way? > > > > We can always get the format from the new plane state of > > modeset->pipe->plane. We'd have this in the atomic_check call. And it > > appears that drm_fbconv_simple_display_pipe_check() is a better place > > for this code anyway. > > Thanks, I'll give that a try! Getting the format from the new plane state of pipe->plane doesn't work, as pipe->plane.state->fb = NULL. But it is indeed available in the drm_simple_display_pipe_funcs.check() callback, so that seems to work... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds