| Message ID | 1682725511-18185-8-git-send-email-quic_khsieh@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add DSC 1.2 dpu supports | expand |
On 29/04/2023 02:45, Kuogee Hsieh wrote: > During DSC preparation, add run time calculation to figure out what > usage modes, split mode and merge mode, is going to be setup. This patch doesn't determine the mode. It changes programming of DSC bits according to the mode being selected. > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 56 ++++++++++++++++------------- > 1 file changed, 31 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 2fdacf1..3d18642 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -529,17 +529,9 @@ void dpu_encoder_helper_split_config( > bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc) > { > struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); > - int i, intf_count = 0, num_dsc = 0; > + struct msm_display_topology *topology = &dpu_enc->topology; > > - for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++) > - if (dpu_enc->phys_encs[i]) > - intf_count++; > - > - /* See dpu_encoder_get_topology, we only support 2:2:1 topology */ > - if (dpu_enc->dsc) > - num_dsc = 2; > - > - return (num_dsc > 0) && (num_dsc > intf_count); > + return (topology->num_dsc > topology->num_intf); > } > > static void dpu_encoder_get_topology( > @@ -1861,41 +1853,55 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, > struct dpu_encoder_phys *enc_master = dpu_enc->cur_master; > struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC]; > struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC]; > - int this_frame_slices; > + struct msm_display_topology *topology = &dpu_enc->topology; > int intf_ip_w, enc_ip_w; > - int dsc_common_mode; > - int pic_width; > + int dsc_common_mode = 0; Please don't top-init variables unless required (or unless they are constant). > u32 initial_lines; > + int num_dsc = topology->num_dsc; > + int num_intf = topology->num_intf; > int i; > > - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) { > + for (i = 0; i < num_dsc; i++) { > hw_pp[i] = dpu_enc->hw_pp[i]; > hw_dsc[i] = dpu_enc->hw_dsc[i]; > > if (!hw_pp[i] || !hw_dsc[i]) { > DPU_ERROR_ENC(dpu_enc, "invalid params for DSC\n"); > return; > - } > + } What is the difference here? > } > > - dsc_common_mode = 0; > - pic_width = dsc->pic_width; > + intf_ip_w = dsc->pic_width; > > - dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL; > if (enc_master->intf_mode == INTF_MODE_VIDEO) > dsc_common_mode |= DSC_MODE_VIDEO; > > - this_frame_slices = pic_width / dsc->slice_width; > - intf_ip_w = this_frame_slices * dsc->slice_width; > - > /* > - * dsc merge case: when using 2 encoders for the same stream, > - * no. of slices need to be same on both the encoders. > + * If this encoder is driving more than one DSC encoder, they > + * operate in tandem, same pic dimension needs to be used by > + * each of them.(pp-split is assumed to be not supported) > + * Extra empty line. Also the comment doesn't make sense here anymore. We already have comment for the division by two below. > */ > - enc_ip_w = intf_ip_w / 2; > + enc_ip_w = intf_ip_w; > + > + intf_ip_w /= num_intf; > + > + if (num_dsc > 1) > + dsc_common_mode |= DSC_MODE_SPLIT_PANEL; > + > + if (dpu_encoder_use_dsc_merge(&dpu_enc->base)) { > + dsc_common_mode |= DSC_MODE_MULTIPLEX; > + /* > + * in dsc merge case: when using 2 encoders for the same > + * stream, no. of slices need to be same on both the > + * encoders. > + */ > + enc_ip_w = intf_ip_w / 2; > + } > + > initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w); > > - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) > + for (i = 0; i < num_dsc; i++) > dpu_encoder_dsc_pipe_cfg(dpu_enc, hw_dsc[i], hw_pp[i], dsc, > dsc_common_mode, initial_lines); > }
On 4/28/2023 5:52 PM, Dmitry Baryshkov wrote: > On 29/04/2023 02:45, Kuogee Hsieh wrote: >> During DSC preparation, add run time calculation to figure out what >> usage modes, split mode and merge mode, is going to be setup. > > This patch doesn't determine the mode. It changes programming of DSC > bits according to the mode being selected. > The term mode is a bit confusing here but he is referring to dsc_common_mode. >> >> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 56 >> ++++++++++++++++------------- >> 1 file changed, 31 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> index 2fdacf1..3d18642 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> @@ -529,17 +529,9 @@ void dpu_encoder_helper_split_config( >> bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc) >> { >> struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); >> - int i, intf_count = 0, num_dsc = 0; >> + struct msm_display_topology *topology = &dpu_enc->topology; >> - for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++) >> - if (dpu_enc->phys_encs[i]) >> - intf_count++; >> - >> - /* See dpu_encoder_get_topology, we only support 2:2:1 topology */ >> - if (dpu_enc->dsc) >> - num_dsc = 2; >> - >> - return (num_dsc > 0) && (num_dsc > intf_count); >> + return (topology->num_dsc > topology->num_intf); >> } >> static void dpu_encoder_get_topology( >> @@ -1861,41 +1853,55 @@ static void dpu_encoder_prep_dsc(struct >> dpu_encoder_virt *dpu_enc, >> struct dpu_encoder_phys *enc_master = dpu_enc->cur_master; >> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC]; >> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC]; >> - int this_frame_slices; >> + struct msm_display_topology *topology = &dpu_enc->topology; >> int intf_ip_w, enc_ip_w; >> - int dsc_common_mode; >> - int pic_width; >> + int dsc_common_mode = 0; > > Please don't top-init variables unless required (or unless they are > constant). > >> u32 initial_lines; >> + int num_dsc = topology->num_dsc; >> + int num_intf = topology->num_intf; >> int i; >> - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) { >> + for (i = 0; i < num_dsc; i++) { >> hw_pp[i] = dpu_enc->hw_pp[i]; >> hw_dsc[i] = dpu_enc->hw_dsc[i]; >> if (!hw_pp[i] || !hw_dsc[i]) { >> DPU_ERROR_ENC(dpu_enc, "invalid params for DSC\n"); >> return; >> - } >> + } > > What is the difference here? > >> } >> - dsc_common_mode = 0; >> - pic_width = dsc->pic_width; >> + intf_ip_w = dsc->pic_width; >> - dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL; >> if (enc_master->intf_mode == INTF_MODE_VIDEO) >> dsc_common_mode |= DSC_MODE_VIDEO; >> - this_frame_slices = pic_width / dsc->slice_width; >> - intf_ip_w = this_frame_slices * dsc->slice_width; >> - >> /* >> - * dsc merge case: when using 2 encoders for the same stream, >> - * no. of slices need to be same on both the encoders. >> + * If this encoder is driving more than one DSC encoder, they >> + * operate in tandem, same pic dimension needs to be used by >> + * each of them.(pp-split is assumed to be not supported) >> + * > > Extra empty line. Also the comment doesn't make sense here anymore. We > already have comment for the division by two below. > >> */ >> - enc_ip_w = intf_ip_w / 2; >> + enc_ip_w = intf_ip_w; >> + >> + intf_ip_w /= num_intf; >> + >> + if (num_dsc > 1) >> + dsc_common_mode |= DSC_MODE_SPLIT_PANEL; >> + >> + if (dpu_encoder_use_dsc_merge(&dpu_enc->base)) { >> + dsc_common_mode |= DSC_MODE_MULTIPLEX; >> + /* >> + * in dsc merge case: when using 2 encoders for the same >> + * stream, no. of slices need to be same on both the >> + * encoders. >> + */ >> + enc_ip_w = intf_ip_w / 2; >> + } >> + >> initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w); >> - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) >> + for (i = 0; i < num_dsc; i++) >> dpu_encoder_dsc_pipe_cfg(dpu_enc, hw_dsc[i], hw_pp[i], dsc, >> dsc_common_mode, initial_lines); >> } >
On 29/04/2023 04:22, Abhinav Kumar wrote: > > > On 4/28/2023 5:52 PM, Dmitry Baryshkov wrote: >> On 29/04/2023 02:45, Kuogee Hsieh wrote: >>> During DSC preparation, add run time calculation to figure out what >>> usage modes, split mode and merge mode, is going to be setup. >> >> This patch doesn't determine the mode. It changes programming of DSC >> bits according to the mode being selected. >> > > The term mode is a bit confusing here but he is referring to > dsc_common_mode. Yes, that's clear. The patch description is not correct. > >>> >>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> >>> --- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 56 >>> ++++++++++++++++------------- >>> 1 file changed, 31 insertions(+), 25 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> index 2fdacf1..3d18642 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >>> @@ -529,17 +529,9 @@ void dpu_encoder_helper_split_config( >>> bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc) >>> { >>> struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); >>> - int i, intf_count = 0, num_dsc = 0; >>> + struct msm_display_topology *topology = &dpu_enc->topology; >>> - for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++) >>> - if (dpu_enc->phys_encs[i]) >>> - intf_count++; >>> - >>> - /* See dpu_encoder_get_topology, we only support 2:2:1 topology */ >>> - if (dpu_enc->dsc) >>> - num_dsc = 2; >>> - >>> - return (num_dsc > 0) && (num_dsc > intf_count); >>> + return (topology->num_dsc > topology->num_intf); >>> } >>> static void dpu_encoder_get_topology( >>> @@ -1861,41 +1853,55 @@ static void dpu_encoder_prep_dsc(struct >>> dpu_encoder_virt *dpu_enc, >>> struct dpu_encoder_phys *enc_master = dpu_enc->cur_master; >>> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC]; >>> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC]; >>> - int this_frame_slices; >>> + struct msm_display_topology *topology = &dpu_enc->topology; >>> int intf_ip_w, enc_ip_w; >>> - int dsc_common_mode; >>> - int pic_width; >>> + int dsc_common_mode = 0; >> >> Please don't top-init variables unless required (or unless they are >> constant). >> >>> u32 initial_lines; >>> + int num_dsc = topology->num_dsc; >>> + int num_intf = topology->num_intf; >>> int i; >>> - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) { >>> + for (i = 0; i < num_dsc; i++) { >>> hw_pp[i] = dpu_enc->hw_pp[i]; >>> hw_dsc[i] = dpu_enc->hw_dsc[i]; >>> if (!hw_pp[i] || !hw_dsc[i]) { >>> DPU_ERROR_ENC(dpu_enc, "invalid params for DSC\n"); >>> return; >>> - } >>> + } >> >> What is the difference here? >> >>> } >>> - dsc_common_mode = 0; >>> - pic_width = dsc->pic_width; >>> + intf_ip_w = dsc->pic_width; >>> - dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL; >>> if (enc_master->intf_mode == INTF_MODE_VIDEO) >>> dsc_common_mode |= DSC_MODE_VIDEO; >>> - this_frame_slices = pic_width / dsc->slice_width; >>> - intf_ip_w = this_frame_slices * dsc->slice_width; >>> - >>> /* >>> - * dsc merge case: when using 2 encoders for the same stream, >>> - * no. of slices need to be same on both the encoders. >>> + * If this encoder is driving more than one DSC encoder, they >>> + * operate in tandem, same pic dimension needs to be used by >>> + * each of them.(pp-split is assumed to be not supported) >>> + * >> >> Extra empty line. Also the comment doesn't make sense here anymore. We >> already have comment for the division by two below. >> >>> */ >>> - enc_ip_w = intf_ip_w / 2; >>> + enc_ip_w = intf_ip_w; >>> + >>> + intf_ip_w /= num_intf; >>> + >>> + if (num_dsc > 1) >>> + dsc_common_mode |= DSC_MODE_SPLIT_PANEL; >>> + >>> + if (dpu_encoder_use_dsc_merge(&dpu_enc->base)) { >>> + dsc_common_mode |= DSC_MODE_MULTIPLEX; >>> + /* >>> + * in dsc merge case: when using 2 encoders for the same >>> + * stream, no. of slices need to be same on both the >>> + * encoders. >>> + */ >>> + enc_ip_w = intf_ip_w / 2; >>> + } >>> + >>> initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w); >>> - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) >>> + for (i = 0; i < num_dsc; i++) >>> dpu_encoder_dsc_pipe_cfg(dpu_enc, hw_dsc[i], hw_pp[i], dsc, >>> dsc_common_mode, initial_lines); >>> } >>
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 2fdacf1..3d18642 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -529,17 +529,9 @@ void dpu_encoder_helper_split_config( bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc) { struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); - int i, intf_count = 0, num_dsc = 0; + struct msm_display_topology *topology = &dpu_enc->topology; - for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++) - if (dpu_enc->phys_encs[i]) - intf_count++; - - /* See dpu_encoder_get_topology, we only support 2:2:1 topology */ - if (dpu_enc->dsc) - num_dsc = 2; - - return (num_dsc > 0) && (num_dsc > intf_count); + return (topology->num_dsc > topology->num_intf); } static void dpu_encoder_get_topology( @@ -1861,41 +1853,55 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, struct dpu_encoder_phys *enc_master = dpu_enc->cur_master; struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC]; struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC]; - int this_frame_slices; + struct msm_display_topology *topology = &dpu_enc->topology; int intf_ip_w, enc_ip_w; - int dsc_common_mode; - int pic_width; + int dsc_common_mode = 0; u32 initial_lines; + int num_dsc = topology->num_dsc; + int num_intf = topology->num_intf; int i; - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) { + for (i = 0; i < num_dsc; i++) { hw_pp[i] = dpu_enc->hw_pp[i]; hw_dsc[i] = dpu_enc->hw_dsc[i]; if (!hw_pp[i] || !hw_dsc[i]) { DPU_ERROR_ENC(dpu_enc, "invalid params for DSC\n"); return; - } + } } - dsc_common_mode = 0; - pic_width = dsc->pic_width; + intf_ip_w = dsc->pic_width; - dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL; if (enc_master->intf_mode == INTF_MODE_VIDEO) dsc_common_mode |= DSC_MODE_VIDEO; - this_frame_slices = pic_width / dsc->slice_width; - intf_ip_w = this_frame_slices * dsc->slice_width; - /* - * dsc merge case: when using 2 encoders for the same stream, - * no. of slices need to be same on both the encoders. + * If this encoder is driving more than one DSC encoder, they + * operate in tandem, same pic dimension needs to be used by + * each of them.(pp-split is assumed to be not supported) + * */ - enc_ip_w = intf_ip_w / 2; + enc_ip_w = intf_ip_w; + + intf_ip_w /= num_intf; + + if (num_dsc > 1) + dsc_common_mode |= DSC_MODE_SPLIT_PANEL; + + if (dpu_encoder_use_dsc_merge(&dpu_enc->base)) { + dsc_common_mode |= DSC_MODE_MULTIPLEX; + /* + * in dsc merge case: when using 2 encoders for the same + * stream, no. of slices need to be same on both the + * encoders. + */ + enc_ip_w = intf_ip_w / 2; + } + initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w); - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) + for (i = 0; i < num_dsc; i++) dpu_encoder_dsc_pipe_cfg(dpu_enc, hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, initial_lines); }
During DSC preparation, add run time calculation to figure out what usage modes, split mode and merge mode, is going to be setup. Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 56 ++++++++++++++++------------- 1 file changed, 31 insertions(+), 25 deletions(-)