From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 75145EB64DC for ; Tue, 11 Jul 2023 15:23:37 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1706C10E3B5; Tue, 11 Jul 2023 15:23:37 +0000 (UTC) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9902B10E3C1 for ; Tue, 11 Jul 2023 15:23:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1689089014; x=1720625014; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=RS5RPddlLZEDynDGNG5yGufT1IbQLGl3HTfuM/Na+P8=; b=IMVN/xRKADSk1FlzAAGiWpQrqL7aQmgFGCiD8INk6QKFdeYoM8BDKWhc B26OXx1zqpEqE7WEXcJHQY7MvJhJAhZM6qNuAycym9qh4xtSLpQwZr0Ho mVAnGE3cq9vFjFaWoRnBOXanMdJ3pjbwcqE/iZapAcoC8Hen0kfpDHikw Ix7FLfPTOzfbdUIIf2HUobg4p7p7QVQTVy+1JkMp0wW1EPPgyTO+4f3Ib i4+aR0V5Ib4vLBhxWvM9mCk22wz0E0r/cskGcaGoAHEDq5nWYh+3pyKM+ CSxuY2CmI/dpM6lm44x6NIpd5eUVfrQJPegmTDRQ8CXSgPbgOHz5EbDFM w==; X-IronPort-AV: E=McAfee;i="6600,9927,10768"; a="349457461" X-IronPort-AV: E=Sophos;i="6.01,196,1684825200"; d="scan'208";a="349457461" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Jul 2023 08:23:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10768"; a="834730963" X-IronPort-AV: E=Sophos;i="6.01,196,1684825200"; d="scan'208";a="834730963" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.116.40]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Jul 2023 08:23:32 -0700 Date: Tue, 11 Jul 2023 08:23:32 -0700 Message-ID: <87pm4yh3pn.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Andi Shyti In-Reply-To: References: <20230705084403.3922130-1-tejas.upadhyay@intel.com> <87y1jucv1g.wl-ashutosh.dixit@intel.com> <87wmzecqd6.wl-ashutosh.dixit@intel.com> <29fe7224-92e8-0f62-6801-b2588dbb1001@linux.intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [Intel-xe] [PATCH V2] drm/xe: make GT sysfs init return void X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-xe@lists.freedesktop.org, Nirmoy Das Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Tue, 11 Jul 2023 03:42:11 -0700, Andi Shyti wrote: > Hi Andi, > On Wed, Jul 05, 2023 at 06:37:46PM +0200, Nirmoy Das wrote: > > Hi Ashutosh, > > > > On 7/5/2023 5:47 PM, Dixit, Ashutosh wrote: > > > On Wed, 05 Jul 2023 08:39:20 -0700, Nirmoy Das wrote: > > > > Hi Ashutosh, > > > > > > > > On 7/5/2023 4:06 PM, Dixit, Ashutosh wrote: > > > > > On Wed, 05 Jul 2023 01:44:03 -0700, Tejas Upadhyay wrote: > > > > > > Currently return from xe_gt_sysfs_init() is ignored > > > > > > and also a failure in xe_gt_sysfs_init() isn't fatal > > > > > > so make it return void. > > > > > But why is the failure not fatal? I really don't understand the concept of > > > > > these non-fatal failures. Do we really want to say the device is up if > > > > > sysfs initialization has failed for some reason and people are unable to > > > > > see card freq's e.g.? This was done in i915 but do we really want to repeat > > > > > this for xe? IMO the simplest thing to do would be to fail the probe unless > > > > > ALL required/intended functionality is clearly up. > > > > > > > > I agree with the concern but the situation is different with a graphics > > > > driver. > > > > > > > > If we return error on probe, (if I am not wrong) a user will have no way to > > > > interact > > > > > > > > with the system other than ssh. We should ignore non-fatal error and let > > > > the driver load > > > > > > > > so a user can have something to work with(may be report a bug :) ) > > > Hmm, good point. Agreed :) > > > > > > This way though only display is critical and everything else non-critical? > > > > Yes, that would be wrong, I am not saying that. We do return error during > > the probe at multiple locations, > > > > I believe we can prioritize system usability by considering this specific > > error as non-critical. Although those sysfs files are important, > > I have been one of the supporters of the non fatal failures in > sysfs for a usability reason. A big warning printed should be > more than enaough while the driver can still be up and running. Yes ok, that was the reason I acked the patch. > > Besides, I believe that if sysfs fails, then most probably the > system has something wrong. Afaiu it sysfs creation failure can only happen in extreme low memory conditions. But my conclusion from that is exactly the opposite. If that is the case, why make exceptions in the code and split failures into fatal and non-fatal, why not consider all failures as fatal and implement identical error handling in all cases? If sysfs is failing something else would fail too. Not having display is fatal for someone who cannot ssh into the system. Not having sysfs is fatal for someone who wants to do performance analysis and wants to see GPU freq's. Thanks. -- Ashutosh