From patchwork Wed Nov 29 10:47:57 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: sagar.a.kamble@intel.com X-Patchwork-Id: 10081835 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9043F60234 for ; Wed, 29 Nov 2017 10:48:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9062729731 for ; Wed, 29 Nov 2017 10:48:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 836082973F; Wed, 29 Nov 2017 10:48:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id B18A229731 for ; Wed, 29 Nov 2017 10:48:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5C37D6E8BB; Wed, 29 Nov 2017 10:48:03 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id BB6F06E8BB for ; Wed, 29 Nov 2017 10:48:01 +0000 (UTC) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Nov 2017 02:48:01 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,472,1505804400"; d="scan'208";a="7165480" Received: from sakamble-mobl.gar.corp.intel.com (HELO [10.223.178.72]) ([10.223.178.72]) by FMSMGA003.fm.intel.com with ESMTP; 29 Nov 2017 02:47:58 -0800 To: Yaodong Li , Michal Wajdeczko , intel-gfx@lists.freedesktop.org References: <1510766229-19062-1-git-send-email-yaodong.li@intel.com> <8e50f3ab-6f67-4c29-718c-e67d56fd9c1c@intel.com> <645933a4-4a47-f89d-9421-e4d5df8a57d3@intel.com> From: Sagar Arun Kamble Message-ID: <140385c1-775b-0ee0-1ec0-e90cb3e06143@intel.com> Date: Wed, 29 Nov 2017 16:17:57 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US Cc: Sujaritha Sundaresan Subject: Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: Impletments dynamic WOPCM partitioning. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP On 11/29/2017 6:31 AM, Yaodong Li wrote: > On 11/16/2017 08:00 PM, Sagar Arun Kamble wrote: >> >> >> On 11/17/2017 3:17 AM, Michal Wajdeczko wrote: >>> On Thu, 16 Nov 2017 08:34:01 +0100, Sagar Arun Kamble >>> wrote: >>> >>>> Typo in the subject. >>>> GLK showing failure to load GuC with this approach on CI. >>>> >>>> On 11/15/2017 10:47 PM, Jackie Li wrote: >>>>> Static WOPCM partitioning will lead to GuC loading failure >>>>> if the GuC/HuC firmware size exceeded current static 512KB >>>>> partition boundary. >>>>> >>>>> This patch enables the dynamical calculation of the WOPCM aperture >>>>> used by GuC/HuC firmware. GuC WOPCM offset is  set to >>>>> HuC size + reserved WOPCM size. GuC WOPCM size is set to >>>>> total WOPCM size - GuC WOPCM offset - RC6CTX size. In this case, >>>>> GuC WOPCM offset will be updated based on the size of HuC firmware >>>>> while GuC WOPCM size will be set to use all the remaining WOPCM >>>>> space. >>>>> >>>>> v2: >>>>>   - Removed intel_wopcm_init (Ville/Sagar/Joonas) >>>>>   - Renamed and Moved the intel_wopcm_partition into intel_guc >>>>> (Sagar) >>>>>   - Removed unnecessary function calls (Joonas) >>>>>   - Init GuC WOPCM partition as soon as firmware fetching is >>>>> completed >>>>> >>>>> Signed-off-by: Jackie Li >>>>> +    err = do_wopcm_partition(i915, offset + huc_size, reserved); >>>>> +    if (err) { >>>>> +        if (!huc_size) >>>>> +            goto fail; >>>>> + >>>>> +        /* Partition without loading HuC FW. */ >>>>> +        err = do_wopcm_partition(i915, offset, reserved); >>>>> +        if (err) >>>>> +            goto fail; >>>>> +    } >>>>> + >>>>> +    /* >>>>> +     * Check hardware restriction on Gen9 >>>>> +     * GuC WOPCM size is at least 4 bytes larger than GuC WOPCM >>>>> base due >>>>> +     * to hardware limitation on Gen9. >>>>> +     */ >>>>> +    if (IS_GEN9(i915)) { >>>>> +        wopcm_base = wp->offset + GEN9_GUC_WOPCM_OFFSET; >>>>> +        if (unlikely(wopcm_base > wp->size)) >>>>> +            goto fail; >>>>> + >>>>> +        delta = wp->size - wopcm_base; >>>>> +        if (unlikely(delta < GEN9_GUC_WOPCM_DELTA)) >>>>> +            goto fail; >>>>> +    } >>>>> + >>>>> +    if (guc_fw->fetch_status == INTEL_UC_FIRMWARE_SUCCESS) { >>>>> +        guc_size = guc_fw->header_size + guc_fw->ucode_size; >>>>> +        /* Need 8K stack for GuC */ >>>>> +        guc_size += GUC_WOPCM_STACK_RESERVED; >>>>> +    } >>>>> + >>>>> +    if (guc_size > wp->size) >>>>> +        goto fail; >>>>> + >>>>> +    DRM_DEBUG_DRIVER("GuC WOPCM offset %dKB, size %dKB, top %dKB\n", >>>>> +        wp->offset >> 10, wp->size >> 10, wp->top >> 10); >>>>> + >>>>> +    return; >>>>> + >>>>> +fail: >>>>> +    DRM_ERROR("WOPCM partitioning failed. Falling back to >>>>> execlist mode\n"); >>>>> + >>>>> +    intel_uc_fini_fw(i915); >> This is correct but should be handled from intel_uc_init_fw with this >> function returning status. > it's a good idea. I will do it. Something like this will be good >>>>> +    if (i915_modparams.enable_guc_submission) >>>>> +        i915_modparams.enable_guc_submission = 0; >>>>> + >>>>> +    i915_modparams.enable_guc_loading = 0; >>>>> +} >> This sanitization will be handled by intel_guc_fw_upload during >> intel_uc_init_hw so not needed. > It's too late to do it until intel_uc_init_hw. Yes. Let us not do wopcm_init in uc_init_hw as wopcm_init is one time task. This modparam update is correct. > what I wanted to guarantee here was guc submission > would be disabled as long as the partitioning failed, so that we won't > need to allocate the kernel > context above guc wopcm top. we sure can ignore the partitioning > failure can continue allocating > the context above guc wopcm top, but the problem is we don't have a > valid guc wopcm top value > if the partitioning was failed. another benefit to disable the guc > here is we won't even bother to call > down into the logics in intel_uc_init_hw since we've already known for > sure. I cannot enable guc > submission on the partitioning failure. Agree. >>>>> + >>>>>   void intel_uc_init_fw(struct drm_i915_private *dev_priv) >>>>>   { >>>>>       intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw); >>>>>       intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw); >>>>> +    guc_init_wopcm_partition(dev_priv); >>>> Create intel_uc_init_wopcm_partition(dev_priv) and call >>>> intel_guc_init_wopcm_partition(guc) from there. >>> >>> hmm, I'm not sure what benefit you expect from this call chain ? >>> >> wanted to avoid firmware specific calls from here but I was wrong as >> we are not expecting this function to be called from >> outside of intel_uc. sorry. >>>>>   } >>>>>     void intel_uc_fini_fw(struct drm_i915_private *dev_priv) >>>>> @@ -174,9 +286,9 @@ int intel_uc_init_hw(struct drm_i915_private >>>>> *dev_priv) >>>>>       } >>>>>         /* init WOPCM */ >>>>> -    I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv)); >>>>> +    I915_WRITE(GUC_WOPCM_SIZE, guc->wopcm.size); >>>>>       I915_WRITE(DMA_GUC_WOPCM_OFFSET, >>>>> -           GUC_WOPCM_OFFSET_VALUE | HUC_LOADING_AGENT_GUC); >>>>> +           guc->wopcm.offset | HUC_LOADING_AGENT_GUC); >>>>>         /* WaEnableuKernelHeaderValidFix:skl */ >>>>>       /* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */ >>>>> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c >>>>> b/drivers/gpu/drm/i915/intel_uc_fw.c >>>>> index 4bc82d3..34db2b1 100644 >>>>> --- a/drivers/gpu/drm/i915/intel_uc_fw.c >>>>> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c >>>>> @@ -95,9 +95,13 @@ void intel_uc_fw_fetch(struct drm_i915_private >>>>> *dev_priv, >>>>>       uc_fw->ucode_offset = uc_fw->header_offset + >>>>> uc_fw->header_size; >>>>>       uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * >>>>> sizeof(u32); >>>>>   -    /* Header and uCode will be loaded to WOPCM */ >>>>> +    /* >>>>> +     * Header and uCode will be loaded to WOPCM >>>>> +     * Only check the size against the overall available WOPCM >>>>> here. Will >>>>> +     * continue to check the size during WOPCM partition >>>>> calculation. >>>>> +     */ >>>>>       size = uc_fw->header_size + uc_fw->ucode_size; >>>>> -    if (size > intel_guc_wopcm_size(dev_priv)) { >>>>> +    if (size > WOPCM_DEFAULT_SIZE) { >>>>>           DRM_WARN("%s: Firmware is too large to fit in WOPCM\n", >>>>>                intel_uc_fw_type_repr(uc_fw->type)); >>>>>           err = -E2BIG; >>>>> @@ -207,6 +211,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, >>>>>                  int (*xfer)(struct intel_uc_fw *uc_fw, >>>>>                      struct i915_vma *vma)) >>>>>   { >>>>> +    struct drm_i915_private *i915 = to_i915(uc_fw->obj->base.dev); >>>>>       struct i915_vma *vma; >>>>>       int err; >>>>>   @@ -230,7 +235,7 @@ int intel_uc_fw_upload(struct intel_uc_fw >>>>> *uc_fw, >>>>>       } >>>>>         vma = i915_gem_object_ggtt_pin(uc_fw->obj, NULL, 0, 0, >>>>> -                       PIN_OFFSET_BIAS | GUC_WOPCM_TOP); >>>>> +                       PIN_OFFSET_BIAS | i915->guc.wopcm.top); >>>>>       if (IS_ERR(vma)) { >>>>>           err = PTR_ERR(vma); >>>>>           DRM_DEBUG_DRIVER("%s fw ggtt-pin err=%d\n", >> > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 1e2a30a..04c45d0 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -90,6 +90,15 @@ void intel_uc_init_fw(struct drm_i915_private *dev_priv)  {         intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw);         intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw); + +       ret = intel_uc_init_wopcm_partition(dev_priv); +       if (ret) { +               intel_uc_fw_fini(&dev_priv->guc.fw); +               intel_uc_fw_fini(&dev_priv->huc.fw); +               i915_modparams.enable_guc_loading = 0; +               i915_modparams.enable_guc_submission = 0; +               i915_modparams.guc_log_level = -1; +       }  } >>>>> +    /* GuC submission won't work without valid GuC WOPCM >>>>> partition */