Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bfb03d40-a023-12a9-9554-1b6e6c474134@redhat.com>
Date:   Wed, 4 May 2022 13:32:37 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH v3 3/3] drm: Allow simpledrm to setup its emulated FB as
 firmware provided
Content-Language: en-US
To:     linux-kernel@vger.kernel.org,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        dri-devel@lists.freedesktop.org
References: <20220503071540.471667-1-javierm@redhat.com>
 <20220503071540.471667-4-javierm@redhat.com>
 <YnJcaaDcIsJKhSwQ@phenom.ffwll.local>
From:   Javier Martinez Canillas <javierm@redhat.com>
In-Reply-To: <YnJcaaDcIsJKhSwQ@phenom.ffwll.local>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/4/22 12:58, Daniel Vetter wrote:
> On Tue, May 03, 2022 at 09:15:40AM +0200, Javier Martinez Canillas wrote:
>> Indicate to fbdev subsystem that the registered framebuffer is provided by
>> the system firmware, so that it can handle accordingly. For example, would
>> unregister the FB devices if asked to remove the conflicting framebuffers.
>>
>> Add a new DRM_FB_FW field to drm_fbdev_generic_setup() options parameter.
>> Drivers can use this to indicate the FB helper initialization that the FB
>> registered is provided by the firmware, so it can be configured as such.
>>
>> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>>
>> Changes in v3:
>> - Drop the firmware local variable (Laurent Pinchart).
>> - Use DRM_FB_OPTION() since DRM_FB_SET_OPTION() got renamed (kernel test robot).
> 
> Just for the record what I brought up on irc already:
> 
> FBINFO_MISC_FIRMWARE is purely an internal flag with no uapi impact, and
> it's only to control whether we nuke this from
> remove_conflicting_framebuffer or not. Since simpledrm only ever binds
> against sysfb I think it'd be cleaner to only rely on that, and relegate

That's not actually true. The OF subsystem also registers "simple-framebuffer"
devices when there are Device Tree nodes that contain a "simple-framebuffer"
compatible string. In that case these pdev will also bind against simpledrm.

> that entire FBINFO_MISC_FIRMWARE misc hack to the fbdev dungeons and let
> it quietly wither away there.
>
> Also I'm not a huge fan of these midlayer flags in general :-)

And while I agree with you that these midlayer flags are horrible, that is
what any other fbdev that makes use of a firmware-provided framebuffer set,
so simpledrm emulated fbdev shouldn't be the exception IMO.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat