From: Wenjing Liu <wenjing....@amd.com>

[why]
Current we have hundreds of if/else or switch statement to check
encoder type in dc_link level. The reason is because depending
on the type of encoder dc_link needs to perform similar programming
task but with different encoder interfaces. The story is to abstract
these interfaces so dc_link can just perform the programming task
without knowing the detail of which encoder it's dealing with.

Reviewed-by: Aric Cyr <aric....@amd.com>
Acked-by: Rodrigo Siqueira <rodrigo.sique...@amd.com>
Signed-off-by: Wenjing Liu <wenjing....@amd.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link.c | 83 ++++++++----------
 .../drm/amd/display/dc/core/dc_link_hwss.c    | 85 +++++++++++++++++++
 .../amd/display/dc/inc/hw/stream_encoder.h    |  4 +
 .../gpu/drm/amd/display/dc/inc/link_hwss.h    | 19 +++++
 4 files changed, 143 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 33a22c90cf4a..6e10d3e7fefa 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -3432,6 +3432,8 @@ static enum dc_status dc_link_update_sst_payload(struct 
pipe_ctx *pipe_ctx,
        struct hpo_dp_stream_encoder *hpo_dp_stream_encoder = 
pipe_ctx->stream_res.hpo_dp_stream_enc;
        struct link_mst_stream_allocation_table proposed_table = {0};
        struct fixed31_32 avg_time_slots_per_mtp;
+       const struct dc_link_settings empty_link_settings = {0};
+       const struct dc_link_hwss *link_hwss = dc_link_hwss_get(link, 
&pipe_ctx->link_res);
        DC_LOGGER_INIT(link->ctx->logger);
 
        /* slot X.Y for SST payload deallocate */
@@ -3440,10 +3442,11 @@ static enum dc_status dc_link_update_sst_payload(struct 
pipe_ctx *pipe_ctx,
 
                dc_log_vcp_x_y(link, avg_time_slots_per_mtp);
 
-               hpo_dp_link_encoder->funcs->set_throttled_vcp_size(
-                               hpo_dp_link_encoder,
-                               hpo_dp_stream_encoder->inst,
-                               avg_time_slots_per_mtp);
+               link_hwss->set_throttled_vcp_size(pipe_ctx, 
avg_time_slots_per_mtp);
+               if (link_hwss->set_hblank_min_symbol_width)
+                       link_hwss->set_hblank_min_symbol_width(pipe_ctx,
+                                       &empty_link_settings,
+                                       avg_time_slots_per_mtp);
        }
 
        /* calculate VC payload and update branch with new payload allocation 
table*/
@@ -3487,10 +3490,11 @@ static enum dc_status dc_link_update_sst_payload(struct 
pipe_ctx *pipe_ctx,
 
                dc_log_vcp_x_y(link, avg_time_slots_per_mtp);
 
-               hpo_dp_link_encoder->funcs->set_throttled_vcp_size(
-                               hpo_dp_link_encoder,
-                               hpo_dp_stream_encoder->inst,
-                               avg_time_slots_per_mtp);
+               link_hwss->set_throttled_vcp_size(pipe_ctx, 
avg_time_slots_per_mtp);
+               if (link_hwss->set_hblank_min_symbol_width)
+                       link_hwss->set_hblank_min_symbol_width(pipe_ctx,
+                                       &link->cur_link_settings,
+                                       avg_time_slots_per_mtp);
        }
 
        /* Always return DC_OK.
@@ -3507,15 +3511,14 @@ enum dc_status dc_link_allocate_mst_payload(struct 
pipe_ctx *pipe_ctx)
        struct dc_stream_state *stream = pipe_ctx->stream;
        struct dc_link *link = stream->link;
        struct link_encoder *link_encoder = NULL;
-       struct stream_encoder *stream_encoder = pipe_ctx->stream_res.stream_enc;
        struct hpo_dp_link_encoder *hpo_dp_link_encoder = 
pipe_ctx->link_res.hpo_dp_link_enc;
-       struct hpo_dp_stream_encoder *hpo_dp_stream_encoder = 
pipe_ctx->stream_res.hpo_dp_stream_enc;
        struct dp_mst_stream_allocation_table proposed_table = {0};
        struct fixed31_32 avg_time_slots_per_mtp;
        struct fixed31_32 pbn;
        struct fixed31_32 pbn_per_slot;
        int i;
        enum act_return_status ret;
+       const struct dc_link_hwss *link_hwss = dc_link_hwss_get(link, 
&pipe_ctx->link_res);
        DC_LOGGER_INIT(link->ctx->logger);
 
        /* Link encoder may have been dynamically assigned to non-physical 
display endpoint. */
@@ -3621,22 +3624,13 @@ enum dc_status dc_link_allocate_mst_payload(struct 
pipe_ctx *pipe_ctx)
        pbn = get_pbn_from_timing(pipe_ctx);
        avg_time_slots_per_mtp = dc_fixpt_div(pbn, pbn_per_slot);
 
-       switch (dp_get_link_encoding_format(&link->cur_link_settings)) {
-       case DP_8b_10b_ENCODING:
-               stream_encoder->funcs->set_throttled_vcp_size(
-                       stream_encoder,
-                       avg_time_slots_per_mtp);
-               break;
-       case DP_128b_132b_ENCODING:
-               hpo_dp_link_encoder->funcs->set_throttled_vcp_size(
-                               hpo_dp_link_encoder,
-                               hpo_dp_stream_encoder->inst,
+       dc_log_vcp_x_y(link, avg_time_slots_per_mtp);
+
+       link_hwss->set_throttled_vcp_size(pipe_ctx, avg_time_slots_per_mtp);
+       if (link_hwss->set_hblank_min_symbol_width)
+               link_hwss->set_hblank_min_symbol_width(pipe_ctx,
+                               &link->cur_link_settings,
                                avg_time_slots_per_mtp);
-               break;
-       case DP_UNKNOWN_ENCODING:
-               DC_LOG_ERROR("Failure: unknown encoding format\n");
-               return DC_ERROR_UNEXPECTED;
-       }
 
        return DC_OK;
 
@@ -3650,10 +3644,10 @@ enum dc_status dc_link_reduce_mst_payload(struct 
pipe_ctx *pipe_ctx, uint32_t bw
        struct fixed31_32 pbn;
        struct fixed31_32 pbn_per_slot;
        struct link_encoder *link_encoder = link->link_enc;
-       struct stream_encoder *stream_encoder = pipe_ctx->stream_res.stream_enc;
        struct dp_mst_stream_allocation_table proposed_table = {0};
        uint8_t i;
        enum act_return_status ret;
+       const struct dc_link_hwss *link_hwss = dc_link_hwss_get(link, 
&pipe_ctx->link_res);
        DC_LOGGER_INIT(link->ctx->logger);
 
        /* decrease throttled vcp size */
@@ -3661,8 +3655,10 @@ enum dc_status dc_link_reduce_mst_payload(struct 
pipe_ctx *pipe_ctx, uint32_t bw
        pbn = get_pbn_from_bw_in_kbps(bw_in_kbps);
        avg_time_slots_per_mtp = dc_fixpt_div(pbn, pbn_per_slot);
 
-       stream_encoder->funcs->set_throttled_vcp_size(
-                               stream_encoder,
+       link_hwss->set_throttled_vcp_size(pipe_ctx, avg_time_slots_per_mtp);
+       if (link_hwss->set_hblank_min_symbol_width)
+               link_hwss->set_hblank_min_symbol_width(pipe_ctx,
+                               &link->cur_link_settings,
                                avg_time_slots_per_mtp);
 
        /* send ALLOCATE_PAYLOAD sideband message with updated pbn */
@@ -3730,10 +3726,10 @@ enum dc_status dc_link_increase_mst_payload(struct 
pipe_ctx *pipe_ctx, uint32_t
        struct fixed31_32 pbn;
        struct fixed31_32 pbn_per_slot;
        struct link_encoder *link_encoder = link->link_enc;
-       struct stream_encoder *stream_encoder = pipe_ctx->stream_res.stream_enc;
        struct dp_mst_stream_allocation_table proposed_table = {0};
        uint8_t i;
        enum act_return_status ret;
+       const struct dc_link_hwss *link_hwss = dc_link_hwss_get(link, 
&pipe_ctx->link_res);
        DC_LOGGER_INIT(link->ctx->logger);
 
        /* notify immediate branch device table update */
@@ -3792,8 +3788,10 @@ enum dc_status dc_link_increase_mst_payload(struct 
pipe_ctx *pipe_ctx, uint32_t
        pbn_per_slot = get_pbn_per_slot(stream);
        avg_time_slots_per_mtp = dc_fixpt_div(pbn, pbn_per_slot);
 
-       stream_encoder->funcs->set_throttled_vcp_size(
-                               stream_encoder,
+       link_hwss->set_throttled_vcp_size(pipe_ctx, avg_time_slots_per_mtp);
+       if (link_hwss->set_hblank_min_symbol_width)
+               link_hwss->set_hblank_min_symbol_width(pipe_ctx,
+                               &link->cur_link_settings,
                                avg_time_slots_per_mtp);
 
        return DC_OK;
@@ -3804,13 +3802,13 @@ static enum dc_status deallocate_mst_payload(struct 
pipe_ctx *pipe_ctx)
        struct dc_stream_state *stream = pipe_ctx->stream;
        struct dc_link *link = stream->link;
        struct link_encoder *link_encoder = NULL;
-       struct stream_encoder *stream_encoder = pipe_ctx->stream_res.stream_enc;
        struct hpo_dp_link_encoder *hpo_dp_link_encoder = 
pipe_ctx->link_res.hpo_dp_link_enc;
-       struct hpo_dp_stream_encoder *hpo_dp_stream_encoder = 
pipe_ctx->stream_res.hpo_dp_stream_enc;
        struct dp_mst_stream_allocation_table proposed_table = {0};
        struct fixed31_32 avg_time_slots_per_mtp = dc_fixpt_from_int(0);
        int i;
        bool mst_mode = (link->type == dc_connection_mst_branch);
+       const struct dc_link_hwss *link_hwss = dc_link_hwss_get(link, 
&pipe_ctx->link_res);
+       const struct dc_link_settings empty_link_settings = {0};
        DC_LOGGER_INIT(link->ctx->logger);
 
        /* Link encoder may have been dynamically assigned to non-physical 
display endpoint. */
@@ -3828,22 +3826,11 @@ static enum dc_status deallocate_mst_payload(struct 
pipe_ctx *pipe_ctx)
         */
 
        /* slot X.Y */
-       switch (dp_get_link_encoding_format(&link->cur_link_settings)) {
-       case DP_8b_10b_ENCODING:
-               stream_encoder->funcs->set_throttled_vcp_size(
-                       stream_encoder,
-                       avg_time_slots_per_mtp);
-               break;
-       case DP_128b_132b_ENCODING:
-               hpo_dp_link_encoder->funcs->set_throttled_vcp_size(
-                               hpo_dp_link_encoder,
-                               hpo_dp_stream_encoder->inst,
+       link_hwss->set_throttled_vcp_size(pipe_ctx, avg_time_slots_per_mtp);
+       if (link_hwss->set_hblank_min_symbol_width)
+               link_hwss->set_hblank_min_symbol_width(pipe_ctx,
+                               &empty_link_settings,
                                avg_time_slots_per_mtp);
-               break;
-       case DP_UNKNOWN_ENCODING:
-               DC_LOG_ERROR("Failure: unknown encoding format\n");
-               return DC_ERROR_UNEXPECTED;
-       }
 
        /* TODO: which component is responsible for remove payload table? */
        if (mst_mode) {
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
index c84822cd7e53..93392c67c909 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
@@ -844,4 +844,89 @@ void setup_dp_hpo_stream(struct pipe_ctx *pipe_ctx, bool 
enable)
        }
 }
 
+/******************************* dio_link_hwss 
********************************/
+static void set_dio_throttled_vcp_size(struct pipe_ctx *pipe_ctx,
+               struct fixed31_32 throttled_vcp_size)
+{
+       struct stream_encoder *stream_encoder = pipe_ctx->stream_res.stream_enc;
+
+       stream_encoder->funcs->set_throttled_vcp_size(
+                               stream_encoder,
+                               throttled_vcp_size);
+}
+
+/***************************** hpo_dp_link_hwss 
*******************************/
+static void set_dp_hpo_throttled_vcp_size(struct pipe_ctx *pipe_ctx,
+               struct fixed31_32 throttled_vcp_size)
+{
+       struct hpo_dp_stream_encoder *hpo_dp_stream_encoder = 
pipe_ctx->stream_res.hpo_dp_stream_enc;
+       struct hpo_dp_link_encoder *hpo_dp_link_encoder = 
pipe_ctx->link_res.hpo_dp_link_enc;
+
+       hpo_dp_link_encoder->funcs->set_throttled_vcp_size(hpo_dp_link_encoder,
+                       hpo_dp_stream_encoder->inst,
+                       throttled_vcp_size);
+}
+
+static void set_dp_hpo_hblank_min_symbol_width(struct pipe_ctx *pipe_ctx,
+               const struct dc_link_settings *link_settings,
+               struct fixed31_32 throttled_vcp_size)
+{
+       struct hpo_dp_stream_encoder *hpo_dp_stream_encoder = 
pipe_ctx->stream_res.hpo_dp_stream_enc;
+       struct dc_crtc_timing *timing = &pipe_ctx->stream->timing;
+       struct fixed31_32 h_blank_in_ms, time_slot_in_ms, mtp_cnt_per_h_blank;
+       uint32_t link_bw_in_kbps = 
dc_link_bandwidth_kbps(pipe_ctx->stream->link, link_settings);
+       uint16_t hblank_min_symbol_width = 0;
+
+       if (link_bw_in_kbps > 0) {
+               h_blank_in_ms = 
dc_fixpt_div(dc_fixpt_from_int(timing->h_total-timing->h_addressable),
+                               dc_fixpt_from_fraction(timing->pix_clk_100hz, 
10));
+               time_slot_in_ms = dc_fixpt_from_fraction(32 * 4, 
link_bw_in_kbps);
+               mtp_cnt_per_h_blank = dc_fixpt_div(h_blank_in_ms, 
dc_fixpt_mul_int(time_slot_in_ms, 64));
+               hblank_min_symbol_width = dc_fixpt_floor(
+                               dc_fixpt_mul(mtp_cnt_per_h_blank, 
throttled_vcp_size));
+       }
+
+       
hpo_dp_stream_encoder->funcs->set_hblank_min_symbol_width(hpo_dp_stream_encoder,
+                       hblank_min_symbol_width);
+}
+
+static const struct dc_link_hwss hpo_dp_link_hwss = {
+       .set_throttled_vcp_size = set_dp_hpo_throttled_vcp_size,
+
+       /* function pointers below this point require check for NULL
+        * *********************************************************************
+        */
+       .set_hblank_min_symbol_width = set_dp_hpo_hblank_min_symbol_width,
+};
+
+static const struct dc_link_hwss dio_link_hwss = {
+       .set_throttled_vcp_size = set_dio_throttled_vcp_size,
+};
+
+const struct dc_link_hwss *dc_link_hwss_get(const struct dc_link *link,
+               const struct link_resource *link_res)
+{
+       if (link_res->hpo_dp_link_enc)
+               /* TODO: some assumes that if decided link settings is 128b/132b
+                * channel coding format hpo_dp_link_enc should be used.
+                * Others believe that if hpo_dp_link_enc is available in link
+                * resource then hpo_dp_link_enc must be used. This bound 
between
+                * hpo_dp_link_enc != NULL and decided link settings is loosely 
coupled
+                * with a premise that both hpo_dp_link_enc pointer and decided 
link
+                * settings are determined based on single policy function like
+                * "decide_link_settings" from upper layer. This "convention"
+                * cannot be maintained and enforced at current level.
+                * Therefore a refactor is due so we can enforce a strong bound
+                * between those two parameters at this level.
+                *
+                * To put it simple, we want to make enforcement at low level 
so that
+                * we will not return link hwss if caller plans to do 8b/10b
+                * with an hpo encoder. Or we can return a very dummy one that 
doesn't
+                * do work for all functions
+                */
+               return &hpo_dp_link_hwss;
+       else
+               return &dio_link_hwss;
+}
+
 #undef DC_LOGGER
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h 
b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
index d9a3a204cc23..36ec56524afd 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
@@ -327,6 +327,10 @@ struct hpo_dp_stream_encoder_funcs {
        void (*read_state)(
                        struct hpo_dp_stream_encoder *enc,
                        struct hpo_dp_stream_encoder_state *state);
+
+       void (*set_hblank_min_symbol_width)(
+                       struct hpo_dp_stream_encoder *enc,
+                       uint16_t width);
 };
 
 #endif /* STREAM_ENCODER_H_ */
diff --git a/drivers/gpu/drm/amd/display/dc/inc/link_hwss.h 
b/drivers/gpu/drm/amd/display/dc/inc/link_hwss.h
index 69d63763a10e..bd3b2b807431 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/link_hwss.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/link_hwss.h
@@ -72,4 +72,23 @@ void dp_retrain_link_dp_test(struct dc_link *link,
                struct dc_link_settings *link_setting,
                bool skip_video_pattern);
 
+struct dc_link;
+struct link_resource;
+struct fixed31_32;
+struct pipe_ctx;
+
+struct dc_link_hwss {
+       void (*set_throttled_vcp_size)(struct pipe_ctx *pipe_ctx,
+                       struct fixed31_32 throttled_vcp_size);
+
+       /* function pointers below this point require check for NULL
+        * *********************************************************************
+        */
+       void (*set_hblank_min_symbol_width)(struct pipe_ctx *pipe_ctx,
+                       const struct dc_link_settings *link_settings,
+                       struct fixed31_32 throttled_vcp_size);
+};
+
+const struct dc_link_hwss *dc_link_hwss_get(const struct dc_link *link, const 
struct link_resource *link_res);
+
 #endif /* __DC_LINK_HWSS_H__ */
-- 
2.25.1

Reply via email to