| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e4508d96-a6ff-7c26-9b3c-9f37ef5d541a@linaro.org>
Date: Tue, 24 Jan 2023 16:15:34 +0100
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Kuogee Hsieh <quic_khsieh@...cinc.com>,
dri-devel@...ts.freedesktop.org, robdclark@...il.com,
sean@...rly.run, swboyd@...omium.org, dianders@...omium.org,
vkoul@...nel.org, daniel@...ll.ch, airlied@...il.com,
agross@...nel.org, dmitry.baryshkov@...aro.org,
andersson@...nel.org
Cc: quic_abhinavk@...cinc.com, quic_sbillaka@...cinc.com,
freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 06/14] drm/msm/dp: add display compression related
struct
On 23/01/2023 19:24, Kuogee Hsieh wrote:
> Add display compression related struct to support variant compression
> mechanism. However, DSC is the only one supported at this moment.
> VDC may be added later.
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@...cinc.com>
> ---
> drivers/gpu/drm/msm/dp/dp_panel.h | 42 ++++++++++++++++++
> drivers/gpu/drm/msm/msm_drv.h | 89 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 131 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
> index 1153e88..4c45d51 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> @@ -21,12 +21,54 @@ struct edid;
> #define DP_DOWNSTREAM_PORTS 4
> #define DP_DOWNSTREAM_CAP_SIZE 4
>
> +
> +#define DP_PANEL_CAPS_DSC BIT(0)
> +
> +enum dp_output_format {
> + DP_OUTPUT_FORMAT_RGB,
> + DP_OUTPUT_FORMAT_YCBCR420,
> + DP_OUTPUT_FORMAT_YCBCR422,
> + DP_OUTPUT_FORMAT_YCBCR444,
> + DP_OUTPUT_FORMAT_INVALID,
> +};
> +
> +
> +struct dp_panel_info {
> + u32 h_active;
> + u32 v_active;
> + u32 h_back_porch;
> + u32 h_front_porch;
> + u32 h_sync_width;
> + u32 h_active_low;
> + u32 v_back_porch;
> + u32 v_front_porch;
> + u32 v_sync_width;
> + u32 v_active_low;
> + u32 h_skew;
> + u32 refresh_rate;
> + u32 pixel_clk_khz;
> + u32 bpp;
> + bool widebus_en;
> + struct msm_compression_info comp_info;
> + s64 dsc_overhead_fp;
> +};
> +
> struct dp_display_mode {
> struct drm_display_mode drm_mode;
> + struct dp_panel_info timing;
> u32 capabilities;
> + s64 fec_overhead_fp;
> + s64 dsc_overhead_fp;
> u32 bpp;
> u32 h_active_low;
> u32 v_active_low;
> + /**
> + * @output_format:
> + *
> + * This is used to indicate DP output format.
> + * The output format can be read from drm_mode.
> + */
> + enum dp_output_format output_format;
> };
>
> struct dp_panel_in {
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 9f0c184..f155803 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -1,6 +1,7 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> /*
> * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2023. Qualcomm Innovation Center, Inc. All rights reserved
> * Copyright (C) 2013 Red Hat
> * Author: Rob Clark <robdclark@...il.com>
> */
> @@ -70,6 +71,16 @@ enum msm_dp_controller {
> #define MAX_H_TILES_PER_DISPLAY 2
>
> /**
> + * enum msm_display_compression_type - compression method used for pixel stream
> + * @MSM_DISPLAY_COMPRESSION_NONE: Pixel data is not compressed
> + * @MSM_DISPLAY_COMPRESSION_DSC: DSC compresison is used
> + */
> +enum msm_display_compression_type {
> + MSM_DISPLAY_COMPRESSION_NONE,
> + MSM_DISPLAY_COMPRESSION_DSC,
> +};
> +
> +/**
> * enum msm_event_wait - type of HW events to wait for
> * @MSM_ENC_COMMIT_DONE - wait for the driver to flush the registers to HW
> * @MSM_ENC_TX_COMPLETE - wait for the HW to transfer the frame to panel
> @@ -82,6 +93,84 @@ enum msm_event_wait {
> };
>
> /**
> + * struct msm_display_dsc_info - defines dsc configuration
> + * @config DSC encoder configuration
> + * @scr_rev: DSC revision.
> + * @initial_lines: Number of initial lines stored in encoder.
> + * @pkt_per_line: Number of packets per line.
> + * @bytes_in_slice: Number of bytes in slice.
> + * @eol_byte_num: Valid bytes at the end of line.
> + * @bytes_per_pkt Number of bytes in DSI packet
> + * @pclk_per_line: Compressed width.
> + * @slice_last_group_size: Size of last group in pixels.
> + * @slice_per_pkt: Number of slices per packet.
> + * @num_active_ss_per_enc: Number of active soft slices per encoder.
> + * @source_color_space: Source color space of DSC encoder
> + * @chroma_format: Chroma_format of DSC encoder.
> + * @det_thresh_flatness: Flatness threshold.
> + * @extra_width: Extra width required in timing calculations.
> + * @pps_delay_ms: Post PPS command delay in milliseconds.
> + * @dsc_4hsmerge_en: Using DSC 4HS merge topology
> + * @dsc_4hsmerge_padding 4HS merge DSC pair padding value in bytes
> + * @dsc_4hsmerge_alignment 4HS merge DSC alignment value in bytes
> + * @half_panel_pu True for single and dual dsc encoders if partial
> + * update sets the roi width to half of mode width
> + * False in all other cases
> + */
> +struct msm_display_dsc_info {
> + struct drm_dsc_config drm_dsc;
> + u8 scr_rev;
> +
> + int initial_lines;
> + int pkt_per_line;
> + int bytes_in_slice;
> + int bytes_per_pkt;
> + int eol_byte_num;
> + int pclk_per_line;
> + int slice_last_group_size;
> + int slice_per_pkt;
> + int num_active_ss_per_enc;
> + int source_color_space;
> + int chroma_format;
> + int det_thresh_flatness;
> + u32 extra_width;
> + u32 pps_delay_ms;
> + bool dsc_4hsmerge_en;
> + u32 dsc_4hsmerge_padding;
> + u32 dsc_4hsmerge_alignment;
> + bool half_panel_pu;
> +};
> +
> +/*
> + * conver from struct drm_dsc_config to struct msm_display_dsc_info
> + */
> +#define to_msm_dsc_info(dsc) container_of((dsc), struct msm_display_dsc_info, drm_dsc)
This is weird, you consider dsc struct is in control of dpu, but no with DSC it's part
of the DSI panel struct.
msm_display_dsc_info->drm_dsc should be a pointer.
If some values caculated in msm_display_dsc_info are *really* needed, then a new structure
should be added, but as a companion of the dsm dsc pointer, not containing it.
> +
> +/**
> + * Bits/pixel target >> 4 (removing the fractional bits)
> + * returns the integer bpp value from the drm_dsc_config struct
> + */
> +#define DSC_BPP(config) ((config).bits_per_pixel >> 4)
> +
> +/**
> + * struct msm_compression_info - defined panel compression
> + * @enabled: enabled/disabled
> + * @comp_type: type of compression supported
> + * @comp_ratio: compression ratio
> + * @src_bpp: bits per pixel before compression
> + * @tgt_bpp: bits per pixel after compression
> + * @msm_dsc_info: msm dsc info if the compression supported is DSC
> + */
> +struct msm_compression_info {
> + bool enabled;
I would rather use comp_type = MSM_DISPLAY_COMPRESSION_NONE instead of having a supplementary bool....
> + enum msm_display_compression_type comp_type;
> + u32 comp_ratio;
> + u32 src_bpp;
> + u32 tgt_bpp;
Those are never used outside of dp, so get them out.
> + struct msm_display_dsc_info msm_dsc_info;
> +};
> +
> +/**
> * struct msm_display_topology - defines a display topology pipeline
> * @num_lm: number of layer mixers used
> * @num_intf: number of interfaces the panel is mounted on
Powered by blists - more mailing lists