On 5/9/2023 11:02 AM, Abhinav Kumar wrote:
On 5/9/2023 4:42 AM, Dmitry Baryshkov wrote:
On 09/05/2023 11:54, Konrad Dybcio wrote:
On 9.05.2023 10:23, Neil Armstrong wrote:
On 09/05/2023 01:27, Dmitry Baryshkov wrote:
On 08/05/2023 23:09, Abhinav Kumar wrote:
On 5/3/2023 1:26 AM, Dmitry Baryshkov wrote:
On 03/05/2023 04:19, Jessica Zhang wrote:
Currently, word count is calculated using slice_count. This is
incorrect
as downstream uses slice per packet, which is different from
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 word count calculation.
Fixes: bc6b6ff8135c ("drm/msm/dsi: Use DSC slice(s) packet size
to compute word count")
Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 35c69dbe5f6f..b0d448ffb078 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -996,7 +996,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 drm_panel struct to hold
slice_per_packet info
+ * and adjust DSC math to account for
slice_per_packet.
slice_per_packet is not a part of the standard DSC, so I'm not
sure how that can be implemented. And definitely we should not
care about the drm_panel here. It should be either a part of
drm_dsc_config, or mipi_dsi_device.
This is not correct.
It is part of the DSI standard (not DSC standard). Please refer to
Figure 40 "One Line Containing One Packet with Data from One or
More Compressed Slices" and Figure 41 "One Line Containing More
than One Compressed Pixel Stream Packet".
I have reviewed section 8.8.24 and Annex D of the DSI standard.
It is not clear to me, if we can get away with always using
slice_per_packet = 1. What is the DSI sink's difference between
Fig. 40.(b) and Fig 41?
Are there are known panels that require slice_per_packet != 1? If
so, we will have to implement support for such configurations.
This has details about this. So I still stand by my point that
this should be in the drm_panel.
Note, the driver doesn't use drm_panel directly. So
slices_per_packet should go to mipi_dsi_device instead (which in
turn can be filled from e.g. drm_panel or from any other source).
This is a big question, where should we set those parameters ?
It's an even bigger questions for panels optionally supporting DSC
in Video or Command mode (like the vtdr6130),
how to select DSC or not ? DT is not an option.
Compressed vs uncompressed modes, maybe? Would be nice to make this
togglable from userspace.. But then it may not scale for panels with
e.g.
10 resolutions, all cmd/vid/dsc/nodsc
Currently the panel/panel-bridge make decision on command vs video
mode. We have no way to influence that decision. If you want to make
that negotiable, I'd start with adding
'cmd_supported/video_supported/dsc_supported' flags to struct
mipi_dsi_hosts.
Right. Isn't that issue there even today that if a panel supports DSC in
only one of the modes, we have no way to tell that as we have not
encountered such a panel in upstream yet.
Also, fundamental question to folks who had panels requiring
slice_per_pkt as 2,
if you had some panels which need a slice_per_pkt as 2, this support
could have been added even earlier by someone who had these panels even
in DSC 1.1.
If these panels are not yet upstreamed, may i please know why this is
considered as a "breakage"? If they were working "somehow" due to
incorrect math / panel settings / DPU calculations, unfortunately we
have to work towards bringing up these panels properly and upstreaming
them rather than saying "oh, these panels were working somehow and now
we need to keep it working".
I also want to add on top of Abhinav's point:
Currently, the panel I'm testing this series on only uses
slice_per_pkt=1, so I have no way to testing slice_per_pkt > 1.
So in the case where we add support for slice_per_pkt > 1, I would
prefer that be as a separate series as I would not be able to validate
those changes on my current setup.
Thanks,
Jessica Zhang
Konrad
Those should tied to a panel+controller tuple.
Neil
+ */
+ 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) |
