[PATCH 14/17] drm/amd/display: Isolate DSC module from driver dependencies
Dave Airlie
airlied at gmail.com
Thu Aug 29 05:38:31 UTC 2019
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.
>
> 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
More information about the amd-gfx
mailing list