Nouveau - Dec 2014 - [PATCH nouveau 09/11] drm: export some variable and functions to resue the PM functions

On 23/12/14 10:40, Vince Hsu wrote:
> This patch adds some checks in the suspend/resume functions to distinguish
> the dGPU and mobile GPU and exports some variables/functions so that the
> nouveau platform device can reuse them.
> 
Hi Vince,

Afaiu one needs to export a symbol as it's used by another module or
subsystem. With the follow up two patches you are not doing either one,
so I'd assume that you can just omit the EXPORT_* changes.

I could be wrong though :-)

Cheers
Emil

> Signed-off-by: Vince Hsu <vinceh at nvidia.com>
> ---
>  drm/nouveau_drm.c | 16 +++++++++++-----
>  drm/nouveau_drm.h |  2 ++
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drm/nouveau_drm.c b/drm/nouveau_drm.c
> index afb93bb72f97..0ed99ef80211 100644
> --- a/drm/nouveau_drm.c
> +++ b/drm/nouveau_drm.c
> @@ -72,6 +72,7 @@ module_param_named(modeset, nouveau_modeset, int, 0400);
>  
>  MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus only
default (-1)");
>  int nouveau_runtime_pm = -1;
> +EXPORT_SYMBOL(nouveau_runtime_pm);
>  module_param_named(runpm, nouveau_runtime_pm, int, 0400);
>  
>  static struct drm_driver driver_stub;
> @@ -543,7 +544,7 @@ nouveau_drm_remove(struct pci_dev *pdev)
>  	nouveau_drm_device_remove(dev);
>  }
>  
> -static int
> +int
>  nouveau_do_suspend(struct drm_device *dev, bool runtime)
>  {
>  	struct nouveau_drm *drm = nouveau_drm(dev);
> @@ -559,8 +560,10 @@ nouveau_do_suspend(struct drm_device *dev, bool
runtime)
>  			return ret;
>  	}
>  
> -	NV_INFO(drm, "evicting buffers...\n");
> -	ttm_bo_evict_mm(&drm->ttm.bdev, TTM_PL_VRAM);
> +	if (dev->pdev) {
> +		NV_INFO(drm, "evicting buffers...\n");
> +		ttm_bo_evict_mm(&drm->ttm.bdev, TTM_PL_VRAM);
> +	}
>  
>  	NV_INFO(drm, "waiting for kernel channels to go idle...\n");
>  	if (drm->cechan) {
> @@ -612,8 +615,9 @@ fail_display:
>  	}
>  	return ret;
>  }
> +EXPORT_SYMBOL(nouveau_do_suspend);
>  
> -static int
> +int
>  nouveau_do_resume(struct drm_device *dev, bool runtime)
>  {
>  	struct nouveau_drm *drm = nouveau_drm(dev);
> @@ -635,7 +639,8 @@ nouveau_do_resume(struct drm_device *dev, bool runtime)
>  		nvif_client_resume(&cli->base);
>  	}
>  
> -	nouveau_run_vbios_init(dev);
> +	if (dev->pdev)
> +		nouveau_run_vbios_init(dev);
>  
>  	if (dev->mode_config.num_crtc) {
>  		NV_INFO(drm, "resuming display...\n");
> @@ -646,6 +651,7 @@ nouveau_do_resume(struct drm_device *dev, bool runtime)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL(nouveau_do_resume);
>  
>  int
>  nouveau_pmops_suspend(struct device *dev)
> diff --git a/drm/nouveau_drm.h b/drm/nouveau_drm.h
> index 8ae36f265fb8..897d295dd1e3 100644
> --- a/drm/nouveau_drm.h
> +++ b/drm/nouveau_drm.h
> @@ -177,6 +177,8 @@ nouveau_drm(struct drm_device *dev)
>  
>  int nouveau_pmops_suspend(struct device *);
>  int nouveau_pmops_resume(struct device *);
> +int nouveau_do_suspend(struct drm_device *dev, bool runtime);
> +int nouveau_do_resume(struct drm_device *dev, bool runtime);
>  
>  #define nouveau_platform_device_create(p, u)                              
\
>  	nouveau_platform_device_create_(p, sizeof(**u), (void **)u)
>

Hi Emil,

On 12/30/2014 10:34 AM, Emil Velikov wrote:
> On 23/12/14 10:40, Vince Hsu wrote:
>> This patch adds some checks in the suspend/resume functions to
distinguish
>> the dGPU and mobile GPU and exports some variables/functions so that
the
>> nouveau platform device can reuse them.
>>
> Hi Vince,
>
> Afaiu one needs to export a symbol as it's used by another module or
> subsystem. With the follow up two patches you are not doing either one,
> so I'd assume that you can just omit the EXPORT_* changes.
The nouveau platform device driver is built as another module - 
nouveau_platform.ko. :)

Thanks,
Vince
>
> I could be wrong though :-)
>
> Cheers
> Emil
>
>
>> Signed-off-by: Vince Hsu <vinceh at nvidia.com>
>> ---
>>   drm/nouveau_drm.c | 16 +++++++++++-----
>>   drm/nouveau_drm.h |  2 ++
>>   2 files changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drm/nouveau_drm.c b/drm/nouveau_drm.c
>> index afb93bb72f97..0ed99ef80211 100644
>> --- a/drm/nouveau_drm.c
>> +++ b/drm/nouveau_drm.c
>> @@ -72,6 +72,7 @@ module_param_named(modeset, nouveau_modeset, int,
0400);
>>   
>>   MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus
only default (-1)");
>>   int nouveau_runtime_pm = -1;
>> +EXPORT_SYMBOL(nouveau_runtime_pm);
>>   module_param_named(runpm, nouveau_runtime_pm, int, 0400);
>>   
>>   static struct drm_driver driver_stub;
>> @@ -543,7 +544,7 @@ nouveau_drm_remove(struct pci_dev *pdev)
>>   	nouveau_drm_device_remove(dev);
>>   }
>>   
>> -static int
>> +int
>>   nouveau_do_suspend(struct drm_device *dev, bool runtime)
>>   {
>>   	struct nouveau_drm *drm = nouveau_drm(dev);
>> @@ -559,8 +560,10 @@ nouveau_do_suspend(struct drm_device *dev, bool
runtime)
>>   			return ret;
>>   	}
>>   
>> -	NV_INFO(drm, "evicting buffers...\n");
>> -	ttm_bo_evict_mm(&drm->ttm.bdev, TTM_PL_VRAM);
>> +	if (dev->pdev) {
>> +		NV_INFO(drm, "evicting buffers...\n");
>> +		ttm_bo_evict_mm(&drm->ttm.bdev, TTM_PL_VRAM);
>> +	}
>>   
>>   	NV_INFO(drm, "waiting for kernel channels to go
idle...\n");
>>   	if (drm->cechan) {
>> @@ -612,8 +615,9 @@ fail_display:
>>   	}
>>   	return ret;
>>   }
>> +EXPORT_SYMBOL(nouveau_do_suspend);
>>   
>> -static int
>> +int
>>   nouveau_do_resume(struct drm_device *dev, bool runtime)
>>   {
>>   	struct nouveau_drm *drm = nouveau_drm(dev);
>> @@ -635,7 +639,8 @@ nouveau_do_resume(struct drm_device *dev, bool
runtime)
>>   		nvif_client_resume(&cli->base);
>>   	}
>>   
>> -	nouveau_run_vbios_init(dev);
>> +	if (dev->pdev)
>> +		nouveau_run_vbios_init(dev);
>>   
>>   	if (dev->mode_config.num_crtc) {
>>   		NV_INFO(drm, "resuming display...\n");
>> @@ -646,6 +651,7 @@ nouveau_do_resume(struct drm_device *dev, bool
runtime)
>>   
>>   	return 0;
>>   }
>> +EXPORT_SYMBOL(nouveau_do_resume);
>>   
>>   int
>>   nouveau_pmops_suspend(struct device *dev)
>> diff --git a/drm/nouveau_drm.h b/drm/nouveau_drm.h
>> index 8ae36f265fb8..897d295dd1e3 100644
>> --- a/drm/nouveau_drm.h
>> +++ b/drm/nouveau_drm.h
>> @@ -177,6 +177,8 @@ nouveau_drm(struct drm_device *dev)
>>   
>>   int nouveau_pmops_suspend(struct device *);
>>   int nouveau_pmops_resume(struct device *);
>> +int nouveau_do_suspend(struct drm_device *dev, bool runtime);
>> +int nouveau_do_resume(struct drm_device *dev, bool runtime);
>>   
>>   #define nouveau_platform_device_create(p, u)                         
\
>>   	nouveau_platform_device_create_(p, sizeof(**u), (void **)u)
>>

On Tue, Dec 30, 2014 at 11:18:34AM +0800, Vince Hsu
wrote:
> Hi Emil,
> 
> On 12/30/2014 10:34 AM, Emil Velikov wrote:
> >On 23/12/14 10:40, Vince Hsu wrote:
> >>This patch adds some checks in the suspend/resume functions to
distinguish
> >>the dGPU and mobile GPU and exports some variables/functions so
that the
> >>nouveau platform device can reuse them.
> >>
> >Hi Vince,
> >
> >Afaiu one needs to export a symbol as it's used by another module
or
> >subsystem. With the follow up two patches you are not doing either one,
> >so I'd assume that you can just omit the EXPORT_* changes.
> The nouveau platform device driver is built as another module -
> nouveau_platform.ko. :)
I'd like to hear the opinion of the nouveau people and Alex, but I'd
very much prefer if nouveau_platform.o was simply linked into the
nouveau.ko module. I don't see any good reason to keep it separate.

Something like the attached patch (untested) ought to do it.

Thierry
-------------- next part --------------
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 40afc69a3778..5ab13e7939db 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -26,7 +26,7 @@ config DRM_NOUVEAU
 	  Choose this option for open-source NVIDIA support.
 
 config NOUVEAU_PLATFORM_DRIVER
-	tristate "Nouveau (NVIDIA) SoC GPUs"
+	bool "Nouveau (NVIDIA) SoC GPUs"
 	depends on DRM_NOUVEAU && ARCH_TEGRA
 	default y
 	help
diff --git a/drivers/gpu/drm/nouveau/Makefile b/drivers/gpu/drm/nouveau/Makefile
index ea40cd653c7c..13acf90c11c8 100644
--- a/drivers/gpu/drm/nouveau/Makefile
+++ b/drivers/gpu/drm/nouveau/Makefile
@@ -394,8 +394,6 @@ nouveau-$(CONFIG_ACPI) += nouveau_acpi.o
 endif
 nouveau-$(CONFIG_DRM_NOUVEAU_BACKLIGHT) += nouveau_backlight.o
 nouveau-$(CONFIG_DEBUG_FS) += nouveau_debugfs.o
+nouveau-$(CONFIG_NOUVEAU_PLATFORM_DRIVER) += nouveau_platform.o
 
 obj-$(CONFIG_DRM_NOUVEAU)+= nouveau.o
-
-# platform driver
-obj-$(CONFIG_NOUVEAU_PLATFORM_DRIVER) += nouveau_platform.o
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/nouveau/attachments/20150105/66cde126/attachment.sig>