| Message ID | 20230329-rfc-msm-dsc-helper-v11-8-30270e1eeac3@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce MSM-specific DSC helpers | expand |
On 2023-05-17 11:51:17, Jessica Zhang wrote: > > Use MSM and DRM DSC helper methods to configure DSC for DSI. > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 74d38f90398a..b21108948061 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -28,6 +28,7 @@ > #include "dsi.xml.h" > #include "sfpb.xml.h" > #include "dsi_cfg.h" > +#include "msm_dsc_helper.h" > #include "msm_kms.h" > #include "msm_gem.h" > #include "phy/dsi_phy.h" > @@ -848,7 +849,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod > /* 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_slices_per_intf(dsc, hdisplay); > > /* > * If slice_count is greater than slice_per_intf > @@ -858,7 +859,7 @@ 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; > + total_bytes_per_intf = dsc->slice_count * slice_per_intf; Oh no, this line shouldn't have changed. Besides not conforming to the "use MSM and DRM DSC helper methods" title, this is now no longer computing the bytes that we were in v10. Was this tested? - Marijn > > eol_byte_num = total_bytes_per_intf % 3; > pkt_per_line = slice_per_intf / dsc->slice_count; > @@ -1759,7 +1760,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc > return ret; > } > > - dsc->initial_scale_value = 32; > + dsc->initial_scale_value = drm_dsc_initial_scale_value(dsc); > dsc->line_buf_depth = dsc->bits_per_component + 1; > > return drm_dsc_compute_rc_parameters(dsc); > > -- > 2.40.1 >
On 2023-05-17 21:13:36, Marijn Suijten wrote: > > On 2023-05-17 11:51:17, Jessica Zhang wrote: > > > > Use MSM and DRM DSC helper methods to configure DSC for DSI. > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > > --- > > drivers/gpu/drm/msm/dsi/dsi_host.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > > index 74d38f90398a..b21108948061 100644 > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > > @@ -28,6 +28,7 @@ > > #include "dsi.xml.h" > > #include "sfpb.xml.h" > > #include "dsi_cfg.h" > > +#include "msm_dsc_helper.h" > > #include "msm_kms.h" > > #include "msm_gem.h" > > #include "phy/dsi_phy.h" > > @@ -848,7 +849,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod > > /* 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_slices_per_intf(dsc, hdisplay); > > > > /* > > * If slice_count is greater than slice_per_intf > > @@ -858,7 +859,7 @@ 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; > > + total_bytes_per_intf = dsc->slice_count * slice_per_intf; > > Oh no, this line shouldn't have changed. Besides not conforming to the > "use MSM and DRM DSC helper methods" title, this is now no longer > computing the bytes that we were in v10. Was this tested? Regarding testing, it probably goes unnoticed easily because of only being used in eol_byte_num = total_bytes_per_intf % 3: on hdisplay=1096 and slice_count=slice_per_intf=2 both result in eol_byte_num=1 :) - Marijn > > - Marijn > > > > > eol_byte_num = total_bytes_per_intf % 3; > > pkt_per_line = slice_per_intf / dsc->slice_count; > > @@ -1759,7 +1760,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc > > return ret; > > } > > > > - dsc->initial_scale_value = 32; > > + dsc->initial_scale_value = drm_dsc_initial_scale_value(dsc); > > dsc->line_buf_depth = dsc->bits_per_component + 1; > > > > return drm_dsc_compute_rc_parameters(dsc); > > > > -- > > 2.40.1 > >
On 5/17/2023 12:25 PM, Marijn Suijten wrote: > On 2023-05-17 21:13:36, Marijn Suijten wrote: >> >> On 2023-05-17 11:51:17, Jessica Zhang wrote: >>> >>> Use MSM and DRM DSC helper methods to configure DSC for DSI. >>> >>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> >>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >>> --- >>> drivers/gpu/drm/msm/dsi/dsi_host.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c >>> index 74d38f90398a..b21108948061 100644 >>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c >>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c >>> @@ -28,6 +28,7 @@ >>> #include "dsi.xml.h" >>> #include "sfpb.xml.h" >>> #include "dsi_cfg.h" >>> +#include "msm_dsc_helper.h" >>> #include "msm_kms.h" >>> #include "msm_gem.h" >>> #include "phy/dsi_phy.h" >>> @@ -848,7 +849,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod >>> /* 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_slices_per_intf(dsc, hdisplay); >>> >>> /* >>> * If slice_count is greater than slice_per_intf >>> @@ -858,7 +859,7 @@ 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; >>> + total_bytes_per_intf = dsc->slice_count * slice_per_intf; >> >> Oh no, this line shouldn't have changed. Besides not conforming to the >> "use MSM and DRM DSC helper methods" title, this is now no longer >> computing the bytes that we were in v10. Was this tested? > > Regarding testing, it probably goes unnoticed easily because of only > being used in eol_byte_num = total_bytes_per_intf % 3: on hdisplay=1096 > and slice_count=slice_per_intf=2 both result in eol_byte_num=1 :) Hi Marijn, Acked. Will change this to slice_chunk_size. Thanks, Jessica Zhang > > - Marijn > >> >> - Marijn >> >>> >>> eol_byte_num = total_bytes_per_intf % 3; >>> pkt_per_line = slice_per_intf / dsc->slice_count; >>> @@ -1759,7 +1760,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc >>> return ret; >>> } >>> >>> - dsc->initial_scale_value = 32; >>> + dsc->initial_scale_value = drm_dsc_initial_scale_value(dsc); >>> dsc->line_buf_depth = dsc->bits_per_component + 1; >>> >>> return drm_dsc_compute_rc_parameters(dsc); >>> >>> -- >>> 2.40.1 >>>
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 74d38f90398a..b21108948061 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -28,6 +28,7 @@ #include "dsi.xml.h" #include "sfpb.xml.h" #include "dsi_cfg.h" +#include "msm_dsc_helper.h" #include "msm_kms.h" #include "msm_gem.h" #include "phy/dsi_phy.h" @@ -848,7 +849,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod /* 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_slices_per_intf(dsc, hdisplay); /* * If slice_count is greater than slice_per_intf @@ -858,7 +859,7 @@ 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; + total_bytes_per_intf = dsc->slice_count * slice_per_intf; eol_byte_num = total_bytes_per_intf % 3; pkt_per_line = slice_per_intf / dsc->slice_count; @@ -1759,7 +1760,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc return ret; } - dsc->initial_scale_value = 32; + dsc->initial_scale_value = drm_dsc_initial_scale_value(dsc); dsc->line_buf_depth = dsc->bits_per_component + 1; return drm_dsc_compute_rc_parameters(dsc);