[PATCH 2/5] drm/tegra: vic: Load firmware on demand

[Dmitry Osipenko]

23.01.2019 12:39, Thierry Reding пишет:
> From: Thierry Reding <treding at nvidia.com>
> 
> Loading the firmware requires an allocation of IOVA space to make sure
> that the VIC's Falcon microcontroller can read the firmware if address
> translation via the SMMU is enabled.
> 
> However, the allocation currently happens at a time where the geometry
> of an IOMMU domain may not have been initialized yet. This happens for
> example on Tegra186 and later where an ARM SMMU is used. Domains which
> are created by the ARM SMMU driver postpone the geometry setup until a
> device is attached to the domain. This is because IOMMU domains aren't
> attached to a specific IOMMU instance at allocation time and hence the
> input address space, which defines the geometry, is not known yet.
> 
> Work around this by postponing the firmware load until it is needed at
> the time where a channel is opened to the VIC. At this time the shared
> IOMMU domain's geometry has been properly initialized.
> 
> As a byproduct this allows the Tegra DRM to be created in the absence
> of VIC firmware, since the VIC initialization no longer fails if the
> firmware can't be found.
> 
> Signed-off-by: Thierry Reding <treding at nvidia.com>
> ---
>  drivers/gpu/drm/tegra/vic.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> index d47983deb1cf..afbdc33f49bc 100644
> --- a/drivers/gpu/drm/tegra/vic.c
> +++ b/drivers/gpu/drm/tegra/vic.c
> @@ -181,13 +181,6 @@ static int vic_init(struct host1x_client *client)
>  		vic->domain = tegra->domain;
>  	}
>  
> -	if (!vic->falcon.data) {
> -		vic->falcon.data = tegra;
> -		err = falcon_load_firmware(&vic->falcon);
> -		if (err < 0)
> -			goto detach;
> -	}
> -
>  	vic->channel = host1x_channel_request(client->dev);
>  	if (!vic->channel) {
>  		err = -ENOMEM;
> @@ -256,6 +249,16 @@ static int vic_open_channel(struct tegra_drm_client *client,
>  	if (err < 0)
>  		return err;
>  
> +	if (!vic->falcon.data) {
> +		vic->falcon.data = client->drm;
> +
> +		err = falcon_load_firmware(&vic->falcon);
> +		if (err < 0) {

Also, notice that this hunk misses the "vic->falcon.data = NULL", hence the second channel opening will likely to crash kernel.

> +			pm_runtime_put(vic->dev);
> +			return err;
> +		}
> +	}
> +
>  	err = vic_boot(vic);
>  	if (err < 0) {
>  		pm_runtime_put(vic->dev);
>