On Mon, 21 Mar 2022, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote:
> On Mon, Mar 21, 2022 at 04:34:49PM -0700, Casey Bowman wrote:
>>Wanted to ping this older thread to find out where we stand with this patch,
>>Are we OK with the current state of these changes?
>>
>>With more recent information gathered from feedback on other patches, would
>>we prefer changing this to a more arch-neutral control flow?
>>
>>e.g.
>>#if IS_ENABLED(CONFIG_X86)
>>...
>>#else
>>...
>>#endif
>>
>>Would we also prefer this RFC series be merged or would it be preferred to
>>create a new series instead?
>
> for this specific function, that is used in only 2 places I think it's
> ok to do:
>
> static inline bool run_as_guest(void)
> {
> #if IS_ENABLED(CONFIG_X86)
> return !hypervisor_is_type(X86_HYPER_NATIVE);
> #else
> /* Not supported yet */
> return false;
> #endif
> }
>
> For PCH it doesn't really matter as we don't execute that function
> for discrete. For intel_vtd_active() I figure anything other than
> x86 would be fine with false here.
>
> Jani, that this look good to you?
It's more important to me to get this out of i915_drv.h, which is not
supposed to be a collection of random stuff anymore. I've sent patches
to this effect but they've stalled a bit.
In general I like the preprocessor control flow outside of functions,
i.e. completely separate function definitions, but for one-line function
implementations I guess this is fine. This is less important for me than
the first point.
BR,
Jani.
>
> Lucas De Marchi
>
>>
>>Regards,
>>Casey
>>
>>On 2/15/22 15:41, Casey Bowman wrote:
>>>Splitting out run_as_guest into platform-specific functions
>>>as arm64 does not support this functionality.
>>>
>>>Signed-off-by: Casey Bowman <casey.g.bowman@xxxxxxxxx>
>>>---
>>> drivers/gpu/drm/i915/i915_drv.h | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>index 1bca510a946d..fdec2b025540 100644
>>>--- a/drivers/gpu/drm/i915/i915_drv.h
>>>+++ b/drivers/gpu/drm/i915/i915_drv.h
>>>@@ -1381,10 +1381,18 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>>> #define INTEL_DISPLAY_ENABLED(dev_priv) \
>>> (drm_WARN_ON(&(dev_priv)->drm, !HAS_DISPLAY(dev_priv)), !(dev_priv)->params.disable_display)
>>>+#if IS_ENABLED(CONFIG_X86)
>>> static inline bool run_as_guest(void)
>>> {
>>> return !hypervisor_is_type(X86_HYPER_NATIVE);
>>> }
>>>+#elif IS_ENABLED(CONFIG_ARM64)
>>>+static inline bool run_as_guest(void)
>>>+{
>>>+ /* Not supported for arm64, so we return false */
>>>+ return false;
>>>+}
>>>+#endif
>>> #define HAS_D12_PLANE_MINIMIZATION(dev_priv) (IS_ROCKETLAKE(dev_priv) || \
>>> IS_ALDERLAKE_S(dev_priv))
>>
--
Jani Nikula, Intel Open Source Graphics Center
