Re: [Intel-gfx] [PATCH V2 2/5] drm/i915/gt: Use cmd_cctl override for platforms >= gen12
-Original Message- From: Roper, Matthew D Sent: Tuesday, August 17, 2021 3:06 AM To: Siddiqui, Ayaz A Cc: intel-gfx@lists.freedesktop.org; S, Srinivasan ; Wilson, Chris P Subject: Re: [PATCH V2 2/5] drm/i915/gt: Use cmd_cctl override for platforms >= gen12 On Mon, Aug 16, 2021 at 10:22:26AM +0530, Ayaz A Siddiqui wrote: > From: Srinivasan Shanmugam > > Program CMD_CCTL to use a mocs entry for uncached access. > This controls memory accesses by CS as it reads instructions from the > ring and batch buffers. > > v2: Added CMD_CCTL in guc_mmio_regset_init(), so that this register > can restored after engine reset. > > Signed-off-by: Srinivasan Shanmugam > Signed-off-by: Ayaz A Siddiqui > Cc: Chris Wilson > Cc: Matt Roper > --- > drivers/gpu/drm/i915/gt/intel_mocs.c | 96 ++ > drivers/gpu/drm/i915/gt/selftest_mocs.c| 49 +++ > drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 1 + > drivers/gpu/drm/i915/i915_reg.h| 16 > 4 files changed, 162 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c > b/drivers/gpu/drm/i915/gt/intel_mocs.c > index 10cc508c1a4f6..92141cf6f9a79 100644 > --- a/drivers/gpu/drm/i915/gt/intel_mocs.c > +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c > @@ -25,6 +25,15 @@ struct drm_i915_mocs_table { > u8 uc_index; > }; > > +struct drm_i915_aux_table { It's not clear to me exactly what the term "aux table" refers to here. I guess it's just extra context registers (that aren't associated with a workaround) that we want to initialize before the point where the default context gets recorded? Maybe calling it something like "ctx_init_table" would make it more clear what these are for? However a possibly simpler approach would just be to add these registers directly to the ctx workaround list with a comment noting that they're "fake" workarounds and describing what they're for (we already have other similar context programming for disabling fine-grained preemption, disabling nested batchbuffer mode, etc. The benefit of just tossing these on the workaround list is that the settings get automatically verified by the workaround checking that we already have without needing to code up new table management, register readback, value verification, etc. Thanks Matt for comments: The aux table is the separate table, which can be easily dynamically expanded (without disturbing any existing tables of mocs entries for legacy platforms starting from >= gen12 onwards), for any new additions of mocs related registers (like for ex: cmd_cctl) & for its debugging purposes & if required for any other parameters in future easily expandable. As this cmd_cctl register is kind of a new feature & it doesn't seems to be workaround, where currently we are setting default mocs index value to Uncacheable - (which had undefined behavior before programming - where HW team failed to fix this default index to Uncacheable in their hardware & requested for the software driver team) which was meant only for the engines, due to which HW team was seeing some memory related issues, when command streamers where reading instructions from memory & executing. Moreover, since this cmd_cctl was mocs related stuffs - I felt like, it's better to keep tidy & all mocs related stuffs inclined in one place ie., in intel_mocs.c, so that we don't go and search for mocs related stuffs in workarounds file. Though, currently we are only programming cmd_cctl to default uncached mocs index in driver as per the hardware functional requirements. IMO, may be this can be exposed to userspace (except the undefined behaviour)- to control for ex: cacheability & uncacheability behaviour - when command streamers are executing the instructions from memory. And moreover, in the intel_workarounds.c file (as of now , we don't have anything related to mocs related stuffs seen there till so far) - if we still encounter, any mocs related stuffs in future, we can still move from "fake" workarounds to this aux table, so that we don't go and search in the workarounds file for mocs related stuffs. Thanks, Srinivasan S > + const char *name; > + i915_reg_t offset; > + u32 value; > + u32 readmask; > + bool skip_check; > + struct drm_i915_aux_table *next; > +}; > + > /* Defines for the tables (XXX_MOCS_0 - XXX_MOCS_63) */ > #define _LE_CACHEABILITY(value) ((value) << 0) > #define _LE_TGT_CACHE(value) ((value) << 2) > @@ -336,6 +345,86 @@ static bool has_mocs(const struct drm_i915_private *i915) > return !IS_DGFX(i915); > } > > +static struct drm_i915_aux_table * > +add_aux_reg(struct drm_i915_aux_table *aux, > +
Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable
Would the following be appropriate fix? if (connector || connector->base.status == connector_status_connected) { ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr); if (ret) { DRM_ERROR("failed to update payload %d\n", ret); } } Regards, -Original Message- From: dri-devel On Behalf Of Manasi Navare Sent: Wednesday, September 18, 2019 11:55 PM To: Ville Syrjälä Cc: S, Srinivasan ; intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable On Wed, Sep 18, 2019 at 09:11:36PM +0300, Ville Syrjälä wrote: > On Wed, Sep 18, 2019 at 10:50:39AM -0700, Manasi Navare wrote: > > On Wed, Sep 18, 2019 at 07:09:43AM +0530, srinivasa...@intel.com wrote: > > > From: Srinivasan S > > > > > > This patch avoids DP MST payload error message in dmesg, as it is > > > trying to read the payload from the disconnected DP MST device. > > > After the unplug the connector status is disconnected and we > > > should not be looking for the payload and hence remove the error and > > > throw the warning. > > > > > > This details can be found in: > > > https://bugs.freedesktop.org/show_bug.cgi?id=111632 > > > > Please add this link as Bugzilla: > > https://bugs.freedesktop.org/show_bug.cgi?id=111632 after the Sign > > off statement > > > > > > > > Signed-off-by: Srinivasan S > > > --- > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 ++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > index eeeb3f933aa4..5b2278fdf675 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > @@ -215,7 +215,12 @@ static void intel_mst_disable_dp(struct > > > intel_encoder *encoder, > > > > > > ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr); > > > if (ret) { > > > - DRM_ERROR("failed to update payload %d\n", ret); > > > + if (!connector || > > > + connector->base.status != connector_status_connected) { > > > + DRM_WARN("DP MST disconnect\n"); > > > > May be adding this check before calling drm_dp_update_payload_part1() is a > > better idea? > > If the connector is disconnected, why update payload? > > > > Jani, Ville, thoughts? > > Or just convert it to a debug? Sure that will work, but do we really want to update the payload if the connector status is disconnected. So shouldnt checking that before calling the function be a better fix? Manasi > > > > > Regards > > Manasi > > > > > + } else { > > > + DRM_ERROR("failed to update payload %d\n", ret); > > > + } > > > } > > > if (old_crtc_state->has_audio) > > > intel_audio_codec_disable(encoder, > > > -- > > > 2.7.4 > > > > > -- > Ville Syrjälä > Intel > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable
Then it's better that, could we change it to DRM_DEBUG_KMS("failed to update payload %d\n", ret); instead of DRM_ERROR("failed to update payload %d\n", ret);, without any connector status check, would that be fine? Regards, -Original Message- From: Ville Syrjälä Sent: Thursday, September 19, 2019 5:34 PM To: S, Srinivasan Cc: Navare, Manasi D ; intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable On Thu, Sep 19, 2019 at 07:23:30AM +, S, Srinivasan wrote: > Would the following be appropriate fix? > > if (connector || connector->base.status == > connector_status_connected) { > ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr); > if (ret) { > DRM_ERROR("failed to update payload %d\n", ret); > } > } The whole connector->status check is racy. IMO don't do it. > > Regards, > -Original Message- > From: dri-devel On Behalf Of > Manasi Navare > Sent: Wednesday, September 18, 2019 11:55 PM > To: Ville Syrjälä > Cc: S, Srinivasan ; > intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org > Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging > TypeC cable > > On Wed, Sep 18, 2019 at 09:11:36PM +0300, Ville Syrjälä wrote: > > On Wed, Sep 18, 2019 at 10:50:39AM -0700, Manasi Navare wrote: > > > On Wed, Sep 18, 2019 at 07:09:43AM +0530, srinivasa...@intel.com wrote: > > > > From: Srinivasan S > > > > > > > > This patch avoids DP MST payload error message in dmesg, as it > > > > is trying to read the payload from the disconnected DP MST device. > > > > After the unplug the connector status is disconnected and we > > > > should not be looking for the payload and hence remove the error and > > > > throw the warning. > > > > > > > > This details can be found in: > > > > https://bugs.freedesktop.org/show_bug.cgi?id=111632 > > > > > > Please add this link as Bugzilla: > > > https://bugs.freedesktop.org/show_bug.cgi?id=111632 after the Sign > > > off statement > > > > > > > > > > > Signed-off-by: Srinivasan S > > > > --- > > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 ++- > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > index eeeb3f933aa4..5b2278fdf675 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > @@ -215,7 +215,12 @@ static void intel_mst_disable_dp(struct > > > > intel_encoder *encoder, > > > > > > > > ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr); > > > > if (ret) { > > > > - DRM_ERROR("failed to update payload %d\n", ret); > > > > + if (!connector || > > > > + connector->base.status != > > > > connector_status_connected) { > > > > + DRM_WARN("DP MST disconnect\n"); > > > > > > May be adding this check before calling drm_dp_update_payload_part1() is > > > a better idea? > > > If the connector is disconnected, why update payload? > > > > > > Jani, Ville, thoughts? > > > > Or just convert it to a debug? > > Sure that will work, but do we really want to update the payload if the > connector status is disconnected. > So shouldnt checking that before calling the function be a better fix? > > Manasi > > > > > > > > > Regards > > > Manasi > > > > > > > + } else { > > > > + DRM_ERROR("failed to update payload %d\n", ret); > > > > + } > > > > } > > > > if (old_crtc_state->has_audio) > > > > intel_audio_codec_disable(encoder, > > > > -- > > > > 2.7.4 > > > > > > > > -- > > Ville Syrjälä > > Intel > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable
Hi Ville, I have revised the patch from DRM_ERROR to DRM_DEBUG, could you please review? https://patchwork.freedesktop.org/patch/332806/?series=66837&rev=3 Thanks, -Original Message- From: S, Srinivasan Sent: Thursday, September 19, 2019 7:22 PM To: 'Ville Syrjälä' Cc: Navare, Manasi D ; 'intel-gfx@lists.freedesktop.org' ; 'dri-de...@lists.freedesktop.org' Subject: RE: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable Then it's better that, could we change it to DRM_DEBUG_KMS("failed to update payload %d\n", ret); instead of DRM_ERROR("failed to update payload %d\n", ret);, without any connector status check, would that be fine? Regards, -Original Message- From: Ville Syrjälä Sent: Thursday, September 19, 2019 5:34 PM To: S, Srinivasan Cc: Navare, Manasi D ; intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable On Thu, Sep 19, 2019 at 07:23:30AM +, S, Srinivasan wrote: > Would the following be appropriate fix? > > if (connector || connector->base.status == > connector_status_connected) { > ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr); > if (ret) { > DRM_ERROR("failed to update payload %d\n", ret); > } > } The whole connector->status check is racy. IMO don't do it. > > Regards, > -Original Message- > From: dri-devel On Behalf Of > Manasi Navare > Sent: Wednesday, September 18, 2019 11:55 PM > To: Ville Syrjälä > Cc: S, Srinivasan ; > intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org > Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging > TypeC cable > > On Wed, Sep 18, 2019 at 09:11:36PM +0300, Ville Syrjälä wrote: > > On Wed, Sep 18, 2019 at 10:50:39AM -0700, Manasi Navare wrote: > > > On Wed, Sep 18, 2019 at 07:09:43AM +0530, srinivasa...@intel.com wrote: > > > > From: Srinivasan S > > > > > > > > This patch avoids DP MST payload error message in dmesg, as it > > > > is trying to read the payload from the disconnected DP MST device. > > > > After the unplug the connector status is disconnected and we > > > > should not be looking for the payload and hence remove the error and > > > > throw the warning. > > > > > > > > This details can be found in: > > > > https://bugs.freedesktop.org/show_bug.cgi?id=111632 > > > > > > Please add this link as Bugzilla: > > > https://bugs.freedesktop.org/show_bug.cgi?id=111632 after the Sign > > > off statement > > > > > > > > > > > Signed-off-by: Srinivasan S > > > > --- > > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 ++- > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > index eeeb3f933aa4..5b2278fdf675 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > @@ -215,7 +215,12 @@ static void intel_mst_disable_dp(struct > > > > intel_encoder *encoder, > > > > > > > > ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr); > > > > if (ret) { > > > > - DRM_ERROR("failed to update payload %d\n", ret); > > > > + if (!connector || > > > > + connector->base.status != > > > > connector_status_connected) { > > > > + DRM_WARN("DP MST disconnect\n"); > > > > > > May be adding this check before calling drm_dp_update_payload_part1() is > > > a better idea? > > > If the connector is disconnected, why update payload? > > > > > > Jani, Ville, thoughts? > > > > Or just convert it to a debug? > > Sure that will work, but do we really want to update the payload if the > connector status is disconnected. > So shouldnt checking that before calling the function be a better fix? > > Manasi > > > > > > > > > Regards > > > Manasi > > > > > > > + } else { > > > > + DRM_ERROR("failed to update payload %d\n", ret); > > > > + } > > > > } > > > > if (old_crtc_state->has_audio) > > > > intel_audio_codec_disable(encoder, > > > > -- > > > > 2.7.4 > > > > > > > > -- > > Ville Syrjälä > > Intel > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable
Could anyone please review the patch below & let me know if any other ideas please? https://patchwork.freedesktop.org/patch/332806/?series=66837&rev=3 Thanks, > -Original Message----- > From: S, Srinivasan > Sent: Wednesday, September 25, 2019 8:33 PM > To: 'Ville Syrjälä' > Cc: Navare, Manasi D ; 'intel- > g...@lists.freedesktop.org' ; 'dri- > de...@lists.freedesktop.org' > Subject: RE: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC > cable > > Hi Ville, > > I have revised the patch from DRM_ERROR to DRM_DEBUG, could you please > review? > > https://patchwork.freedesktop.org/patch/332806/?series=66837&rev=3 > > Thanks, > > -Original Message- > From: S, Srinivasan > Sent: Thursday, September 19, 2019 7:22 PM > To: 'Ville Syrjälä' > Cc: Navare, Manasi D ; 'intel- > g...@lists.freedesktop.org' ; 'dri- > de...@lists.freedesktop.org' > Subject: RE: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC > cable > > Then it's better that, could we change it to DRM_DEBUG_KMS("failed to update > payload %d\n", ret); instead of DRM_ERROR("failed to update payload %d\n", > ret);, without any connector status check, would that be fine? > > Regards, > -Original Message- > From: Ville Syrjälä > Sent: Thursday, September 19, 2019 5:34 PM > To: S, Srinivasan > Cc: Navare, Manasi D ; intel- > g...@lists.freedesktop.org; dri-de...@lists.freedesktop.org > Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC > cable > > On Thu, Sep 19, 2019 at 07:23:30AM +, S, Srinivasan wrote: > > Would the following be appropriate fix? > > > > if (connector || connector->base.status == > > connector_status_connected) > { > > ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr); > > if (ret) { > > DRM_ERROR("failed to update payload %d\n", ret); > > } > > } > > The whole connector->status check is racy. IMO don't do it. > > > > > Regards, > > -Original Message- > > From: dri-devel On Behalf Of > > Manasi Navare > > Sent: Wednesday, September 18, 2019 11:55 PM > > To: Ville Syrjälä > > Cc: S, Srinivasan ; > > intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org > > Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging > > TypeC cable > > > > On Wed, Sep 18, 2019 at 09:11:36PM +0300, Ville Syrjälä wrote: > > > On Wed, Sep 18, 2019 at 10:50:39AM -0700, Manasi Navare wrote: > > > > On Wed, Sep 18, 2019 at 07:09:43AM +0530, srinivasa...@intel.com > wrote: > > > > > From: Srinivasan S > > > > > > > > > > This patch avoids DP MST payload error message in dmesg, as it > > > > > is trying to read the payload from the disconnected DP MST device. > > > > > After the unplug the connector status is disconnected and we > > > > > should not be looking for the payload and hence remove the error and > throw the warning. > > > > > > > > > > This details can be found in: > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=111632 > > > > > > > > Please add this link as Bugzilla: > > > > https://bugs.freedesktop.org/show_bug.cgi?id=111632 after the Sign > > > > off statement > > > > > > > > > > > > > > Signed-off-by: Srinivasan S > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 ++- > > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > > index eeeb3f933aa4..5b2278fdf675 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > > @@ -215,7 +215,12 @@ static void intel_mst_disable_dp(struct > > > > > intel_encoder *encoder, > > > > > > > > > > ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr); > > > > > if (ret) { > > > > > - DRM_ERROR("failed to update payload %d\n", ret); > > > > > + if (!connector || > > > > > + conne
Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable
Thanks a lot Manasi, Ville, Mika, Jani, Lakshmi, for all your time in reviewing this patch. Best Regards, > -Original Message- > From: dri-devel On Behalf Of Ville > Syrjälä > Sent: Tuesday, October 1, 2019 5:31 PM > To: S, Srinivasan > Cc: Navare, Manasi D ; intel- > g...@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Vudum, > Lakshminarayana > Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC > cable > > On Wed, Sep 25, 2019 at 06:05:42AM +0530, srinivasa...@intel.com wrote: > > From: Srinivasan S > > > > This patch avoids DP MST payload error message in dmesg, as it is trying > > to update the payload to the disconnected DP MST device. After DP MST > > device is disconnected we should not be updating the payload and > > hence remove the error. > > > > v2: Removed the connector status check and converted from error to debug. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111632 > > Signed-off-by: Srinivasan S > > Pushed to dinq. Thanks for the patch. > > PS. Next time please use --in-reply-to when sending an updated patch > so that it's easier to keep track of the discussion. > > > --- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > index eeeb3f933aa4..497a6ae0d2c0 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > @@ -215,7 +215,7 @@ static void intel_mst_disable_dp(struct intel_encoder > *encoder, > > > > ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr); > > if (ret) { > > - DRM_ERROR("failed to update payload %d\n", ret); > > + DRM_DEBUG_KMS("failed to update payload %d\n", ret); > > } > > if (old_crtc_state->has_audio) > > intel_audio_codec_disable(encoder, > > -- > > 2.7.4 > > -- > Ville Syrjälä > Intel > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 0/3] Dynamic EU configuration of Slice/Sub-slice/EU
> -Original Message- > From: Tvrtko Ursulin > Sent: Friday, March 13, 2020 10:48 PM > To: S, Srinivasan ; intel-gfx@lists.freedesktop.org; > ch...@chris-wilson.co.uk; Francisco Jerez > Subject: Re: [Intel-gfx] [PATCH v7 0/3] Dynamic EU configuration of Slice/Sub- > slice/EU > > > Hi, > > On 13/03/2020 11:12, srinivasa...@intel.com wrote: > > From: Srinivasan S > > > >drm/i915: Context aware user agnostic EU/Slice/Sub-slice control > > within > kernel > > > > This patch sets improves GPU power consumption on Linux kernel based OS > such as > > Chromium OS, Ubuntu, etc. Following are the power savings. > > > > Power savings on GLK-GT1 Bobba platform running on Chrome OS. > > ---| > > App /KPI| % Power Benefit (mW) | > > |--| > > Hangout Call- 20 minute | 1.8% | > > Youtube 4K VPB | 14.13% | > > WebGL Aquarium | 13.76% | > > Unity3D | 6.78% | > > | | > > |--| > > Chrome PLT | BatteryLife Improves | > > | by ~45 minute| > > ---| > > > > Power savings on KBL-GT3 running on Android and Ubuntu (Linux). > > ---| > > App /KPI| % Power Benefit (mW) | > > |--| > > | Android | Ubuntu | > > |--|---| > > 3D Mark (Ice storm) | 2.30%| N.A. | > > TRex On screen | 2.49%| 2.97% | > > Manhattan On screen | 3.11%| 4.90% | > > Carchase On Screen | N.A. | 5.06% | > > AnTuTu 6.1.4| 3.42%| N.A. | > > SynMark2| N.A. | 1.7% | > > ---| > > Have a look at the result Francisco obtained on Icelake with a different > approach: https://patchwork.freedesktop.org/series/74540/ > > Not all benchmarks overlap but if you are set up to easily test his > patches it may be for a mutual benefit. [S, Srinivasan] Thanks!, Could we reuse only his gfx related patches alone as below - to focus on our gfx power saving benefits as first step, or could you please let me know the entire patch series needs to be considered or is there any dependency of GPU power on CPU? ie., https://patchwork.freedesktop.org/patch/357098/?series=74540&rev=2 https://patchwork.freedesktop.org/patch/357103/?series=74540&rev=2 > > Regards, > > Tvrtko > > > We have also observed GPU core residencies improves by 1.035%. > > > > Technical Insights of the patch: > > Current GPU configuration code for i915 does not allow us to change > > EU/Slice/Sub-slice configuration dynamically. Its done only once while > > context > > is created. > > > > While particular graphics application is running, if we examine the command > > requests from user space, we observe that command density is not consistent. > > It means there is scope to change the graphics configuration dynamically > > even > > while context is running actively. This patch series proposes the solution > > to > > find the active pending load for all active context at given time and based > > on > > that, dynamically perform graphics configuration for each context. > > > > We use a hr (high resolution) timer with i915 driver in kernel to get a > > callback every few milliseconds (this timer value can be configured through > > debugfs, default is '0' indicating timer is in disabled state i.e. original > > system without any intervention).In the timer callback, we examine pending > > commands for a context in the queue, essentially, we intercept them before > > they are executed by GPU and we update context with required number of > EUs. > > > > Two questions, how did we arrive at right timer value? and what's the right > > number of EUs? For the prior one, empirical data to achieve best performance > > in least power was considered. For the later one, we roughly categorized > number > > of EUs logically based on platform. Now we compare number of pending > commands > > with a particular threshold and then set number of EUs accordingly with > update > > context. That threshold is also based on experiments & findings. If GPU is &
Re: [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Dynamic EU configuration of Slice/Sub-slice/EU (rev6)
May I know any specific timelines? > -Original Message- > From: Navik, Ankit P > Sent: Saturday, March 14, 2020 10:03 AM > To: intel-gfx@lists.freedesktop.org > Cc: S, Srinivasan ; Ursulin, Tvrtko > ; Kumar, Purushotam > ; Chelladurai, Paul S > > Subject: RE: ✗ Fi.CI.SPARSE: warning for Dynamic EU configuration of > Slice/Sub- > slice/EU (rev6) > > Hi Srinivas, > > This will break OA counter. > I am already working with Tvrtko to make patch scalable > and to make it as per the flow. > Kindly wait for upcoming patch. > > PS: Coding guideline: > https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html > > Regards, > Ankit > > > -Original Message- > > From: Patchwork > > Sent: Friday, March 13, 2020 7:34 PM > > To: Navik, Ankit P > > Cc: intel-gfx@lists.freedesktop.org > > Subject: ✗ Fi.CI.SPARSE: warning for Dynamic EU configuration of Slice/Sub- > > slice/EU (rev6) > > > > == Series Details == > > > > Series: Dynamic EU configuration of Slice/Sub-slice/EU (rev6) > > URL : https://patchwork.freedesktop.org/series/69980/ > > State : warning > > > > == Summary == > > > > $ dim sparse origin/drm-tip > > Sparse version: v0.6.0 > > Commit: drm/i915: Get active pending request for given context > > +drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61:expected struct atomic_t > const > > [usertype] *v > > +drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61:got struct atomic_t > > [noderef] > > * > > +drivers/gpu/drm/i915/gt/intel_lrc.c:2162:61: warning: incorrect type in > > argument 1 (different address spaces) > > +drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64:expected struct atomic_t > > [usertype] *v > > +drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64:got struct atomic_t > > [noderef] > > * > > +drivers/gpu/drm/i915/gt/intel_lrc.c:2163:64: warning: incorrect type in > > +argument 1 (different address spaces) > > > > Commit: drm/i915: set optimum eu/slice/sub-slice configuration based on load > > type > > +drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59:expected struct > > i915_gem_context *ctx > > +drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59:got struct i915_gem_context > > [noderef] *const gem_context > > +drivers/gpu/drm/i915/gt/intel_lrc.c:3047:59: warning: incorrect type in > > +argument 1 (different address spaces) > > +drivers/gpu/drm/i915/gt/intel_lrc.c:3077:15: warning: dereference of > > +noderef expression > > +drivers/gpu/drm/i915/gt/intel_lrc.c:3077:45: warning: dereference of > > +noderef expression > > +drivers/gpu/drm/i915/gt/intel_lrc.c:3078:19: warning: dereference of > > +noderef expression > > +drivers/gpu/drm/i915/gt/intel_lrc.c:3078:48: warning: dereference of > > +noderef expression > > > > Commit: drm/i915: Predictive governor to control slice/subslice/eu Okay! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx