Re: [PATCH 2/5] drm/i915: Rename i915_load_modeset_init() to i915_modeset_load()

[Jani Nikula <jani.nikula@intel.com>]

On Fri, 01 Mar 2019, José Roberto de Souza <jose.souza@intel.com> wrote:
> i915_load_modeset_init() sounds horrible also lets rename it so
> the future cleanup function of it can be easially recognized.

We load the driver, but init modeset. The name implies modeset init at
driver load.

The modeset load/unload sounds a bit odd to me. Even more so with
begin/finish load and begin unload. It's not obvious to me what those
should do. They're not a pattern we have.

I'm not all that averse to renaming stuff in general, but any rename
increases the cognitive burden for a while, and should make things
*easier* to understand. I don't think that's the case in this series.

I can understand i915_load_modeset_init sounding funny. If you insist on
renaming, I'd go with i915_driver_modeset_init. It's in line with the
rest of the functions.

Here are my suggestions for the names:

i915_modeset_load/i915_modeset_begin_load
-> i915_driver_modeset_init

i915_modeset_unload/i915_modeset_begin_unload
-> i915_driver_modeset_cleanup or _fini

i915_modeset_finish_load
-> i915_driver_modeset_init_late


BR,
Jani.


>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index c08abdef5eb6..90c77fab3d70 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -639,7 +639,7 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
>  	.can_switch = i915_switcheroo_can_switch,
>  };
>  
> -static int i915_load_modeset_init(struct drm_device *dev)
> +static int i915_modeset_load(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct pci_dev *pdev = dev_priv->drm.pdev;
> @@ -1748,7 +1748,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (ret < 0)
>  		goto out_cleanup_mmio;
>  
> -	ret = i915_load_modeset_init(&dev_priv->drm);
> +	ret = i915_modeset_load(&dev_priv->drm);
>  	if (ret < 0)
>  		goto out_cleanup_hw;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx