[PATCH v3 05/16] drm/i915: Disable the "binder"

[Nirmoy Das]

On 1/19/2024 11:47 AM, Nirmoy Das wrote:
>
>
> On 1/19/2024 12:12 AM, Ville Syrjälä wrote:
>> On Wed, Jan 17, 2024 at 06:46:24PM +0100, Nirmoy Das wrote:
>>> On 1/17/2024 3:13 PM, Michał Winiarski wrote:
>>>> On Tue, Jan 16, 2024 at 09:56:25AM +0200, Ville Syrjala wrote:
>>>>> From: Ville Syrjälä<ville.syrjala at linux.intel.com>
>>>>>
>>>>> Now that the GGTT PTE updates go straight to GSMBASE (bypassing
>>>>> GTTMMADR) there should be no more risk of system hangs? So the
>>>>> "binder" (ie. update the PTEs via MI_UPDATE_GTT) is no longer
>>>>> necessary, disable it.
>>>>>
>>>>> My main worry with the MI_UPDATE_GTT are:
>>>>> - only used on this one platform so very limited testing coverage
>>>>> - async so more opprtunities to screw things up
>>>>> - what happens if the engine hangs while we're waiting for MI_UPDATE_GTT
>>>>>     to finish?
>>>>> - requires working command submission, so even getting a working
>>>>>     display now depends on a lot more extra components working correctly
>>>>>
>>>>> TODO: MI_UPDATE_GTT might be interesting as an optimization
>>>>> though, so perhaps someone should look into always using it
>>>>> (assuming the GPU is alive and well)?
>>>>>
>>>>> v2: Keep using MI_UPDATE_GTT on VM guests
>>>>>
>>>>> Cc: Paz Zcharya<pazz at chromium.org>
>>>>> Cc: Nirmoy Das<nirmoy.das at intel.com>
>>>>> Signed-off-by: Ville Syrjälä<ville.syrjala at linux.intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/gt/intel_gtt.c | 3 ++-
>>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
>>>>> index 86f73fe558ca..e83dabc56a14 100644
>>>>> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
>>>>> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
>>>>> @@ -24,7 +24,8 @@
>>>>>    bool i915_ggtt_require_binder(struct drm_i915_private *i915)
>>>>>    {
>>>>>    	/* Wa_13010847436 & Wa_14019519902 */
>>>>> -	return MEDIA_VER_FULL(i915) == IP_VER(13, 0);
>>>>> +	return i915_run_as_guest() &&
>>>>> +		MEDIA_VER_FULL(i915) == IP_VER(13, 0);
>>>> Note that i915_run_as_guest() is not the most reliable way to decide
>>>> whether to use MI_UPDATE_GTT or straight to GSMBASE, as it requires the
>>>> hypervisor to "opt-in" and set the X86_FEATURE_HYPERVISOR.
>>>> If it's not set - the driver will go into GSMBASE, which is not mapped
>>>> inside the guest.
>>>> Does the system firmware advertise whether GSMBASE is "open" or "closed"
>>>> to CPU access in any way?
>>> Had a chat with David from IVE team, David suggested to read 0x138914 to
>>> determine that.  "GOP needs to qualify the WA by reading GFX MMIO offset
>>> 0x138914 and verify the value there is 0x1." -> as per the HSD-22018444074
>> OK, so we can confirm the firmware is on board. I suppose no real harm
>> in doing so even though it would clearly be a rather weird if someone
>> would ship some ancient firmware that doesn't handle this.
>>
>> But that still won't help with the guest side handling because that
>> register will read the same in the guest.
>
>
> We are back to the same question :/ How about
> if (boot_cpu_has(X86_FEATURE_HYPERVISOR) && !i915_run_as_guest()
>
hmm, never mind that was stupid.


> disable binder
>
> Regards,
>
> Nirmoy
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20240119/0fe235ac/attachment.htm>