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

[Lucas De Marchi <lucas.demarchi@intel.com>]

On Mon, Mar 04, 2019 at 01:00:40PM +0200, Jani Nikula wrote:
>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

+1 for the name suggestions

Lucas De Marchi

>
>
>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