On 3/30/2023 6:25 PM, Dmitry Baryshkov wrote:
On 31/03/2023 04:12, Jessica Zhang wrote:
On 3/30/2023 5:16 PM, Dmitry Baryshkov wrote:
On Fri, 31 Mar 2023 at 03:07, Jessica Zhang
<quic_jesszhan@xxxxxxxxxxx> wrote:
On 3/30/2023 4:14 PM, Dmitry Baryshkov wrote:
On 31/03/2023 01:49, Jessica Zhang wrote:
On 3/29/2023 4:48 PM, Dmitry Baryshkov wrote:
On 30/03/2023 02:18, Jessica Zhang wrote:
Use MSM and DRM DSC helper methods.
Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 74d38f90398a..7419fe58a941 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -31,6 +31,7 @@
#include "msm_kms.h"
#include "msm_gem.h"
#include "phy/dsi_phy.h"
+#include "disp/msm_dsc_helper.h"
#define DSI_RESET_TOGGLE_DELAY_MS 20
@@ -841,14 +842,14 @@ static void dsi_update_dsc_timing(struct
msm_dsi_host *msm_host, bool is_cmd_mod
{
struct drm_dsc_config *dsc = msm_host->dsc;
u32 reg, reg_ctrl, reg_ctrl2;
- u32 slice_per_intf, total_bytes_per_intf;
+ u32 slice_per_intf;
u32 pkt_per_line;
u32 eol_byte_num;
/* first calculate dsc parameters and then program
* compress mode registers
*/
- slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->slice_width);
+ slice_per_intf = msm_dsc_get_slice_per_intf(dsc, hdisplay);
This looks good
/*
* If slice_count is greater than slice_per_intf
@@ -858,10 +859,10 @@ static void dsi_update_dsc_timing(struct
msm_dsi_host *msm_host, bool is_cmd_mod
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 = msm_dsc_get_eol_byte_num(msm_host->dsc,
hdisplay,
+ dsi_get_bpp(msm_host->format));
- eol_byte_num = total_bytes_per_intf % 3;
- pkt_per_line = slice_per_intf / dsc->slice_count;
+ pkt_per_line = slice_per_intf / MSM_DSC_SLICE_PER_PKT;
And for these values the result is definitely changed. Separate
patch
& description please. Just in case, "values per downstream
kernel" is
not a proper description for such changes.
Hi Dmitry,
Sure, I can put this into a separate patch.
The reason this was changed from slice_count to SLICE_PER_PKT was
because slice count and slice per packet aren't always equivalent.
There can be cases where panel configures DSC to have multiple soft
slices per interface, but the panel only specifies 1 slice per
packet.
Please put this nice description into the commit message. It is
exactly
what I was looking for!
BTW: Do you expect to change MSM_DSC_SLICE_PER_PKT later or it will
stay
at "1"? If so, it might be easier to drop it and instead add a
comment.
MSM_DSC_SLICE_PER_PKT is the default value for panels that don't
specify
a slice_per_pkt value. (Now that I think about it, might be better to
call it MSM_DSC_DEFAULT_SLICE_PER_PKT instead...)
Note, there is no slice_per_pkt in drm_dsc_config, so we must come up
with another way to pass this data from the panel or to deduce the
value in our driver.
AFAIK we aren't developing on any panels that have a non-default
slice-per-packet count right now, so I'm not sure if it would be worth
the effort to add this if there's no panel we can validate it on.
FWIW, downstream reads slice_per_pkt from a custom DT entry [1]
As you might guess, this approach is frowned upon in upstream kernel.
This info should come from panel (e.g. extend drm_dsc_config or deduce
the value in msm driver).
Yes, I am already aware slice_per_pkt should come from the panel
otherwise defaults to 1. Today MSM driver or even drm_panels for that
matter dont support passing this.
We can extend adding it to the drm_panel and coming from there in
another series. Let this series first add the support for DSC 1.2 for
existing DSC 1.1 configurations. Even DSC 1.1 doesnt use custom
slice_per_pkt today. So nothing broken.
[1]
https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/dsi-panel-r66451-dsc-fhd-plus-120hz-cmd.dtsi#115
I don't expect it to change in the future, but it's a little more
readable than just dividing by 1 IMO. If you prefer dropping the macro
and adding a comment, I'm also okay with that.
There is no need to divide by 1, the value doesn't change. So I'd
probably prefer something like:
/* Default to 1 slice per packet */
if (panel_slice_per_pkt)
pkt_per_line = slice_per_intf / panel_slice_per_pkt;
else
pkt_per_line = slice_per_intf;
Or:
/* Default to 1 slice per packet */
slice_per_pkt = 1;
if (panel_slice_per_pkt)
slice_per_pkt = panel_slice_per_pkt;
pkt_per_line = slice_per_intf / slice_per_pkt;
Acked.
