[PATCH 14/17] drm/amd/display: Isolate DSC module from driver dependencies
Kazlauskas, Nicholas
Nicholas.Kazlauskas at amd.com
Thu Aug 29 12:44:57 UTC 2019
On 2019-08-29 1:38 a.m., Dave Airlie wrote:
> On Thu, 29 Aug 2019 at 07:04, Bhawanpreet Lakha
> <Bhawanpreet.Lakha at amd.com> wrote:
>>
>> From: Bayan Zabihiyan <bayan.zabihiyan at amd.com>
>>
>> [Why]
>> Edid Utility wishes to include DSC module from driver instead
>> of doing it's own logic which will need to be updated every time
>> someone modifies the driver logic.
>>
>> [How]
>> Modify some functions such that we dont need to pass the entire
>> DC structure as parameter.
>> -Remove DC inclusion from module.
>> -Filter out problematic types and inclusions
>
> Do we really want the ifdef stuff upstream, the EDID utility isn't
> shipped with the kernel is it.
>
> Dave.
It's not, and this isn't a kernel configurable option anyway.
So this really should be dropped or split out in a way that we don't
need to use this.
Nicholas Kazlauskas
>
>>
>> Signed-off-by: Bayan Zabihiyan <bayan.zabihiyan at amd.com>
>> Reviewed-by: Jun Lei <Jun.Lei at amd.com>
>> Acked-by: Bhawanpreet Lakha <Bhawanpreet.Lakha at amd.com>
>> ---
>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-
>> drivers/gpu/drm/amd/display/dc/dc_dsc.h | 14 +++-
>> drivers/gpu/drm/amd/display/dc/dc_hw_types.h | 57 ++++++++------
>> drivers/gpu/drm/amd/display/dc/dc_types.h | 9 +++
>> drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c | 75 ++++++++++++++++---
>> drivers/gpu/drm/amd/display/dc/inc/hw/dsc.h | 12 ++-
>> 6 files changed, 125 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 654679c4fded..82ea8cf8563e 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -3677,8 +3677,9 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
>> dc_link_get_link_cap(aconnector->dc_link));
>>
>> if (dsc_caps.is_dsc_supported)
>> - if (dc_dsc_compute_config(aconnector->dc_link->ctx->dc,
>> + if (dc_dsc_compute_config(aconnector->dc_link->ctx->dc->res_pool->dscs[0],
>> &dsc_caps,
>> + aconnector->dc_link->ctx->dc->debug.dsc_min_slice_height_override,
>> link_bandwidth_kbps,
>> &stream->timing,
>> &stream->timing.dsc_cfg))
>> diff --git a/drivers/gpu/drm/amd/display/dc/dc_dsc.h b/drivers/gpu/drm/amd/display/dc/dc_dsc.h
>> index 6e42209f0e20..0ed2962add5a 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dc_dsc.h
>> +++ b/drivers/gpu/drm/amd/display/dc/dc_dsc.h
>> @@ -30,6 +30,7 @@
>> #define DP_DSC_BRANCH_OVERALL_THROUGHPUT_0 0x0a0 /* DP 1.4a SCR */
>> #define DP_DSC_BRANCH_OVERALL_THROUGHPUT_1 0x0a1
>> #define DP_DSC_BRANCH_MAX_LINE_WIDTH 0x0a2
>> +#include "dc_types.h"
>>
>> struct dc_dsc_bw_range {
>> uint32_t min_kbps; /* Bandwidth if min_target_bpp_x16 is used */
>> @@ -39,13 +40,21 @@ struct dc_dsc_bw_range {
>> uint32_t stream_kbps; /* Uncompressed stream bandwidth */
>> };
>>
>> +struct display_stream_compressor {
>> + const struct dsc_funcs *funcs;
>> +#ifndef AMD_EDID_UTILITY
>> + struct dc_context *ctx;
>> + int inst;
>> +#endif
>> +};
>>
>> bool dc_dsc_parse_dsc_dpcd(const uint8_t *dpcd_dsc_basic_data,
>> const uint8_t *dpcd_dsc_ext_data,
>> struct dsc_dec_dpcd_caps *dsc_sink_caps);
>>
>> bool dc_dsc_compute_bandwidth_range(
>> - const struct dc *dc,
>> + const struct display_stream_compressor *dsc,
>> + const uint32_t dsc_min_slice_height_override,
>> const uint32_t min_kbps,
>> const uint32_t max_kbps,
>> const struct dsc_dec_dpcd_caps *dsc_sink_caps,
>> @@ -53,8 +62,9 @@ bool dc_dsc_compute_bandwidth_range(
>> struct dc_dsc_bw_range *range);
>>
>> bool dc_dsc_compute_config(
>> - const struct dc *dc,
>> + const struct display_stream_compressor *dsc,
>> const struct dsc_dec_dpcd_caps *dsc_sink_caps,
>> + const uint32_t dsc_min_slice_height_override,
>> uint32_t target_bandwidth_kbps,
>> const struct dc_crtc_timing *timing,
>> struct dc_dsc_config *dsc_cfg);
>> diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
>> index dafc19a7b699..2869b26d966a 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
>> +++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
>> @@ -26,6 +26,8 @@
>> #ifndef DC_HW_TYPES_H
>> #define DC_HW_TYPES_H
>>
>> +#ifndef AMD_EDID_UTILITY
>> +
>> #include "os_types.h"
>> #include "fixed31_32.h"
>> #include "signal_types.h"
>> @@ -587,6 +589,8 @@ struct scaling_taps {
>> bool integer_scaling;
>> };
>>
>> +#endif /* AMD_EDID_UTILITY */
>> +
>> enum dc_timing_standard {
>> DC_TIMING_STANDARD_UNDEFINED,
>> DC_TIMING_STANDARD_DMT,
>> @@ -708,30 +712,6 @@ enum dc_timing_3d_format {
>> TIMING_3D_FORMAT_MAX,
>> };
>>
>> -enum trigger_delay {
>> - TRIGGER_DELAY_NEXT_PIXEL = 0,
>> - TRIGGER_DELAY_NEXT_LINE,
>> -};
>> -
>> -enum crtc_event {
>> - CRTC_EVENT_VSYNC_RISING = 0,
>> - CRTC_EVENT_VSYNC_FALLING
>> -};
>> -
>> -struct crtc_trigger_info {
>> - bool enabled;
>> - struct dc_stream_state *event_source;
>> - enum crtc_event event;
>> - enum trigger_delay delay;
>> -};
>> -
>> -struct dc_crtc_timing_adjust {
>> - uint32_t v_total_min;
>> - uint32_t v_total_max;
>> - uint32_t v_total_mid;
>> - uint32_t v_total_mid_frame_num;
>> -};
>> -
>> #ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
>> struct dc_dsc_config {
>> uint32_t num_slices_h; /* Number of DSC slices - horizontal */
>> @@ -775,6 +755,33 @@ struct dc_crtc_timing {
>> #endif
>> };
>>
>> +#ifndef AMD_EDID_UTILITY
>> +
>> +enum trigger_delay {
>> + TRIGGER_DELAY_NEXT_PIXEL = 0,
>> + TRIGGER_DELAY_NEXT_LINE,
>> +};
>> +
>> +enum crtc_event {
>> + CRTC_EVENT_VSYNC_RISING = 0,
>> + CRTC_EVENT_VSYNC_FALLING
>> +};
>> +
>> +struct crtc_trigger_info {
>> + bool enabled;
>> + struct dc_stream_state *event_source;
>> + enum crtc_event event;
>> + enum trigger_delay delay;
>> +};
>> +
>> +struct dc_crtc_timing_adjust {
>> + uint32_t v_total_min;
>> + uint32_t v_total_max;
>> + uint32_t v_total_mid;
>> + uint32_t v_total_mid_frame_num;
>> +};
>> +
>> +
>> /* Passed on init */
>> enum vram_type {
>> VIDEO_MEMORY_TYPE_GDDR5 = 2,
>> @@ -845,5 +852,7 @@ struct tg_color {
>> uint16_t color_b_cb;
>> };
>>
>> +#endif /* AMD_EDID_UTILITY */
>> +
>> #endif /* DC_HW_TYPES_H */
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dc_types.h b/drivers/gpu/drm/amd/display/dc/dc_types.h
>> index 82abc4ff6c49..e6ae66791943 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dc_types.h
>> +++ b/drivers/gpu/drm/amd/display/dc/dc_types.h
>> @@ -25,6 +25,11 @@
>> #ifndef DC_TYPES_H_
>> #define DC_TYPES_H_
>>
>> +#ifndef AMD_EDID_UTILITY
>> +/* AND EdidUtility only needs a portion
>> + * of this file, including the rest only
>> + * causes additional issues.
>> + */
>> #include "os_types.h"
>> #include "fixed31_32.h"
>> #include "irq_types.h"
>> @@ -745,6 +750,9 @@ struct dc_clock_config {
>> uint32_t current_clock_khz;/*current clock in use*/
>> };
>>
>> +#endif /*AMD_EDID_UTILITY*/
>> +//AMD EDID UTILITY does not need any of the above structures
>> +
>> #ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
>> /* DSC DPCD capabilities */
>> union dsc_slice_caps1 {
>> @@ -816,4 +824,5 @@ struct dsc_dec_dpcd_caps {
>> uint32_t branch_max_line_width;
>> };
>> #endif
>> +
>> #endif /* DC_TYPES_H_ */
>> diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
>> index 929ebd4cfb8c..e60f760585e4 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
>> @@ -23,8 +23,7 @@
>> */
>>
>> #ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
>> -#include "dc.h"
>> -#include "core_types.h"
>> +#include "dc_hw_types.h"
>> #include "dsc.h"
>> #include <drm/drm_dp_helper.h>
>>
>> @@ -47,6 +46,59 @@ const struct dc_dsc_policy dsc_policy = {
>>
>> /* This module's internal functions */
>>
>> +static uint32_t dc_dsc_bandwidth_in_kbps_from_timing(
>> + const struct dc_crtc_timing *timing)
>> +{
>> + uint32_t bits_per_channel = 0;
>> + uint32_t kbps;
>> +
>> + if (timing->flags.DSC) {
>> + kbps = (timing->pix_clk_100hz * timing->dsc_cfg.bits_per_pixel);
>> + kbps = kbps / 160 + ((kbps % 160) ? 1 : 0);
>> + return kbps;
>> + }
>> +
>> + switch (timing->display_color_depth) {
>> + case COLOR_DEPTH_666:
>> + bits_per_channel = 6;
>> + break;
>> + case COLOR_DEPTH_888:
>> + bits_per_channel = 8;
>> + break;
>> + case COLOR_DEPTH_101010:
>> + bits_per_channel = 10;
>> + break;
>> + case COLOR_DEPTH_121212:
>> + bits_per_channel = 12;
>> + break;
>> + case COLOR_DEPTH_141414:
>> + bits_per_channel = 14;
>> + break;
>> + case COLOR_DEPTH_161616:
>> + bits_per_channel = 16;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + ASSERT(bits_per_channel != 0);
>> +
>> + kbps = timing->pix_clk_100hz / 10;
>> + kbps *= bits_per_channel;
>> +
>> + if (timing->flags.Y_ONLY != 1) {
>> + /*Only YOnly make reduce bandwidth by 1/3 compares to RGB*/
>> + kbps *= 3;
>> + if (timing->pixel_encoding == PIXEL_ENCODING_YCBCR420)
>> + kbps /= 2;
>> + else if (timing->pixel_encoding == PIXEL_ENCODING_YCBCR422)
>> + kbps = kbps * 2 / 3;
>> + }
>> +
>> + return kbps;
>> +
>> +}
>> +
>> static bool dsc_buff_block_size_from_dpcd(int dpcd_buff_block_size, int *buff_block_size)
>> {
>>
>> @@ -178,12 +230,11 @@ static bool dsc_bpp_increment_div_from_dpcd(int bpp_increment_dpcd, uint32_t *bp
>> }
>>
>> static void get_dsc_enc_caps(
>> - const struct dc *dc,
>> + const struct display_stream_compressor *dsc,
>> struct dsc_enc_caps *dsc_enc_caps,
>> int pixel_clock_100Hz)
>> {
>> // This is a static HW query, so we can use any DSC
>> - struct display_stream_compressor *dsc = dc->res_pool->dscs[0];
>>
>> memset(dsc_enc_caps, 0, sizeof(struct dsc_enc_caps));
>> if (dsc)
>> @@ -290,7 +341,7 @@ static void get_dsc_bandwidth_range(
>> struct dc_dsc_bw_range *range)
>> {
>> /* native stream bandwidth */
>> - range->stream_kbps = dc_bandwidth_in_kbps_from_timing(timing);
>> + range->stream_kbps = dc_dsc_bandwidth_in_kbps_from_timing(timing);
>>
>> /* max dsc target bpp */
>> range->max_kbps = dsc_div_by_10_round_up(max_bpp * timing->pix_clk_100hz);
>> @@ -806,7 +857,8 @@ bool dc_dsc_parse_dsc_dpcd(const uint8_t *dpcd_dsc_basic_data, const uint8_t *dp
>> * If DSC is not possible, leave '*range' untouched.
>> */
>> bool dc_dsc_compute_bandwidth_range(
>> - const struct dc *dc,
>> + const struct display_stream_compressor *dsc,
>> + const uint32_t dsc_min_slice_height_override,
>> const uint32_t min_bpp,
>> const uint32_t max_bpp,
>> const struct dsc_dec_dpcd_caps *dsc_sink_caps,
>> @@ -818,14 +870,14 @@ bool dc_dsc_compute_bandwidth_range(
>> struct dsc_enc_caps dsc_common_caps;
>> struct dc_dsc_config config;
>>
>> - get_dsc_enc_caps(dc, &dsc_enc_caps, timing->pix_clk_100hz);
>> + get_dsc_enc_caps(dsc, &dsc_enc_caps, timing->pix_clk_100hz);
>>
>> is_dsc_possible = intersect_dsc_caps(dsc_sink_caps, &dsc_enc_caps,
>> timing->pixel_encoding, &dsc_common_caps);
>>
>> if (is_dsc_possible)
>> is_dsc_possible = setup_dsc_config(dsc_sink_caps, &dsc_enc_caps, 0, timing,
>> - dc->debug.dsc_min_slice_height_override, &config);
>> + dsc_min_slice_height_override, &config);
>>
>> if (is_dsc_possible)
>> get_dsc_bandwidth_range(min_bpp, max_bpp, &dsc_common_caps, timing, range);
>> @@ -834,8 +886,9 @@ bool dc_dsc_compute_bandwidth_range(
>> }
>>
>> bool dc_dsc_compute_config(
>> - const struct dc *dc,
>> + const struct display_stream_compressor *dsc,
>> const struct dsc_dec_dpcd_caps *dsc_sink_caps,
>> + const uint32_t dsc_min_slice_height_override,
>> uint32_t target_bandwidth_kbps,
>> const struct dc_crtc_timing *timing,
>> struct dc_dsc_config *dsc_cfg)
>> @@ -843,11 +896,11 @@ bool dc_dsc_compute_config(
>> bool is_dsc_possible = false;
>> struct dsc_enc_caps dsc_enc_caps;
>>
>> - get_dsc_enc_caps(dc, &dsc_enc_caps, timing->pix_clk_100hz);
>> + get_dsc_enc_caps(dsc, &dsc_enc_caps, timing->pix_clk_100hz);
>> is_dsc_possible = setup_dsc_config(dsc_sink_caps,
>> &dsc_enc_caps,
>> target_bandwidth_kbps,
>> - timing, dc->debug.dsc_min_slice_height_override, dsc_cfg);
>> + timing, dsc_min_slice_height_override, dsc_cfg);
>> return is_dsc_possible;
>> }
>> #endif /* CONFIG_DRM_AMD_DC_DSC_SUPPORT */
>> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/dsc.h b/drivers/gpu/drm/amd/display/dc/inc/hw/dsc.h
>> index 1ddb1c6fa149..c6ff3d78b435 100644
>> --- a/drivers/gpu/drm/amd/display/dc/inc/hw/dsc.h
>> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/dsc.h
>> @@ -28,7 +28,11 @@
>>
>> #include "dc_dsc.h"
>> #include "dc_hw_types.h"
>> -#include "dc_dp_types.h"
>> +#include "dc_types.h"
>> +/* do not include any other headers
>> + * or else it might break Edid Utility functionality.
>> + */
>> +
>>
>> /* Input parameters for configuring DSC from the outside of DSC */
>> struct dsc_config {
>> @@ -81,12 +85,6 @@ struct dsc_enc_caps {
>> uint32_t bpp_increment_div; /* bpp increment divisor, e.g. if 16, it's 1/16th of a bit */
>> };
>>
>> -struct display_stream_compressor {
>> - const struct dsc_funcs *funcs;
>> - struct dc_context *ctx;
>> - int inst;
>> -};
>> -
>> struct dsc_funcs {
>> void (*dsc_get_enc_caps)(struct dsc_enc_caps *dsc_enc_caps, int pixel_clock_100Hz);
>> void (*dsc_read_state)(struct display_stream_compressor *dsc, struct dcn_dsc_state *s);
>> --
>> 2.17.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
More information about the amd-gfx
mailing list