Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <cf99d227-4fee-483a-ac51-fe8fc4acf579@intel.com>
Date:   Wed, 13 Sep 2023 08:59:00 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/4] drm/xe/gsc: add gsc device support
Content-Language: en-US
To:     "Usyskin, Alexander" <alexander.usyskin@intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "De Marchi, Lucas" <lucas.demarchi@intel.com>,
        "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
CC:     "Winkler, Tomas" <tomas.winkler@intel.com>,
        "Lubart, Vitaly" <vitaly.lubart@intel.com>,
        "intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20230912130835.2488728-1-alexander.usyskin@intel.com>
 <20230912130835.2488728-4-alexander.usyskin@intel.com>
 <5521adaf-724e-4de8-ae56-74dc349b66c8@intel.com>
 <CY5PR11MB6366B470B6B6A72F32F2EB18EDF0A@CY5PR11MB6366.namprd11.prod.outlook.com>
From:   Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
In-Reply-To: <CY5PR11MB6366B470B6B6A72F32F2EB18EDF0A@CY5PR11MB6366.namprd11.prod.outlook.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VkhjUHlLeUxEY28yblh6all2cjczQnVVRGx1aW5yTzhVU0kvWWtSRTBhZUZw?=
 =?utf-8?B?cTMvQlRYM0VQbGoycTVUVmRWa1hPbFR1cTV1VkFrNnZxZ21IZkNKRnYwTXIz?=
 =?utf-8?B?dnczNHJpaDNKcmNVM0piQUk4cjBIcWpDRnpXbUhxbFpDdjVsSU1ER1NucTdY?=
 =?utf-8?B?TDFlUDVPUVF6MUg2ZHdaSzhQMGVHY2lmbW9DakU0SmY4TVJTUlh3YTRHY3Jj?=
 =?utf-8?B?NVJsbjB5UjlXSHlZaDVLVlNJd3JuazNQclFGT0F2UFd3bVFhdVcyOHhDTE1k?=
 =?utf-8?B?UmZSRWYwY3BXMyt6aGxDWUF3ZDAyVGlRODdnREFyZXpYRVZxcmp5Q2xzMURo?=
 =?utf-8?B?dmZzWlBwQzNqZTVsMHpLRTZKSy9qRk16QnRSaW5vMUV3djRPcFRQN0xRc1Nv?=
 =?utf-8?B?M3ZVYTZGckxYZDFIRVNYTDEvajFaQW5wR3ByVWdxbXRhZGtOdEZhb1BYNHpI?=
 =?utf-8?B?c0JXUDdCa3p2Vkg4TDJiYXhiUUFGMDlMN21ZanppbUpENlkzVmJ6elhFeEJV?=
 =?utf-8?B?b2VkVkNzTUNoMVVaaCszaDN0UU15T3pqaitWdDlYVThsKzJzaU5aR092Q2VV?=
 =?utf-8?B?R1RHNDhKWlRVcnhZNVdXVG1aNlF3ekdtanNveHBnYnQybG15VW5aU21GRkd1?=
 =?utf-8?B?L0VLcXlJdWl0aXVpTlRoWHRweU50TFpvS3JnbTB6UnlQTVFZYnJUYm5GbUY2?=
 =?utf-8?B?M2NzUDc2eUFxYk85TkZnYXlLNlVNd21PS3VkRm81aWdFNDFLMzNWdUVVMDJv?=
 =?utf-8?B?ZjNCV3MrV0NSVS94UFNaMWJ2Z1k4dWFCT1hmb3BsdUpzbEZacDdUSHUrT0w3?=
 =?utf-8?B?Y0M5UjY4WG1LWDlKZy9ONVZJSnYrMlRjbEU5Wk5PeUM4QURRdWg3ajJiSzRH?=
 =?utf-8?B?bVZveUJFYmxVdFdNdEJlcncxU01lMW93R0JXUDBEVlM5d05PVTFuTmZwbGlS?=
 =?utf-8?B?d3FscXJ2d1ZEQ28xY2s5d3lMVkg5MlZoQytWWXRNR3l6WThURUtrcFhqb3Jm?=
 =?utf-8?B?QkY5b254V3FTRmlBMWRIaUthRXJMb2ZKVkFOOWVpYWtKd3dLWU9OYlVpOFZm?=
 =?utf-8?B?aVdMcTdBS21vMGFmcmNXbE9RYTRhejZTUFRUWWRoSUNtOGlyemJUNkp1WTND?=
 =?utf-8?B?eWtsV2Njc2JQejNmbXJtUG9XS09Na3lQUURPaVoxaUw4am9XK0tKOGJPV0Nv?=
 =?utf-8?B?eU5yVEZ2eWdjV3ExZjdFcnhvK1ZIMUxBNlp1RkdCVThzTVFXc3NiaWtaQS96?=
 =?utf-8?B?S2daL25rN09WejRrNnRZZWJjcnNRdzA0aTZqQWd5ZHZhSzFIREVGSEN3M3A4?=
 =?utf-8?B?T1JhWmJPaHNHVG1IbXVMZVQ3c1UyUVJkaTQ1QlVQTnlQbjZQeW9jYlNCd29R?=
 =?utf-8?B?UXgycENYdHdsMGhMTmxnd1c2ZE9tQnRMaXcwck1Zb3BaWWdIUC9EcXFhRzV4?=
 =?utf-8?B?SkFvVE94S2VILytLNGpKbG9JUjJyeDhSckE1QTh3bi9vQ2pXbHZPWHlYRmYv?=
 =?utf-8?B?TldHS3IxVC9hT0tyYU0vSExxMGV3Z1dpd2JNTXh0S3NXa1dhemVqZkMxWDlx?=
 =?utf-8?B?Y2hPL2gyU0cvdTVSc2Q1c0NvS2ZQbjJKV1p5VU10R0gxV0ZDMGVrZ2hlckNy?=
 =?utf-8?B?bG03eUFVTEQ1Ukt2elk3ejE3eWdUUzZLZ25CSXo0NzF0aUZLMGozTHRNU3Zp?=
 =?utf-8?B?NmVQMTkwL3MwSXkwYnErZlA5eDZWak80YS8zS01tTHh4OVhFQlJobzdTcHNX?=
 =?utf-8?B?c0krVU1CNHlHM3ZrcEY4eGc2S1lNMUQ4MDE0RmNkUXlKNk1qOHZPUHZDeUN3?=
 =?utf-8?B?VU5xZXVPU0I4K1lxaVhCb0RpUVRVZ2V4TVZHVjdPOVpESS9mT3FxTDloak1B?=
 =?utf-8?B?ZFVwck8wb3JZMnU4V1RIUTVqd0ZCaUVWRmNEUzBHQ2tDeFU1L1NzVitWKzAw?=
 =?utf-8?B?dUM2aSt2SmNRSmt0dmJmaDU5VC9aQUFjWVBIMFE5azR6clM2cnNqdCtKeUhj?=
 =?utf-8?B?akxYcUx0RFZZck5JZ0p3NzZuQVFJcUJ3dUlIcU53bG5Jb2xUb0o4a0JvSU5H?=
 =?utf-8?B?aFlrY0tobGhyT3ZSdDVRN1h5NS9PcVB0YnJWcGRHTlRKRjJVeThRckFjaGdr?=
 =?utf-8?B?NU5ab3ZqaTBjQkNORWRwMXhjekRzc282Q3BHbEkvTzFRcVRSaG9IT0E2dkkz?=
 =?utf-8?Q?D4PHuuiO+XTV6wkdLhDDVM0=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 69593730-baf5-4ea9-ad7a-08dbb4725a30
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB7605.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Sep 2023 15:59:03.7095
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: HauQI6qWoAXp06wfh+tAf6T9Q8tMDOoer3XaBd+pc0x5GK1JCb6e+HkcnFGhDGVqxhPu3DBEmCc8chxj3lDgzuuk6fdKyocWFAS4Suj1Td4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR11MB5964
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Wed, 13 Sep 2023 08:59:22 -0700 (PDT)


On 9/13/2023 5:48 AM, Usyskin, Alexander wrote:
>>> +struct xe_gsc {
>> Please use a different name for this instead of just xe_gsc. In Xe we're
>> likely never going to fully use the GSC via HECI, only the GSCFI/CSC
>> part. In MTL and newer we also have the actual GSC part being split off
>> and placed inside GT (behind the GSCCS), so if we call this just xe_gsc
>> as well it'll be confusing. I suggest calling this something like
>> xe_heci_gsc, xe_heci_interface or something like that. I had actually
>> suggested this for i915 as well
>> (https://patchwork.freedesktop.org/patch/509653/) but Tomas was
>> concerned it might make backporting fixes difficult, so I dropped it;
>> this is not a concern for Xe right now.
>>
>> Note that this means renaming all the exposed functions as well.
>>
>> Daniele
>>
> Sure, will rename it xe_heci_gsc and move to xe_device.
>
>>> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>>> index 1dee3e832eb5..1ae4cc9f1522 100644
>>> --- a/drivers/gpu/drm/xe/xe_irq.c
>>> +++ b/drivers/gpu/drm/xe/xe_irq.c
>>> @@ -128,6 +128,7 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
>>>    	struct xe_device *xe = gt_to_xe(gt);
>>>    	u32 ccs_mask, bcs_mask;
>>>    	u32 irqs, dmask, smask;
>>> +	u32 gsc_mask = GSC_IRQ_INTF(1);
>>>
>>>    	if (xe_device_guc_submission_enabled(xe)) {
>>>    		irqs = GT_RENDER_USER_INTERRUPT |
>>> @@ -180,6 +181,9 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
>>>    		if (xe_hw_engine_mask_per_class(gt,
>> XE_ENGINE_CLASS_OTHER)) {
>>>    			xe_mmio_write32(gt, GUNIT_GSC_INTR_ENABLE,
>> irqs);
>>>    			xe_mmio_write32(gt, GUNIT_GSC_INTR_MASK,
>> ~irqs);
>>> +		} else if (HAS_HECI_GSCFI(xe)) {
>>> +			xe_mmio_write32(gt, GUNIT_GSC_INTR_ENABLE,
>> gsc_mask);
>>> +			xe_mmio_write32(gt, GUNIT_GSC_INTR_MASK,
>> ~gsc_mask);
> Danielle, how this enablement should look when gsc_heci is not gt related?
> I'm somewhat confused here.

This one you should be able to leave as-is, and your check in 
HAS_HECI_GSCFI is already at the xe device level and not at the gt 
level. Unfortunately the mmio writes functions only allow a gt parameter 
for now, so we need to stick with that (I believe there is a plan to 
change it at some point).

What will need to change is xe_gsc_irq_handler(), but you can change 
that to take the heci_gsc as a parameter and then do:

     xe_gsc_irq_handler(&gt_to_xe(gt)->heci_gsc, iir);

Daniele

>