[PATCH 09/16] drm/amd/display: fbc state could not reach while enable fbc

Deucher, Alexander Alexander.Deucher at amd.com
Thu Nov 29 16:27:12 UTC 2018


Do you think this will fix this bug?

https://bugs.freedesktop.org/show_bug.cgi?id=108577

If so, we can re-enable fbc.


Alex

________________________________
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of sunpeng.li at amd.com <sunpeng.li at amd.com>
Sent: Thursday, November 29, 2018 10:52:16 AM
To: amd-gfx at lists.freedesktop.org
Cc: Wu, Hersen
Subject: [PATCH 09/16] drm/amd/display: fbc state could not reach while enable fbc

From: hersen wu <hersenxs.wu at amd.com>

   [WHY] fbc is within the data path from memory to dce. while
   re-configure mc dmif, fbc should be enabled. otherwise, fbc
   may not be enabled properly.

   [HOW] before re-configure mc dmif, disable fbc, only after
   dmif re-configuration fully done, enable fbc again.

Signed-off-by: hersen wu <hersenxs.wu at amd.com>
Reviewed-by: Roman Li <Roman.Li at amd.com>
Acked-by: Leo Li <sunpeng.li at amd.com>
---
 .../drm/amd/display/dc/dce110/dce110_compressor.c  | 91 ++++++++--------------
 .../amd/display/dc/dce110/dce110_hw_sequencer.c    | 57 ++++++++------
 drivers/gpu/drm/amd/display/dc/inc/compressor.h    |  1 +
 3 files changed, 66 insertions(+), 83 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
index 1f7f250..52d50e2 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
@@ -64,65 +64,37 @@ static const struct dce110_compressor_reg_offsets reg_offsets[] = {

 static const uint32_t dce11_one_lpt_channel_max_resolution = 2560 * 1600;

-enum fbc_idle_force {
-       /* Bit 0 - Display registers updated */
-       FBC_IDLE_FORCE_DISPLAY_REGISTER_UPDATE = 0x00000001,
-
-       /* Bit 2 - FBC_GRPH_COMP_EN register updated */
-       FBC_IDLE_FORCE_GRPH_COMP_EN = 0x00000002,
-       /* Bit 3 - FBC_SRC_SEL register updated */
-       FBC_IDLE_FORCE_SRC_SEL_CHANGE = 0x00000004,
-       /* Bit 4 - FBC_MIN_COMPRESSION register updated */
-       FBC_IDLE_FORCE_MIN_COMPRESSION_CHANGE = 0x00000008,
-       /* Bit 5 - FBC_ALPHA_COMP_EN register updated */
-       FBC_IDLE_FORCE_ALPHA_COMP_EN = 0x00000010,
-       /* Bit 6 - FBC_ZERO_ALPHA_CHUNK_SKIP_EN register updated */
-       FBC_IDLE_FORCE_ZERO_ALPHA_CHUNK_SKIP_EN = 0x00000020,
-       /* Bit 7 - FBC_FORCE_COPY_TO_COMP_BUF register updated */
-       FBC_IDLE_FORCE_FORCE_COPY_TO_COMP_BUF = 0x00000040,
-
-       /* Bit 24 - Memory write to region 0 defined by MC registers. */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION0 = 0x01000000,
-       /* Bit 25 - Memory write to region 1 defined by MC registers */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION1 = 0x02000000,
-       /* Bit 26 - Memory write to region 2 defined by MC registers */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION2 = 0x04000000,
-       /* Bit 27 - Memory write to region 3 defined by MC registers. */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION3 = 0x08000000,
-
-       /* Bit 28 - Memory write from any client other than MCIF */
-       FBC_IDLE_FORCE_MEMORY_WRITE_OTHER_THAN_MCIF = 0x10000000,
-       /* Bit 29 - CG statics screen signal is inactive */
-       FBC_IDLE_FORCE_CG_STATIC_SCREEN_IS_INACTIVE = 0x20000000,
-};
-
-
 static uint32_t align_to_chunks_number_per_line(uint32_t pixels)
 {
         return 256 * ((pixels + 255) / 256);
 }

-static void reset_lb_on_vblank(struct dc_context *ctx)
+static void reset_lb_on_vblank(struct compressor *compressor, uint32_t crtc_inst)
 {
-       uint32_t value, frame_count;
+       uint32_t value;
+       uint32_t frame_count;
+       uint32_t status_pos;
         uint32_t retry = 0;
-       uint32_t status_pos =
-                       dm_read_reg(ctx, mmCRTC_STATUS_POSITION);
+       struct dce110_compressor *cp110 = TO_DCE110_COMPRESSOR(compressor);
+
+       cp110->offsets = reg_offsets[crtc_inst];
+
+       status_pos = dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_POSITION));


         /* Only if CRTC is enabled and counter is moving we wait for one frame. */
-       if (status_pos != dm_read_reg(ctx, mmCRTC_STATUS_POSITION)) {
+       if (status_pos != dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_POSITION))) {
                 /* Resetting LB on VBlank */
-               value = dm_read_reg(ctx, mmLB_SYNC_RESET_SEL);
+               value = dm_read_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL));
                 set_reg_field_value(value, 3, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL);
                 set_reg_field_value(value, 1, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL2);
-               dm_write_reg(ctx, mmLB_SYNC_RESET_SEL, value);
+               dm_write_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL), value);

-               frame_count = dm_read_reg(ctx, mmCRTC_STATUS_FRAME_COUNT);
+               frame_count = dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_FRAME_COUNT));


                 for (retry = 10000; retry > 0; retry--) {
-                       if (frame_count != dm_read_reg(ctx, mmCRTC_STATUS_FRAME_COUNT))
+                       if (frame_count != dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_FRAME_COUNT)))
                                 break;
                         udelay(10);
                 }
@@ -130,13 +102,11 @@ static void reset_lb_on_vblank(struct dc_context *ctx)
                         dm_error("Frame count did not increase for 100ms.\n");

                 /* Resetting LB on VBlank */
-               value = dm_read_reg(ctx, mmLB_SYNC_RESET_SEL);
+               value = dm_read_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL));
                 set_reg_field_value(value, 2, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL);
                 set_reg_field_value(value, 0, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL2);
-               dm_write_reg(ctx, mmLB_SYNC_RESET_SEL, value);
-
+               dm_write_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL), value);
         }
-
 }

 static void wait_for_fbc_state_changed(
@@ -226,10 +196,10 @@ void dce110_compressor_enable_fbc(
                 uint32_t addr;
                 uint32_t value, misc_value;

-
                 addr = mmFBC_CNTL;
                 value = dm_read_reg(compressor->ctx, addr);
                 set_reg_field_value(value, 1, FBC_CNTL, FBC_GRPH_COMP_EN);
+               /* params->inst is valid HW CRTC instance start from 0 */
                 set_reg_field_value(
                         value,
                         params->inst,
@@ -238,8 +208,10 @@ void dce110_compressor_enable_fbc(

                 /* Keep track of enum controller_id FBC is attached to */
                 compressor->is_enabled = true;
-               compressor->attached_inst = params->inst;
-               cp110->offsets = reg_offsets[params->inst];
+               /* attached_inst is SW CRTC instance start from 1
+                * 0 = CONTROLLER_ID_UNDEFINED means not attached crtc
+                */
+               compressor->attached_inst = params->inst + CONTROLLER_ID_D0;

                 /* Toggle it as there is bug in HW */
                 set_reg_field_value(value, 0, FBC_CNTL, FBC_GRPH_COMP_EN);
@@ -268,9 +240,10 @@ void dce110_compressor_enable_fbc(
 void dce110_compressor_disable_fbc(struct compressor *compressor)
 {
         struct dce110_compressor *cp110 = TO_DCE110_COMPRESSOR(compressor);
+       uint32_t crtc_inst = 0;

         if (compressor->options.bits.FBC_SUPPORT) {
-               if (dce110_compressor_is_fbc_enabled_in_hw(compressor, NULL)) {
+               if (dce110_compressor_is_fbc_enabled_in_hw(compressor, &crtc_inst)) {
                         uint32_t reg_data;
                         /* Turn off compression */
                         reg_data = dm_read_reg(compressor->ctx, mmFBC_CNTL);
@@ -284,8 +257,10 @@ void dce110_compressor_disable_fbc(struct compressor *compressor)
                         wait_for_fbc_state_changed(cp110, false);
                 }

-               /* Sync line buffer  - dce100/110 only*/
-               reset_lb_on_vblank(compressor->ctx);
+               /* Sync line buffer which fbc was attached to dce100/110 only */
+               if (crtc_inst > CONTROLLER_ID_UNDEFINED && crtc_inst < CONTROLLER_ID_D3)
+                       reset_lb_on_vblank(compressor,
+                                       crtc_inst - CONTROLLER_ID_D0);
         }
 }

@@ -328,6 +303,8 @@ void dce110_compressor_program_compressed_surface_address_and_pitch(
         uint32_t compressed_surf_address_low_part =
                 compressor->compr_surface_address.addr.low_part;

+       cp110->offsets = reg_offsets[params->inst];
+
         /* Clear content first. */
         dm_write_reg(
                 compressor->ctx,
@@ -410,13 +387,7 @@ void dce110_compressor_set_fbc_invalidation_triggers(
         value = dm_read_reg(compressor->ctx, addr);
         set_reg_field_value(
                 value,
-               fbc_trigger |
-               FBC_IDLE_FORCE_GRPH_COMP_EN |
-               FBC_IDLE_FORCE_SRC_SEL_CHANGE |
-               FBC_IDLE_FORCE_MIN_COMPRESSION_CHANGE |
-               FBC_IDLE_FORCE_ALPHA_COMP_EN |
-               FBC_IDLE_FORCE_ZERO_ALPHA_CHUNK_SKIP_EN |
-               FBC_IDLE_FORCE_FORCE_COPY_TO_COMP_BUF,
+               fbc_trigger,
                 FBC_IDLE_FORCE_CLEAR_MASK,
                 FBC_IDLE_FORCE_CLEAR_MASK);
         dm_write_reg(compressor->ctx, addr, value);
@@ -549,7 +520,7 @@ void dce110_compressor_construct(struct dce110_compressor *compressor,
         compressor->base.channel_interleave_size = 0;
         compressor->base.dram_channels_num = 0;
         compressor->base.lpt_channels_num = 0;
-       compressor->base.attached_inst = 0;
+       compressor->base.attached_inst = CONTROLLER_ID_UNDEFINED;
         compressor->base.is_enabled = false;
         compressor->base.funcs = &dce110_compressor_funcs;

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index 2f062ba..6349ba7 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -1766,12 +1766,13 @@ static void set_static_screen_control(struct pipe_ctx **pipe_ctx,
  *  Check if FBC can be enabled
  */
 static bool should_enable_fbc(struct dc *dc,
-                             struct dc_state *context,
-                             uint32_t *pipe_idx)
+               struct dc_state *context,
+               uint32_t *pipe_idx)
 {
         uint32_t i;
         struct pipe_ctx *pipe_ctx = NULL;
         struct resource_context *res_ctx = &context->res_ctx;
+       unsigned int underlay_idx = dc->res_pool->underlay_pipe_index;


         ASSERT(dc->fbc_compressor);
@@ -1786,14 +1787,28 @@ static bool should_enable_fbc(struct dc *dc,

         for (i = 0; i < dc->res_pool->pipe_count; i++) {
                 if (res_ctx->pipe_ctx[i].stream) {
+
                         pipe_ctx = &res_ctx->pipe_ctx[i];
-                       *pipe_idx = i;
-                       break;
+
+                       if (!pipe_ctx)
+                               continue;
+
+                       /* fbc not applicable on underlay pipe */
+                       if (pipe_ctx->pipe_idx != underlay_idx) {
+                               *pipe_idx = i;
+                               break;
+                       }
                 }
         }

-       /* Pipe context should be found */
-       ASSERT(pipe_ctx);
+       if (i == dc->res_pool->pipe_count)
+               return false;
+
+       if (!pipe_ctx->stream->sink)
+               return false;
+
+       if (!pipe_ctx->stream->sink->link)
+               return false;

         /* Only supports eDP */
         if (pipe_ctx->stream->sink->link->connector_signal != SIGNAL_TYPE_EDP)
@@ -1817,8 +1832,9 @@ static bool should_enable_fbc(struct dc *dc,
 /*
  *  Enable FBC
  */
-static void enable_fbc(struct dc *dc,
-                      struct dc_state *context)
+static void enable_fbc(
+               struct dc *dc,
+               struct dc_state *context)
 {
         uint32_t pipe_idx = 0;

@@ -1828,10 +1844,9 @@ static void enable_fbc(struct dc *dc,
                 struct compressor *compr = dc->fbc_compressor;
                 struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[pipe_idx];

-
                 params.source_view_width = pipe_ctx->stream->timing.h_addressable;
                 params.source_view_height = pipe_ctx->stream->timing.v_addressable;
-
+               params.inst = pipe_ctx->stream_res.tg->inst;
                 compr->compr_surface_address.quad_part = dc->ctx->fbc_gpu_addr;

                 compr->funcs->surface_address_and_pitch(compr, &params);
@@ -2046,10 +2061,10 @@ enum dc_status dce110_apply_ctx_to_hw(
                         return status;
         }

-       dcb->funcs->set_scratch_critical_state(dcb, false);
-
         if (dc->fbc_compressor)
-               enable_fbc(dc, context);
+               enable_fbc(dc, dc->current_state);
+
+       dcb->funcs->set_scratch_critical_state(dcb, false);

         return DC_OK;
 }
@@ -2408,7 +2423,6 @@ static void dce110_program_front_end_for_pipe(
         struct dc_plane_state *plane_state = pipe_ctx->plane_state;
         struct xfm_grph_csc_adjustment adjust;
         struct out_csc_color_matrix tbl_entry;
-       unsigned int underlay_idx = dc->res_pool->underlay_pipe_index;
         unsigned int i;
         DC_LOGGER_INIT();
         memset(&tbl_entry, 0, sizeof(tbl_entry));
@@ -2449,15 +2463,6 @@ static void dce110_program_front_end_for_pipe(

         program_scaler(dc, pipe_ctx);

-       /* fbc not applicable on Underlay pipe */
-       if (dc->fbc_compressor && old_pipe->stream &&
-           pipe_ctx->pipe_idx != underlay_idx) {
-               if (plane_state->tiling_info.gfx8.array_mode == DC_ARRAY_LINEAR_GENERAL)
-                       dc->fbc_compressor->funcs->disable_fbc(dc->fbc_compressor);
-               else
-                       enable_fbc(dc, dc->current_state);
-       }
-
         mi->funcs->mem_input_program_surface_config(
                         mi,
                         plane_state->format,
@@ -2534,6 +2539,9 @@ static void dce110_apply_ctx_for_surface(
         if (num_planes == 0)
                 return;

+       if (dc->fbc_compressor)
+               dc->fbc_compressor->funcs->disable_fbc(dc->fbc_compressor);
+
         for (i = 0; i < dc->res_pool->pipe_count; i++) {
                 struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[i];
                 struct pipe_ctx *old_pipe_ctx = &dc->current_state->res_ctx.pipe_ctx[i];
@@ -2576,6 +2584,9 @@ static void dce110_apply_ctx_for_surface(
                         (pipe_ctx->plane_state || old_pipe_ctx->plane_state))
                         dc->hwss.pipe_control_lock(dc, pipe_ctx, false);
         }
+
+       if (dc->fbc_compressor)
+               enable_fbc(dc, dc->current_state);
 }

 static void dce110_power_down_fe(struct dc *dc, struct pipe_ctx *pipe_ctx)
diff --git a/drivers/gpu/drm/amd/display/dc/inc/compressor.h b/drivers/gpu/drm/amd/display/dc/inc/compressor.h
index bcb18f5..7a147a9 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/compressor.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/compressor.h
@@ -77,6 +77,7 @@ struct compressor_funcs {
 };
 struct compressor {
         struct dc_context *ctx;
+       /* CONTROLLER_ID_D0 + instance, CONTROLLER_ID_UNDEFINED = 0 */
         uint32_t attached_inst;
         bool is_enabled;
         const struct compressor_funcs *funcs;
--
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20181129/50e75b9d/attachment-0001.html>


More information about the amd-gfx mailing list