On Thu, 04 Nov 2021, Sam Ravnborg [off-list ref] wrote:

Hi Javier,

quoted

quoted

quoted

quoted

 
-	if (vgacon_text_force() && i915_modparams.modeset == -1)
+	ret = drm_drv_enabled(&driver);

You pass the local driver variable here - which looks wrong as this is
not the same as the driver variable declared in another file.

Yes, Jani mentioned it already. I got confused and thought that the driver
variable was also defined in the same compilation unit...

Maybe I could squash the following change?

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b18a250e5d2e..b8f399b76363 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c

@@ -89,7 +89,7 @@
 #include "intel_region_ttm.h"
 #include "vlv_suspend.h"
 
-static const struct drm_driver driver;
+const struct drm_driver driver;

No, variables with such a generic name will clash.

Just add a
const drm_driver * i915_drm_driver(void)
{
	return &driver;
}

And then use this function to access the drm_driver variable.

Agreed on the general principle of exposing interfaces over data.

But... why? I'm still at a loss what problem this solves. We pass
&driver to exactly one place, devm_drm_dev_alloc(), and it's neatly
hidden away.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center