Re: [PATCH] drm/i915: Inline engine->init_context into its caller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 30/07/2019 10:34, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-07-29 15:49:29)

On 29/07/2019 12:37, Chris Wilson wrote:
We only use the init_context vfunc once while recording the default
context state, and we use the same sequence in each backend (eliding
steps that do not apply). Remove the vfunc for simplicity and
de-duplication.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
   drivers/gpu/drm/i915/gt/intel_engine_types.h |  1 -
   drivers/gpu/drm/i915/gt/intel_lrc.c          | 21 --------------------
   drivers/gpu/drm/i915/gt/intel_mocs.c         |  5 ++++-
   drivers/gpu/drm/i915/gt/intel_mocs.h         |  3 ++-
   drivers/gpu/drm/i915/gt/intel_renderstate.c  |  2 +-
   drivers/gpu/drm/i915/gt/intel_ringbuffer.c   | 18 -----------------
   drivers/gpu/drm/i915/i915_gem.c              | 21 +++++++++++++++++---
   7 files changed, 25 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 8be63019d707..da61dd329210 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -383,7 +383,6 @@ struct intel_engine_cs {
       const struct intel_context_ops *cops;
int (*request_alloc)(struct i915_request *rq);
-     int             (*init_context)(struct i915_request *rq);
int (*emit_flush)(struct i915_request *request, u32 mode);
   #define EMIT_INVALIDATE     BIT(0)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 884dfc1cb033..4d7c4d0dbf75 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -141,7 +141,6 @@
   #include "intel_gt.h"
   #include "intel_lrc_reg.h"
   #include "intel_mocs.h"
-#include "intel_renderstate.h"
   #include "intel_reset.h"
   #include "intel_workarounds.h"
@@ -2727,25 +2726,6 @@ static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
       return gen8_emit_wa_tail(request, cs);
   }
-static int gen8_init_rcs_context(struct i915_request *rq)
-{
-     int ret;
-
-     ret = intel_engine_emit_ctx_wa(rq);
-     if (ret)
-             return ret;
-
-     ret = intel_rcs_context_init_mocs(rq);
-     /*
-      * Failing to program the MOCS is non-fatal.The system will not
-      * run at peak performance. So generate an error and carry on.
-      */
-     if (ret)
-             DRM_ERROR("MOCS failed to program: expect performance issues.\n");
-
-     return intel_renderstate_emit(rq);
-}
-
   static void execlists_park(struct intel_engine_cs *engine)
   {
       del_timer_sync(&engine->execlists.timer);
@@ -2853,7 +2833,6 @@ int intel_execlists_submission_setup(struct intel_engine_cs *engine)
       logical_ring_default_irqs(engine);
if (engine->class == RENDER_CLASS) {
-             engine->init_context = gen8_init_rcs_context;
               engine->emit_flush = gen8_emit_flush_render;
               engine->emit_fini_breadcrumb = gen8_emit_fini_breadcrumb_rcs;
       }
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
index 290a5e9b90b9..e082b25d2db1 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -568,11 +568,14 @@ void intel_mocs_init_l3cc_table(struct intel_gt *gt)
    *
    * Return: 0 on success, otherwise the error status.
    */
-int intel_rcs_context_init_mocs(struct i915_request *rq)
+int intel_mocs_emit(struct i915_request *rq)
   {
       struct drm_i915_mocs_table t;
       int ret;
+ if (rq->engine->class != RENDER_CLASS)
+             return 0;
+
       if (get_mocs_settings(rq->engine->gt, &t)) {
               /* Program the RCS control registers */
               ret = emit_mocs_control_table(rq, &t);
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.h b/drivers/gpu/drm/i915/gt/intel_mocs.h
index 8b9813e6f9ac..a334db2d6d6b 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.h
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.h
@@ -54,8 +54,9 @@ struct i915_request;
   struct intel_engine_cs;
   struct intel_gt;
-int intel_rcs_context_init_mocs(struct i915_request *rq);
   void intel_mocs_init_l3cc_table(struct intel_gt *gt);
   void intel_mocs_init_engine(struct intel_engine_cs *engine);
+int intel_mocs_emit(struct i915_request *rq);
+
   #endif
diff --git a/drivers/gpu/drm/i915/gt/intel_renderstate.c b/drivers/gpu/drm/i915/gt/intel_renderstate.c
index 06a8dc40b19f..be37d4501c67 100644
--- a/drivers/gpu/drm/i915/gt/intel_renderstate.c
+++ b/drivers/gpu/drm/i915/gt/intel_renderstate.c
@@ -41,7 +41,7 @@ struct intel_renderstate {
   static const struct intel_renderstate_rodata *
   render_state_get_rodata(const struct intel_engine_cs *engine)
   {
-     if (engine->id != RCS0)
+     if (engine->class != RENDER_CLASS)
               return NULL;
switch (INTEL_GEN(engine->i915)) {
diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
index 1de19dac4a14..5c7f2fdc5ec3 100644
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -37,7 +37,6 @@
   #include "i915_trace.h"
   #include "intel_context.h"
   #include "intel_gt.h"
-#include "intel_renderstate.h"
   #include "intel_reset.h"
   #include "intel_workarounds.h"
@@ -849,21 +848,6 @@ static void reset_finish(struct intel_engine_cs *engine)
   {
   }
-static int intel_rcs_ctx_init(struct i915_request *rq)
-{
-     int ret;
-
-     ret = intel_engine_emit_ctx_wa(rq);
-     if (ret != 0)
-             return ret;
-
-     ret = intel_renderstate_emit(rq);
-     if (ret)
-             return ret;
-
-     return 0;
-}
-
   static int rcs_resume(struct intel_engine_cs *engine)
   {
       struct drm_i915_private *dev_priv = engine->i915;
@@ -2227,11 +2211,9 @@ static void setup_rcs(struct intel_engine_cs *engine)
       engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
if (INTEL_GEN(i915) >= 7) {
-             engine->init_context = intel_rcs_ctx_init;
               engine->emit_flush = gen7_render_ring_flush;
               engine->emit_fini_breadcrumb = gen7_rcs_emit_breadcrumb;
       } else if (IS_GEN(i915, 6)) {
-             engine->init_context = intel_rcs_ctx_init;
               engine->emit_flush = gen6_render_ring_flush;
               engine->emit_fini_breadcrumb = gen6_rcs_emit_breadcrumb;
       } else if (IS_GEN(i915, 5)) {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 01dd0d1d9bf6..65863e955f40 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -50,6 +50,7 @@
   #include "gt/intel_gt_pm.h"
   #include "gt/intel_mocs.h"
   #include "gt/intel_reset.h"
+#include "gt/intel_renderstate.h"
   #include "gt/intel_workarounds.h"
#include "i915_drv.h"
@@ -1294,10 +1295,24 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
                       goto err_active;
               }
- err = 0;
-             if (rq->engine->init_context)
-                     err = rq->engine->init_context(rq);
+             err = intel_engine_emit_ctx_wa(rq);
+             if (err)
+                     goto err_rq;
+
+             /*
+              * Failing to program the MOCS is non-fatal.The system will not
+              * run at peak performance. So warn the user and carry on.
+              */
+             err = intel_mocs_emit(rq);

I think platforms without MOCS will not be happy unless you also change
the MOCS code.

Older platforms fail gracefully (silently with err=0 as get_mocs_settings() returns false).
New platforms without MOCS descriptions issue a warning (as currently).

True, I've missed the fact WARN_ON on the final else branch is conditional on gen >= 9.

+             if (err)
+                     dev_notice(i915->drm.dev,
+                                "Failed to program MOCS registers; expect performance issues.\n");
+
+             err = intel_renderstate_emit(rq);
+             if (err)
+                     goto err_rq;

Would it be prettier to move all the tree into
intel_engine_cs.c/intel_engine_context_init() or something? Here it
seems a bit too much into details.

Indeed, it's not staying here. But that is tied up in other in-flight
patches.

Okay.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux