Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Received-SPF: None (protection.outlook.com: amd.com does not designate
 permitted sender hosts)
Subject: Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from
 ttm_bo_evict
To:     =?UTF-8?Q?Michel_D=c3=a4nzer?= <michel@daenzer.net>,
        Christian Koenig <christian.koenig@amd.com>,
        Huang Rui <ray.huang@amd.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <maxime.ripard@bootlin.com>,
        Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>
CC:     <dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>
References: <20181205165621.5805-1-michel@daenzer.net>
 <20181205165621.5805-2-michel@daenzer.net>
From:   "Zhang, Jerry(Junwei)" <Jerry.Zhang@amd.com>
Message-ID: <b0b57acb-7152-ab1a-855a-3de8c77fa425@amd.com>
Date:   Thu, 6 Dec 2018 10:43:48 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <20181205165621.5805-2-michel@daenzer.net>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Dec 2018 02:43:57.4016
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: f7520267-835b-4e86-1d1c-08d65b24aba3
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXCHOV01.amd.com]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR12MB2629
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 12/6/18 12:56 AM, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> All the output is related, so it should all be printed the same way.
> Some of it was using pr_debug, but some of it appeared in dmesg by
> default. The caller should handle failure, so there's no need to spam
> dmesg with potentially quite a lot of output by default.
>
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Sounds reasonable, but personally prefer to show error when some
vital incident happens, e.g. no memory on eviction.

Regards,
Jerry

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c | 39 ++++++++++++++++++------------------
>   1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index d87935bf8e30..5e9b9dd91629 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -77,38 +77,39 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
>   	return 0;
>   }
>   
> -static void ttm_mem_type_debug(struct ttm_bo_device *bdev, int mem_type)
> +static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p,
> +			       int mem_type)
>   {
>   	struct ttm_mem_type_manager *man = &bdev->man[mem_type];
> -	struct drm_printer p = drm_debug_printer(TTM_PFX);
>   
> -	pr_err("    has_type: %d\n", man->has_type);
> -	pr_err("    use_type: %d\n", man->use_type);
> -	pr_err("    flags: 0x%08X\n", man->flags);
> -	pr_err("    gpu_offset: 0x%08llX\n", man->gpu_offset);
> -	pr_err("    size: %llu\n", man->size);
> -	pr_err("    available_caching: 0x%08X\n", man->available_caching);
> -	pr_err("    default_caching: 0x%08X\n", man->default_caching);
> +	drm_printf(p, "    has_type: %d\n", man->has_type);
> +	drm_printf(p, "    use_type: %d\n", man->use_type);
> +	drm_printf(p, "    flags: 0x%08X\n", man->flags);
> +	drm_printf(p, "    gpu_offset: 0x%08llX\n", man->gpu_offset);
> +	drm_printf(p, "    size: %llu\n", man->size);
> +	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
> +	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
>   	if (mem_type != TTM_PL_SYSTEM)
> -		(*man->func->debug)(man, &p);
> +		(*man->func->debug)(man, p);
>   }
>   
>   static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
>   					struct ttm_placement *placement)
>   {
> +	struct drm_printer p = drm_debug_printer(TTM_PFX);
>   	int i, ret, mem_type;
>   
> -	pr_err("No space for %p (%lu pages, %luK, %luM)\n",
> -	       bo, bo->mem.num_pages, bo->mem.size >> 10,
> -	       bo->mem.size >> 20);
> +	drm_printf(&p, "No space for %p (%lu pages, %luK, %luM)\n",
> +		   bo, bo->mem.num_pages, bo->mem.size >> 10,
> +		   bo->mem.size >> 20);
>   	for (i = 0; i < placement->num_placement; i++) {
>   		ret = ttm_mem_type_from_place(&placement->placement[i],
>   						&mem_type);
>   		if (ret)
>   			return;
> -		pr_err("  placement[%d]=0x%08X (%d)\n",
> -		       i, placement->placement[i].flags, mem_type);
> -		ttm_mem_type_debug(bo->bdev, mem_type);
> +		drm_printf(&p, "  placement[%d]=0x%08X (%d)\n",
> +			   i, placement->placement[i].flags, mem_type);
> +		ttm_mem_type_debug(bo->bdev, &p, mem_type);
>   	}
>   }
>   
> @@ -728,8 +729,8 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
>   	ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx);
>   	if (ret) {
>   		if (ret != -ERESTARTSYS) {
> -			pr_err("Failed to find memory space for buffer 0x%p eviction\n",
> -			       bo);
> +			pr_debug("Failed to find memory space for buffer 0x%p eviction\n",
> +				 bo);
>   			ttm_bo_mem_space_debug(bo, &placement);
>   		}
>   		goto out;
> @@ -738,7 +739,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
>   	ret = ttm_bo_handle_move_mem(bo, &evict_mem, true, ctx);
>   	if (unlikely(ret)) {
>   		if (ret != -ERESTARTSYS)
> -			pr_err("Buffer eviction failed\n");
> +			pr_debug("Buffer eviction failed\n");
>   		ttm_bo_mem_put(bo, &evict_mem);
>   		goto out;
>   	}