This is a series of changes for DSI to enable command mode support
for DSC v1.2.
This includes:
1) Rounding up `hdisplay / 3` in dsc_timing_setup()
2) Adjusting pclk_rate to account for compression
3) Fixing incorrect uses of slice_count in DSI DSC calculations
4) Setting the DATA_COMPRESS bit when DSC is enabled
With these changes (and the dependency below), DSC v1.2 should work over
DSI in command mode.
Note: Changes that add DSC v1.2 support for video mode will be posted
with the DP support changes.
Depends-on: "add DSC 1.2 dpu supports" [1] and "Introduce MSM-specific
DSC helpers" [2]
[1] https://patchwork.freedesktop.org/series/116789/
[2] https://patchwork.freedesktop.org/series/115833/
Signed-off-by: Jessica Zhang <[email protected]>
---
Changes in v3:
- Added fix to round up hdisplay DSC adjustment
- Fixed inconsistent whitespace in dpu_hw_intf_ops comment doc
- Moved placement of dpu_hw_intf_enable_compression
- Picked up "drm/msm/dsi: Fix calculation for pkt_per_line" patch and
squashed all slice_count fixes into a single patch
- Use drm_mode_vrefresh() to calculate adjusted pclk rate
- Moved compressed pclk adjustment to dsi_adjust_compressed_pclk() helper
- Rebased changes on top of updated dependencies
- Reworded commit message for "drm/msm/dpu: Set DATA_COMPRESS for
command mode" for clarity
- Removed revision changelog in commit messages
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- Changed has_data_compress dpu_cap to a DATA_COMPRESS INTF feature flag
- Changed pclk math to only divide hdisplay by compression ratio
- Reworded word count TODO comment
- Make DATA_COMPRESS an INTF flag
- Read INTF_CONFIG2 before writing to DATA_COMPRESS bit
- Fixed whitespace issue in macro definition
- Removed `inline` from dpu_hw_intf_enable_compression declaration
- Only set dpu_hw_intf_ops.data_compress if DATA_COMPRESS feature is set
- Reworded commit messages and cover letter for clarity
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Jessica Zhang (5):
msm/drm/dsi: Round up DSC hdisplay calculation
drm/msm/dsi: Adjust pclk rate for compression
drm/msm/dpu: Add DPU_INTF_DATA_COMPRESS feature flag
drm/msm/dpu: Set DATA_COMPRESS for command mode
drm/msm/dsi: Remove incorrect references to slice_count
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 3 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 13 ++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +
drivers/gpu/drm/msm/dsi/dsi_host.c | 49 +++++++++++++++-------
6 files changed, 55 insertions(+), 16 deletions(-)
---
base-commit: 2f0218fa4805d7c7eed8dc072e1bdf9f100492c7
change-id: 20230405-add-dsc-support-fe130ba49841
Best regards,
--
Jessica Zhang <[email protected]>
Adjust the pclk rate to divide hdisplay by the compression ratio when DSC
is enabled.
Signed-off-by: Jessica Zhang <[email protected]>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 18d38b90eb28..d04f8bbd707d 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -561,7 +561,18 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host)
clk_disable_unprepare(msm_host->byte_clk);
}
-static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool is_bonded_dsi)
+static unsigned long dsi_adjust_compressed_pclk(const struct drm_display_mode *mode,
+ const struct drm_dsc_config *dsc)
+{
+ int new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc),
+ dsc->bits_per_component * 3);
+
+ return (new_hdisplay + (mode->htotal - mode->hdisplay))
+ * mode->vtotal * drm_mode_vrefresh(mode);
+}
+
+static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode,
+ const struct drm_dsc_config *dsc, bool is_bonded_dsi)
{
unsigned long pclk_rate;
@@ -576,6 +587,10 @@ static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool
if (is_bonded_dsi)
pclk_rate /= 2;
+ /* If DSC is enabled, divide hdisplay by compression ratio */
+ if (dsc)
+ pclk_rate = dsi_adjust_compressed_pclk(mode, dsc);
+
return pclk_rate;
}
@@ -585,7 +600,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d
struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
u8 lanes = msm_host->lanes;
u32 bpp = dsi_get_bpp(msm_host->format);
- unsigned long pclk_rate = dsi_get_pclk_rate(mode, is_bonded_dsi);
+ unsigned long pclk_rate = dsi_get_pclk_rate(mode, msm_host->dsc, is_bonded_dsi);
u64 pclk_bpp = (u64)pclk_rate * bpp;
if (lanes == 0) {
@@ -604,7 +619,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d
static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
{
- msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi);
+ msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi);
msm_host->byte_clk_rate = dsi_byte_clk_get_rate(&msm_host->base, is_bonded_dsi,
msm_host->mode);
@@ -634,7 +649,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
dsi_calc_pclk(msm_host, is_bonded_dsi);
- pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) * bpp;
+ pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi) * bpp;
do_div(pclk_bpp, 8);
msm_host->src_clk_rate = pclk_bpp;
--
2.40.1
Currently, slice_count is being used to calculate word count and
pkt_per_line. In downstream, these values are calculated using slice per
packet, which is not the same as slice_count.
Slice count represents the number of soft slices per interface, and its
value will not always match that of slice per packet. For example, it is
possible to have cases where there are multiple soft slices per interface
but the panel specifies only one slice per packet.
Thus, use the default value of one slice per packet and remove slice_count
from the aforementioned calculations.
Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
Fixes: bc6b6ff8135c ("drm/msm/dsi: Use DSC slice(s) packet size to compute word count")
Signed-off-by: Jessica Zhang <[email protected]>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index d04f8bbd707d..8c8858ee59ec 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -866,18 +866,15 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
*/
slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
- /*
- * If slice_count is greater than slice_per_intf
- * then default to 1. This can happen during partial
- * update.
- */
- if (dsc->slice_count > slice_per_intf)
- dsc->slice_count = 1;
-
total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
eol_byte_num = total_bytes_per_intf % 3;
- pkt_per_line = slice_per_intf / dsc->slice_count;
+
+ /*
+ * Default to 1 slice_per_pkt, so pkt_per_line will be equal to
+ * slice per intf.
+ */
+ pkt_per_line = slice_per_intf;
if (is_cmd_mode) /* packet data type */
reg = DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(MIPI_DSI_DCS_LONG_WRITE);
@@ -1001,7 +998,14 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
if (!msm_host->dsc)
wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
else
- wc = msm_host->dsc->slice_chunk_size * msm_host->dsc->slice_count + 1;
+ /*
+ * When DSC is enabled, WC = slice_chunk_size * slice_per_packet + 1.
+ * Currently, the driver only supports default value of slice_per_packet = 1
+ *
+ * TODO: Expand mipi_dsi_device struct to hold slice_per_packet info
+ * and adjust DSC math to account for slice_per_packet.
+ */
+ wc = msm_host->dsc->slice_chunk_size + 1;
dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
--
2.40.1
On 20/05/2023 00:17, Jessica Zhang wrote:
> Currently, slice_count is being used to calculate word count and
> pkt_per_line. In downstream, these values are calculated using slice per
> packet, which is not the same as slice_count.
I'd say the reference to downstream is not correct. We have seen cases
where the vendor kernel contained errors. So it should be something like
"Instead these values should be calculated using ...."
>
> Slice count represents the number of soft slices per interface, and its
> value will not always match that of slice per packet. For example, it is
> possible to have cases where there are multiple soft slices per interface
> but the panel specifies only one slice per packet.
>
> Thus, use the default value of one slice per packet and remove slice_count
> from the aforementioned calculations.
>
> Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
> Fixes: bc6b6ff8135c ("drm/msm/dsi: Use DSC slice(s) packet size to compute word count")
> Signed-off-by: Jessica Zhang <[email protected]>
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index d04f8bbd707d..8c8858ee59ec 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -866,18 +866,15 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
> */
> slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
>
> - /*
> - * If slice_count is greater than slice_per_intf
> - * then default to 1. This can happen during partial
> - * update.
> - */
> - if (dsc->slice_count > slice_per_intf)
> - dsc->slice_count = 1;
> -
> total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
>
> eol_byte_num = total_bytes_per_intf % 3;
> - pkt_per_line = slice_per_intf / dsc->slice_count;
> +
> + /*
> + * Default to 1 slice_per_pkt, so pkt_per_line will be equal to
> + * slice per intf.
> + */
> + pkt_per_line = slice_per_intf;
>
> if (is_cmd_mode) /* packet data type */
> reg = DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(MIPI_DSI_DCS_LONG_WRITE);
> @@ -1001,7 +998,14 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> if (!msm_host->dsc)
> wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
> else
> - wc = msm_host->dsc->slice_chunk_size * msm_host->dsc->slice_count + 1;
> + /*
> + * When DSC is enabled, WC = slice_chunk_size * slice_per_packet + 1.
> + * Currently, the driver only supports default value of slice_per_packet = 1
> + *
> + * TODO: Expand mipi_dsi_device struct to hold slice_per_packet info
> + * and adjust DSC math to account for slice_per_packet.
> + */
> + wc = msm_host->dsc->slice_chunk_size + 1;
>
> dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
> DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
>
--
With best wishes
Dmitry
On 5/20/2023 5:32 PM, Dmitry Baryshkov wrote:
> On 20/05/2023 00:17, Jessica Zhang wrote:
>> Currently, slice_count is being used to calculate word count and
>> pkt_per_line. In downstream, these values are calculated using slice per
>> packet, which is not the same as slice_count.
>
> I'd say the reference to downstream is not correct. We have seen cases
> where the vendor kernel contained errors. So it should be something like
> "Instead these values should be calculated using ...."
Hi Dmitry,
Acked.
Thanks,
Jessica Zhang
>
>>
>> Slice count represents the number of soft slices per interface, and its
>> value will not always match that of slice per packet. For example, it is
>> possible to have cases where there are multiple soft slices per interface
>> but the panel specifies only one slice per packet.
>>
>> Thus, use the default value of one slice per packet and remove
>> slice_count
>> from the aforementioned calculations.
>>
>> Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
>> Fixes: bc6b6ff8135c ("drm/msm/dsi: Use DSC slice(s) packet size to
>> compute word count")
>> Signed-off-by: Jessica Zhang <[email protected]>
>> ---
>> drivers/gpu/drm/msm/dsi/dsi_host.c | 24 ++++++++++++++----------
>> 1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
>> b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> index d04f8bbd707d..8c8858ee59ec 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> @@ -866,18 +866,15 @@ static void dsi_update_dsc_timing(struct
>> msm_dsi_host *msm_host, bool is_cmd_mod
>> */
>> slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
>> - /*
>> - * If slice_count is greater than slice_per_intf
>> - * then default to 1. This can happen during partial
>> - * update.
>> - */
>> - if (dsc->slice_count > slice_per_intf)
>> - dsc->slice_count = 1;
>> -
>> total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
>> eol_byte_num = total_bytes_per_intf % 3;
>> - pkt_per_line = slice_per_intf / dsc->slice_count;
>> +
>> + /*
>> + * Default to 1 slice_per_pkt, so pkt_per_line will be equal to
>> + * slice per intf.
>> + */
>> + pkt_per_line = slice_per_intf;
>> if (is_cmd_mode) /* packet data type */
>> reg =
>> DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(MIPI_DSI_DCS_LONG_WRITE);
>> @@ -1001,7 +998,14 @@ static void dsi_timing_setup(struct msm_dsi_host
>> *msm_host, bool is_bonded_dsi)
>> if (!msm_host->dsc)
>> wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
>> else
>> - wc = msm_host->dsc->slice_chunk_size *
>> msm_host->dsc->slice_count + 1;
>> + /*
>> + * When DSC is enabled, WC = slice_chunk_size *
>> slice_per_packet + 1.
>> + * Currently, the driver only supports default value of
>> slice_per_packet = 1
>> + *
>> + * TODO: Expand mipi_dsi_device struct to hold
>> slice_per_packet info
>> + * and adjust DSC math to account for
>> slice_per_packet.
>> + */
>> + wc = msm_host->dsc->slice_chunk_size + 1;
>> dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
>> DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
>>
>
> --
> With best wishes
> Dmitry
>