| Message ID | 20230329-rfc-msm-dsc-helper-v7-2-df48a2c54421@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce MSM-specific DSC helpers | expand |
On 2023-05-09 15:06:48, Jessica Zhang wrote: > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Add a helper setting config values which are typically constant across > operating modes (table E-4 of the standard) and mux_word_size (which is > a const according to 3.5.2). > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> Same question about ordering. Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> > --- > drivers/gpu/drm/display/drm_dsc_helper.c | 22 ++++++++++++++++++++++ > include/drm/display/drm_dsc_helper.h | 1 + > 2 files changed, 23 insertions(+) > > diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c > index 65e810a54257..b9c4e10ced41 100644 > --- a/drivers/gpu/drm/display/drm_dsc_helper.c > +++ b/drivers/gpu/drm/display/drm_dsc_helper.c > @@ -270,6 +270,28 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload, > } > EXPORT_SYMBOL(drm_dsc_pps_payload_pack); > > +/** > + * drm_dsc_set_const_params() - Set DSC parameters considered typically > + * constant across operation modes > + * > + * @vdsc_cfg: > + * DSC Configuration data partially filled by driver > + */ > +void drm_dsc_set_const_params(struct drm_dsc_config *vdsc_cfg) > +{ > + if (!vdsc_cfg->rc_model_size) > + vdsc_cfg->rc_model_size = DSC_RC_MODEL_SIZE_CONST; > + vdsc_cfg->rc_edge_factor = DSC_RC_EDGE_FACTOR_CONST; > + vdsc_cfg->rc_tgt_offset_high = DSC_RC_TGT_OFFSET_HI_CONST; > + vdsc_cfg->rc_tgt_offset_low = DSC_RC_TGT_OFFSET_LO_CONST; > + > + if (vdsc_cfg->bits_per_component <= 10) > + vdsc_cfg->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC; > + else > + vdsc_cfg->mux_word_size = DSC_MUX_WORD_SIZE_12_BPC; > +} > +EXPORT_SYMBOL(drm_dsc_set_const_params); > + > /* From DSC_v1.11 spec, rc_parameter_Set syntax element typically constant */ > static const u16 drm_dsc_rc_buf_thresh[] = { > 896, 1792, 2688, 3584, 4480, 5376, 6272, 6720, 7168, 7616, > diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h > index 422135a33d65..bfa7f3acafcb 100644 > --- a/include/drm/display/drm_dsc_helper.h > +++ b/include/drm/display/drm_dsc_helper.h > @@ -21,6 +21,7 @@ void drm_dsc_dp_pps_header_init(struct dp_sdp_header *pps_header); > int drm_dsc_dp_rc_buffer_size(u8 rc_buffer_block_size, u8 rc_buffer_size); > void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp, > const struct drm_dsc_config *dsc_cfg); > +void drm_dsc_set_const_params(struct drm_dsc_config *vdsc_cfg); > void drm_dsc_set_rc_buf_thresh(struct drm_dsc_config *vdsc_cfg); > int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params_kind kind); > int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg); > > -- > 2.40.1 >
On 5/9/2023 11:29 PM, Marijn Suijten wrote: > On 2023-05-09 15:06:48, Jessica Zhang wrote: >> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> >> Add a helper setting config values which are typically constant across >> operating modes (table E-4 of the standard) and mux_word_size (which is >> a const according to 3.5.2). >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > > Same question about ordering. Hi Marijn, This patch was authored by Dmitry and originally part of his DRM DSC helpers series [1], but was removed from that series for mergeability reasons. Looking over the kernel documentation, the last Signed-off-by should be from the patch submitter [2], so I think my s-o-b tag should be at the bottom. As for the order in the previous patch, I can add a duplicate s-o-b before Dmitry's so that it reflects the history of the patch. Thanks, Jessica Zhang [1] https://patchwork.freedesktop.org/patch/530512/?series=114472&rev=4 [2] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by > > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> > >> --- >> drivers/gpu/drm/display/drm_dsc_helper.c | 22 ++++++++++++++++++++++ >> include/drm/display/drm_dsc_helper.h | 1 + >> 2 files changed, 23 insertions(+) >> >> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c >> index 65e810a54257..b9c4e10ced41 100644 >> --- a/drivers/gpu/drm/display/drm_dsc_helper.c >> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c >> @@ -270,6 +270,28 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload, >> } >> EXPORT_SYMBOL(drm_dsc_pps_payload_pack); >> >> +/** >> + * drm_dsc_set_const_params() - Set DSC parameters considered typically >> + * constant across operation modes >> + * >> + * @vdsc_cfg: >> + * DSC Configuration data partially filled by driver >> + */ >> +void drm_dsc_set_const_params(struct drm_dsc_config *vdsc_cfg) >> +{ >> + if (!vdsc_cfg->rc_model_size) >> + vdsc_cfg->rc_model_size = DSC_RC_MODEL_SIZE_CONST; >> + vdsc_cfg->rc_edge_factor = DSC_RC_EDGE_FACTOR_CONST; >> + vdsc_cfg->rc_tgt_offset_high = DSC_RC_TGT_OFFSET_HI_CONST; >> + vdsc_cfg->rc_tgt_offset_low = DSC_RC_TGT_OFFSET_LO_CONST; >> + >> + if (vdsc_cfg->bits_per_component <= 10) >> + vdsc_cfg->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC; >> + else >> + vdsc_cfg->mux_word_size = DSC_MUX_WORD_SIZE_12_BPC; >> +} >> +EXPORT_SYMBOL(drm_dsc_set_const_params); >> + >> /* From DSC_v1.11 spec, rc_parameter_Set syntax element typically constant */ >> static const u16 drm_dsc_rc_buf_thresh[] = { >> 896, 1792, 2688, 3584, 4480, 5376, 6272, 6720, 7168, 7616, >> diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h >> index 422135a33d65..bfa7f3acafcb 100644 >> --- a/include/drm/display/drm_dsc_helper.h >> +++ b/include/drm/display/drm_dsc_helper.h >> @@ -21,6 +21,7 @@ void drm_dsc_dp_pps_header_init(struct dp_sdp_header *pps_header); >> int drm_dsc_dp_rc_buffer_size(u8 rc_buffer_block_size, u8 rc_buffer_size); >> void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp, >> const struct drm_dsc_config *dsc_cfg); >> +void drm_dsc_set_const_params(struct drm_dsc_config *vdsc_cfg); >> void drm_dsc_set_rc_buf_thresh(struct drm_dsc_config *vdsc_cfg); >> int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params_kind kind); >> int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg); >> >> -- >> 2.40.1 >>
On 11/05/2023 01:35, Jessica Zhang wrote: > > > On 5/9/2023 11:29 PM, Marijn Suijten wrote: >> On 2023-05-09 15:06:48, Jessica Zhang wrote: >>> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> >>> Add a helper setting config values which are typically constant across >>> operating modes (table E-4 of the standard) and mux_word_size (which is >>> a const according to 3.5.2). >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> >> Same question about ordering. > > Hi Marijn, > > This patch was authored by Dmitry and originally part of his DRM DSC > helpers series [1], but was removed from that series for mergeability > reasons. > > Looking over the kernel documentation, the last Signed-off-by should be > from the patch submitter [2], so I think my s-o-b tag should be at the > bottom. > > As for the order in the previous patch, I can add a duplicate s-o-b > before Dmitry's so that it reflects the history of the patch. I think this is an overkill. Instead you can drop my SOB from the patch 1. We do not need this level of detail. For this patch the ordering of tags is correct.
On 2023-05-11 07:26:28, Dmitry Baryshkov wrote: > On 11/05/2023 01:35, Jessica Zhang wrote: > > > > > > On 5/9/2023 11:29 PM, Marijn Suijten wrote: > >> On 2023-05-09 15:06:48, Jessica Zhang wrote: > >>> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >>> > >>> Add a helper setting config values which are typically constant across > >>> operating modes (table E-4 of the standard) and mux_word_size (which is > >>> a const according to 3.5.2). > >>> > >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > >> > >> Same question about ordering. > > > > Hi Marijn, > > > > This patch was authored by Dmitry and originally part of his DRM DSC > > helpers series [1], but was removed from that series for mergeability > > reasons. > > > > Looking over the kernel documentation, the last Signed-off-by should be > > from the patch submitter [2], so I think my s-o-b tag should be at the > > bottom. That's true, but I also think the S-o-B at the top should match the From: author. > > As for the order in the previous patch, I can add a duplicate s-o-b > > before Dmitry's so that it reflects the history of the patch. > > I think this is an overkill. Instead you can drop my SOB from the patch > 1. We do not need this level of detail. > > For this patch the ordering of tags is correct. So indeed, that either means duplicating the S-o-B or dropping it entirely as we do not care that it was part of that series earlier. Dmitry will likely sign this off once again when picking the patches. - Marijn
On 11/05/2023 09:18, Marijn Suijten wrote: > On 2023-05-11 07:26:28, Dmitry Baryshkov wrote: >> On 11/05/2023 01:35, Jessica Zhang wrote: >>> >>> >>> On 5/9/2023 11:29 PM, Marijn Suijten wrote: >>>> On 2023-05-09 15:06:48, Jessica Zhang wrote: >>>>> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>>> >>>>> Add a helper setting config values which are typically constant across >>>>> operating modes (table E-4 of the standard) and mux_word_size (which is >>>>> a const according to 3.5.2). >>>>> >>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >>>> >>>> Same question about ordering. >>> >>> Hi Marijn, >>> >>> This patch was authored by Dmitry and originally part of his DRM DSC >>> helpers series [1], but was removed from that series for mergeability >>> reasons. >>> >>> Looking over the kernel documentation, the last Signed-off-by should be >>> from the patch submitter [2], so I think my s-o-b tag should be at the >>> bottom. > > That's true, but I also think the S-o-B at the top should match the > From: author. I'd say, this is usual, but not the required order of tags. > >>> As for the order in the previous patch, I can add a duplicate s-o-b >>> before Dmitry's so that it reflects the history of the patch. >> >> I think this is an overkill. Instead you can drop my SOB from the patch >> 1. We do not need this level of detail. >> >> For this patch the ordering of tags is correct. > > So indeed, that either means duplicating the S-o-B or dropping it > entirely as we do not care that it was part of that series earlier. > Dmitry will likely sign this off once again when picking the patches. Yes. I'd suggest the following tags: Patch 1 (Add flatness...): From: Jessica SOB: Jessica Patches 2 (add helper to set) and 3 (use DRM DSC helpers): From: Dmitry SOB: Dmitry SOB: Jessica > > - Marijn
On 2023-05-11 09:22:47, Dmitry Baryshkov wrote: > On 11/05/2023 09:18, Marijn Suijten wrote: > > On 2023-05-11 07:26:28, Dmitry Baryshkov wrote: > >> On 11/05/2023 01:35, Jessica Zhang wrote: > >>> > >>> > >>> On 5/9/2023 11:29 PM, Marijn Suijten wrote: > >>>> On 2023-05-09 15:06:48, Jessica Zhang wrote: > >>>>> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >>>>> > >>>>> Add a helper setting config values which are typically constant across > >>>>> operating modes (table E-4 of the standard) and mux_word_size (which is > >>>>> a const according to 3.5.2). > >>>>> > >>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >>>>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > >>>> > >>>> Same question about ordering. > >>> > >>> Hi Marijn, > >>> > >>> This patch was authored by Dmitry and originally part of his DRM DSC > >>> helpers series [1], but was removed from that series for mergeability > >>> reasons. > >>> > >>> Looking over the kernel documentation, the last Signed-off-by should be > >>> from the patch submitter [2], so I think my s-o-b tag should be at the > >>> bottom. > > > > That's true, but I also think the S-o-B at the top should match the > > From: author. > > I'd say, this is usual, but not the required order of tags. > > > > >>> As for the order in the previous patch, I can add a duplicate s-o-b > >>> before Dmitry's so that it reflects the history of the patch. > >> > >> I think this is an overkill. Instead you can drop my SOB from the patch > >> 1. We do not need this level of detail. > >> > >> For this patch the ordering of tags is correct. > > > > So indeed, that either means duplicating the S-o-B or dropping it > > entirely as we do not care that it was part of that series earlier. > > Dmitry will likely sign this off once again when picking the patches. > > Yes. > > I'd suggest the following tags: > > Patch 1 (Add flatness...): > From: Jessica > SOB: Jessica > > Patches 2 (add helper to set) and 3 (use DRM DSC helpers): > From: Dmitry > SOB: Dmitry > SOB: Jessica Both sound exactly right, as we do not care about that fact that the first patch was temporarily picked up by you in another series, and then dropped again. - Marijn
diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c index 65e810a54257..b9c4e10ced41 100644 --- a/drivers/gpu/drm/display/drm_dsc_helper.c +++ b/drivers/gpu/drm/display/drm_dsc_helper.c @@ -270,6 +270,28 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload, } EXPORT_SYMBOL(drm_dsc_pps_payload_pack); +/** + * drm_dsc_set_const_params() - Set DSC parameters considered typically + * constant across operation modes + * + * @vdsc_cfg: + * DSC Configuration data partially filled by driver + */ +void drm_dsc_set_const_params(struct drm_dsc_config *vdsc_cfg) +{ + if (!vdsc_cfg->rc_model_size) + vdsc_cfg->rc_model_size = DSC_RC_MODEL_SIZE_CONST; + vdsc_cfg->rc_edge_factor = DSC_RC_EDGE_FACTOR_CONST; + vdsc_cfg->rc_tgt_offset_high = DSC_RC_TGT_OFFSET_HI_CONST; + vdsc_cfg->rc_tgt_offset_low = DSC_RC_TGT_OFFSET_LO_CONST; + + if (vdsc_cfg->bits_per_component <= 10) + vdsc_cfg->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC; + else + vdsc_cfg->mux_word_size = DSC_MUX_WORD_SIZE_12_BPC; +} +EXPORT_SYMBOL(drm_dsc_set_const_params); + /* From DSC_v1.11 spec, rc_parameter_Set syntax element typically constant */ static const u16 drm_dsc_rc_buf_thresh[] = { 896, 1792, 2688, 3584, 4480, 5376, 6272, 6720, 7168, 7616, diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h index 422135a33d65..bfa7f3acafcb 100644 --- a/include/drm/display/drm_dsc_helper.h +++ b/include/drm/display/drm_dsc_helper.h @@ -21,6 +21,7 @@ void drm_dsc_dp_pps_header_init(struct dp_sdp_header *pps_header); int drm_dsc_dp_rc_buffer_size(u8 rc_buffer_block_size, u8 rc_buffer_size); void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp, const struct drm_dsc_config *dsc_cfg); +void drm_dsc_set_const_params(struct drm_dsc_config *vdsc_cfg); void drm_dsc_set_rc_buf_thresh(struct drm_dsc_config *vdsc_cfg); int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params_kind kind); int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);