Message-ID: <1c1aeb13-6e90-ed2b-08db-11ceaa8d0de0@redhat.com>
Date:   Wed, 15 Dec 2021 09:50:34 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [PATCH 00/60] drm: Make all drivers to honour the nomodeset
 parameter
Content-Language: en-US
To:     Thomas Zimmermann <tzimmermann@suse.de>,
        linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Cc:     =?UTF-8?Q?Heiko_St=c3=bcbner?= <heiko@sntech.de>,
        "James (Qian) Wang" <james.qian.wang@arm.com>,
        =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= <noralf@tronnes.org>,
        Alexey Brodkin <abrodkin@synopsys.com>,
        Alison Wang <alison.wang@nxp.com>,
        Anitha Chrisanthus <anitha.chrisanthus@intel.com>,
        Benjamin Gaignard <benjamin.gaignard@linaro.org>,
        Boris Brezillon <bbrezillon@kernel.org>,
        Brian Starkey <brian.starkey@arm.com>,
        Chen-Yu Tsai <wens@csie.org>,
        Chun-Kuang Hu <chunkuang.hu@kernel.org>,
        David Lechner <david@lechnology.com>,
        Deepak Rawat <drawat.floss@gmail.com>,
        Edmund Dea <edmund.j.dea@intel.com>,
        Emma Anholt <emma@anholt.net>,
        Erico Nunes <nunes.erico@gmail.com>,
        Hans de Goede <hdegoede@redhat.com>,
        Hyun Kwon <hyun.kwon@xilinx.com>,
        Inki Dae <inki.dae@samsung.com>, Joel Stanley <joel@jms.id.au>,
        Joonyoung Shim <jy0922.shim@samsung.com>,
        Jyri Sarha <jyri.sarha@iki.fi>,
        Kamlesh Gurudasani <kamlesh.gurudasani@gmail.com>,
        Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Liviu Dudau <liviu.dudau@arm.com>,
        Lucas Stach <l.stach@pengutronix.de>,
        Marek Vasut <marex@denx.de>,
        Maxime Ripard <mripard@kernel.org>,
        Neil Armstrong <narmstrong@baylibre.com>,
        Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>,
        Patrik Jakobsson <patrik.r.jakobsson@gmail.com>,
        Paul Cercueil <paul@crapouillou.net>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Philippe Cornu <philippe.cornu@foss.st.com>,
        Qiang Yu <yuq825@gmail.com>, Rob Clark <robdclark@gmail.com>,
        Rob Herring <robh@kernel.org>,
        Russell King <linux+etnaviv@armlinux.org.uk>,
        Russell King <linux@armlinux.org.uk>,
        Sam Ravnborg <sam@ravnborg.org>,
        Sandy Huang <hjc@rock-chips.com>, Sean Paul <sean@poorly.run>,
        Stefan Agner <stefan@agner.ch>,
        Thierry Reding <thierry.reding@gmail.com>,
        Tian Tao <tiantao6@hisilicon.com>,
        Tomeu Vizoso <tomeu.vizoso@collabora.com>,
        Tomi Valkeinen <tomba@kernel.org>,
        Xinliang Liu <xinliang.liu@linaro.org>,
        Yannick Fertre <yannick.fertre@foss.st.com>
References: <20211215010008.2545520-1-javierm@redhat.com>
 <58d00cac-dbf1-9704-3c0b-16fd837a5b6b@suse.de>
From:   Javier Martinez Canillas <javierm@redhat.com>
In-Reply-To: <58d00cac-dbf1-9704-3c0b-16fd837a5b6b@suse.de>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hello Thomas,

On 12/15/21 09:31, Thomas Zimmermann wrote:
> Hi Javier,
> 
> nothing wrong with your patches, but I'd like to propose slightly 
> differnt solution.
> 
> For many USB drivers, you put the drm_firmware_drivers_only() call into 
> the probe function. For registering, these drivers use 
> module_usb_driver(), which expands to generic device-register functions.
> 
> I'd like to propose a similar macro for DRM drivers that includes the 
> test for drm_firmware_drivers_only().
> 
> In drm_drv.h :
> 
>    #if defined(USB)
>    static int drm_usb_register(struct usb_driver *usb)
>    {
> 	if (drm_firmware_drivers_only())
> 		return -ENODEV;
> 	return usb_register_driver(usb);
>    }
>    #define drm_module_usb_driver(__usb)
> 	module_driver(drm_usb_register, usb_deregister)
>    #endif
> 
> In each of the USB-based DRM drivers, replace module_usb_driver with 
> drm_module_usb_driver.
> 
> And then there's PCI [3] and platform drivers, [4] which can be handled 
> similarly. Many PCI drivers open-code the module init and device 

Thanks for the suggestion. I actually thought about this approach as well,
since seems unnecessary to have all that duplicated logic in every driver.

But at the end decided to just do the less intrusive change, because can't
do any testing for most of the drivers.

> registering with the driver-specific enable parameter. Maybe adding a 
> driver-specific register function would make sense.
>

If I understood you correctly, suggested something similar in #dri-devel but
danvet's suggestion [0] was to keep it simple:

16:32 javierm: danvet: yes, I don't think we would be able to do it at this point. I'll add a function pointer
                       param to the check function so drivers can also define their own check besides nomodeset
16:33 danvet: javierm, uh that sounds a bit like overkill? just keep that part of the check in drivers?
16:33 javierm: danvet: Ok. Wanted to get rid of the duplicated code but I'm indeed over engineering this :)

[0]: https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&date=2021-11-02

> Best regards
> Thomas
> 
Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat