| Message ID | 1683196599-3730-1-git-send-email-quic_dikshita@quicinc.com |
|---|---|
| Headers | show |
| Series | venus: add support for 10 bit decoding | expand |
On 04/05/2023 11:36, Dikshita Agarwal wrote: > This series includes the changes to > - add V4L2_PIX_FMT_P010 as supported decoder format. > - consider dpb color format while calculating buffer > size for dpb buffers. > - update dpb and opb color format when bit depth > changes is detected, also update preferred color > format to P010 in this case. > > With this series, divided the previous version [1] into > multiple patches as suggested in review comments. > > [1] https://patchwork.linuxtv.org/project/linux-media/list/?series=10376 > > Dikshita Agarwal (4): > venus: add support for V4L2_PIX_FMT_P010 color format > venus: update calculation for dpb buffers > venus: add handling of bit depth change from firmwar > venus: return P010 as preferred format for 10 bit decode > > drivers/media/platform/qcom/venus/helpers.c | 24 ++++++++++++++++++++++ > drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++ > .../media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++- > drivers/media/platform/qcom/venus/vdec.c | 16 +++++++++++++-- > 4 files changed, 48 insertions(+), 3 deletions(-) > For future reference a series like this should: 1. Include a log of what changed between the last series and this 2. Describe which comments you addressed I generally try to say "Added newline to dts - Konrad" "Sent the series as a -v3 - Bryan" etc 3. Ideally provide a link to the previous series in https://lore.kernel.org/linux-arm-msm/1682492417-20496-1-git-send-email-quic_dikshita@quicinc.com/ 4. Use versioning This set should be prefixed with "v2-0000-cover-letter" "v2-0001-add-support" etc "git format-patch mybase..targettip --cover-letter -v2" --- bod
On 04/05/2023 14:49, Bryan O'Donoghue wrote: > On 04/05/2023 11:36, Dikshita Agarwal wrote: >> This series includes the changes to >> - add V4L2_PIX_FMT_P010 as supported decoder format. >> - consider dpb color format while calculating buffer >> size for dpb buffers. >> - update dpb and opb color format when bit depth >> changes is detected, also update preferred color >> format to P010 in this case. >> >> With this series, divided the previous version [1] into >> multiple patches as suggested in review comments. >> >> [1] https://patchwork.linuxtv.org/project/linux-media/list/?series=10376 >> >> Dikshita Agarwal (4): >> venus: add support for V4L2_PIX_FMT_P010 color format >> venus: update calculation for dpb buffers >> venus: add handling of bit depth change from firmwar >> venus: return P010 as preferred format for 10 bit decode >> >> drivers/media/platform/qcom/venus/helpers.c | 24 >> ++++++++++++++++++++++ >> drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++ >> .../media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++- >> drivers/media/platform/qcom/venus/vdec.c | 16 +++++++++++++-- >> 4 files changed, 48 insertions(+), 3 deletions(-) >> > > For future reference a series like this should: > > 1. Include a log of what changed between the last series and this > 2. Describe which comments you addressed > I generally try to say > "Added newline to dts - Konrad" > "Sent the series as a -v3 - Bryan" > etc > 3. Ideally provide a link to the previous series in > > https://lore.kernel.org/linux-arm-msm/1682492417-20496-1-git-send-email-quic_dikshita@quicinc.com/ > 4. Use versioning > This set should be prefixed with "v2-0000-cover-letter" > "v2-0001-add-support" etc > > "git format-patch mybase..targettip --cover-letter -v2" > > --- > bod Doh I see you did most of that - just missed the V2. Please remember to version your subsequent series. "git format-patch -v2" --- bod
On 5/4/2023 7:21 PM, Bryan O'Donoghue wrote: > On 04/05/2023 14:49, Bryan O'Donoghue wrote: >> On 04/05/2023 11:36, Dikshita Agarwal wrote: >>> This series includes the changes to >>> - add V4L2_PIX_FMT_P010 as supported decoder format. >>> - consider dpb color format while calculating buffer >>> size for dpb buffers. >>> - update dpb and opb color format when bit depth >>> changes is detected, also update preferred color >>> format to P010 in this case. >>> >>> With this series, divided the previous version [1] into >>> multiple patches as suggested in review comments. >>> >>> [1] >>> https://patchwork.linuxtv.org/project/linux-media/list/?series=10376 >>> >>> Dikshita Agarwal (4): >>> venus: add support for V4L2_PIX_FMT_P010 color format >>> venus: update calculation for dpb buffers >>> venus: add handling of bit depth change from firmwar >>> venus: return P010 as preferred format for 10 bit decode >>> >>> drivers/media/platform/qcom/venus/helpers.c | 24 >>> ++++++++++++++++++++++ >>> drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++ >>> .../media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++- >>> drivers/media/platform/qcom/venus/vdec.c | 16 >>> +++++++++++++-- >>> 4 files changed, 48 insertions(+), 3 deletions(-) >>> >> >> For future reference a series like this should: >> >> 1. Include a log of what changed between the last series and this >> 2. Describe which comments you addressed >> I generally try to say >> "Added newline to dts - Konrad" >> "Sent the series as a -v3 - Bryan" >> etc >> 3. Ideally provide a link to the previous series in >> >> https://lore.kernel.org/linux-arm-msm/1682492417-20496-1-git-send-email-quic_dikshita@quicinc.com/ >> >> 4. Use versioning >> This set should be prefixed with "v2-0000-cover-letter" >> "v2-0001-add-support" etc >> >> "git format-patch mybase..targettip --cover-letter -v2" >> >> --- >> bod > > Doh I see you did most of that - just missed the V2. > > Please remember to version your subsequent series. "git format-patch -v2" Does this qualify for a version upgrade when a single patch is subsequently raised as series ? IMO, the link to previous single patch in cover letter and then starting the series (as v0) seems to provide the required info. > > --- > bod
On 04/05/2023 15:04, Vikash Garodia wrote: >> Doh I see you did most of that - just missed the V2. >> >> Please remember to version your subsequent series. "git format-patch -v2" > > Does this qualify for a version upgrade when a single patch is > subsequently raised as series ? IMO, the link > > to previous single patch in cover letter and then starting the series > (as v0) seems to provide the required info. Hmm. I'd say any series should have an increment in it to differentiate, with the exception being RESEND. Also you are splitting one patch into four. Looking through a bunch of email it might be not immediately obvious to understand that the new series and old series differ, which is IMO how the version numbers help others to know what's going on. --- bod
On 4.05.2023 12:36, Dikshita Agarwal wrote: > add V4L2_PIX_FMT_P010 as supported color format for decoder. > > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > drivers/media/platform/qcom/venus/helpers.c | 2 ++ > drivers/media/platform/qcom/venus/vdec.c | 4 ++++ > 2 files changed, 6 insertions(+) > > diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c > index ab6a29f..5946def 100644 > --- a/drivers/media/platform/qcom/venus/helpers.c > +++ b/drivers/media/platform/qcom/venus/helpers.c > @@ -612,6 +612,8 @@ static u32 to_hfi_raw_fmt(u32 v4l2_fmt) > return HFI_COLOR_FORMAT_NV12_UBWC; > case V4L2_PIX_FMT_QC10C: > return HFI_COLOR_FORMAT_YUV420_TP10_UBWC; > + case V4L2_PIX_FMT_P010: > + return HFI_COLOR_FORMAT_P010; > default: > break; > } > diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c > index 4ceaba3..687d62e 100644 > --- a/drivers/media/platform/qcom/venus/vdec.c > +++ b/drivers/media/platform/qcom/venus/vdec.c > @@ -43,6 +43,10 @@ static const struct venus_format vdec_formats[] = { > .num_planes = 1, > .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, > }, { > + .pixfmt = V4L2_PIX_FMT_P010, > + .num_planes = 1, > + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, > + }, { > .pixfmt = V4L2_PIX_FMT_MPEG4, > .num_planes = 1, > .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
On 4.05.2023 12:36, Dikshita Agarwal wrote: > Use dpb color format, width and height of output port > for calculating buffer size of dpb buffers. > > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- Looks sane but I'm not exactly an expert on this Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > drivers/media/platform/qcom/venus/helpers.c | 4 ++++ > drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++ > drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++- > 3 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c > index 5946def..4ad6232 100644 > --- a/drivers/media/platform/qcom/venus/helpers.c > +++ b/drivers/media/platform/qcom/venus/helpers.c > @@ -641,12 +641,16 @@ static int platform_get_bufreq(struct venus_inst *inst, u32 buftype, > if (is_dec) { > params.width = inst->width; > params.height = inst->height; > + params.out_width = inst->out_width; > + params.out_height = inst->out_height; > params.codec = inst->fmt_out->pixfmt; > params.hfi_color_fmt = to_hfi_raw_fmt(inst->fmt_cap->pixfmt); > params.dec.max_mbs_per_frame = mbs_per_frame_max(inst); > params.dec.buffer_size_limit = 0; > params.dec.is_secondary_output = > inst->opb_buftype == HFI_BUFFER_OUTPUT2; > + if (params.dec.is_secondary_output) > + params.hfi_dpb_color_fmt = inst->dpb_fmt; > params.dec.is_interlaced = > inst->pic_struct != HFI_INTERLACE_FRAME_PROGRESSIVE; > } else { > diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs.h b/drivers/media/platform/qcom/venus/hfi_plat_bufs.h > index 52a51a3..25e6074 100644 > --- a/drivers/media/platform/qcom/venus/hfi_plat_bufs.h > +++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs.h > @@ -12,8 +12,11 @@ > struct hfi_plat_buffers_params { > u32 width; > u32 height; > + u32 out_width; > + u32 out_height; > u32 codec; > u32 hfi_color_fmt; > + u32 hfi_dpb_color_fmt; > enum hfi_version version; > u32 num_vpp_pipes; > union { > diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c > index ea25c45..3855b04 100644 > --- a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c > +++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c > @@ -1185,6 +1185,7 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype, > enum hfi_version version = params->version; > u32 codec = params->codec; > u32 width = params->width, height = params->height, out_min_count; > + u32 out_width = params->out_width, out_height = params->out_height; > struct dec_bufsize_ops *dec_ops; > bool is_secondary_output = params->dec.is_secondary_output; > bool is_interlaced = params->dec.is_interlaced; > @@ -1235,7 +1236,12 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype, > bufreq->count_min = out_min_count; > bufreq->size = > venus_helper_get_framesz_raw(params->hfi_color_fmt, > - width, height); > + out_width, out_height); > + if (buftype == HFI_BUFFER_OUTPUT && > + params->dec.is_secondary_output) > + bufreq->size = > + venus_helper_get_framesz_raw(params->hfi_dpb_color_fmt, > + out_width, out_height); > } else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH(version)) { > bufreq->size = dec_ops->scratch(width, height, is_interlaced); > } else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH_1(version)) {
On 4.05.2023 19:29, Konrad Dybcio wrote: > > > On 4.05.2023 12:36, Dikshita Agarwal wrote: >> Use dpb color format, width and height of output port >> for calculating buffer size of dpb buffers. >> >> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> >> --- > Looks sane but I'm not exactly an expert on this > > Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > Konrad >> drivers/media/platform/qcom/venus/helpers.c | 4 ++++ >> drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++ >> drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++- >> 3 files changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c >> index 5946def..4ad6232 100644 >> --- a/drivers/media/platform/qcom/venus/helpers.c >> +++ b/drivers/media/platform/qcom/venus/helpers.c >> @@ -641,12 +641,16 @@ static int platform_get_bufreq(struct venus_inst *inst, u32 buftype, >> if (is_dec) { >> params.width = inst->width; >> params.height = inst->height; >> + params.out_width = inst->out_width; >> + params.out_height = inst->out_height; >> params.codec = inst->fmt_out->pixfmt; >> params.hfi_color_fmt = to_hfi_raw_fmt(inst->fmt_cap->pixfmt); >> params.dec.max_mbs_per_frame = mbs_per_frame_max(inst); >> params.dec.buffer_size_limit = 0; >> params.dec.is_secondary_output = >> inst->opb_buftype == HFI_BUFFER_OUTPUT2; >> + if (params.dec.is_secondary_output) >> + params.hfi_dpb_color_fmt = inst->dpb_fmt; >> params.dec.is_interlaced = >> inst->pic_struct != HFI_INTERLACE_FRAME_PROGRESSIVE; >> } else { >> diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs.h b/drivers/media/platform/qcom/venus/hfi_plat_bufs.h >> index 52a51a3..25e6074 100644 >> --- a/drivers/media/platform/qcom/venus/hfi_plat_bufs.h >> +++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs.h >> @@ -12,8 +12,11 @@ >> struct hfi_plat_buffers_params { >> u32 width; >> u32 height; >> + u32 out_width; >> + u32 out_height; >> u32 codec; >> u32 hfi_color_fmt; >> + u32 hfi_dpb_color_fmt; >> enum hfi_version version; >> u32 num_vpp_pipes; >> union { >> diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c >> index ea25c45..3855b04 100644 >> --- a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c >> +++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c >> @@ -1185,6 +1185,7 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype, >> enum hfi_version version = params->version; >> u32 codec = params->codec; >> u32 width = params->width, height = params->height, out_min_count; >> + u32 out_width = params->out_width, out_height = params->out_height; >> struct dec_bufsize_ops *dec_ops; >> bool is_secondary_output = params->dec.is_secondary_output; >> bool is_interlaced = params->dec.is_interlaced; >> @@ -1235,7 +1236,12 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype, >> bufreq->count_min = out_min_count; >> bufreq->size = >> venus_helper_get_framesz_raw(params->hfi_color_fmt, >> - width, height); >> + out_width, out_height); >> + if (buftype == HFI_BUFFER_OUTPUT && >> + params->dec.is_secondary_output) I suppose this line could be unbroken Konrad >> + bufreq->size = >> + venus_helper_get_framesz_raw(params->hfi_dpb_color_fmt, >> + out_width, out_height); >> } else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH(version)) { >> bufreq->size = dec_ops->scratch(width, height, is_interlaced); >> } else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH_1(version)) {
On 4.05.2023 12:36, Dikshita Agarwal wrote: > Set opb format to TP10_UWC and dpb to client set format > when bit depth change to 10 bit is detecting by firmware. > > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- > drivers/media/platform/qcom/venus/helpers.c | 18 ++++++++++++++++++ > drivers/media/platform/qcom/venus/vdec.c | 5 ++++- > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c > index 4ad6232..4f48ebd 100644 > --- a/drivers/media/platform/qcom/venus/helpers.c > +++ b/drivers/media/platform/qcom/venus/helpers.c > @@ -1770,6 +1770,24 @@ int venus_helper_get_out_fmts(struct venus_inst *inst, u32 v4l2_fmt, > if (!caps) > return -EINVAL; > > + if (inst->bit_depth == VIDC_BITDEPTH_10 && > + inst->session_type == VIDC_SESSION_TYPE_DEC) { This could be unbroken > + found_ubwc = find_fmt_from_caps(caps, HFI_BUFFER_OUTPUT, > + HFI_COLOR_FORMAT_YUV420_TP10_UBWC); > + found = find_fmt_from_caps(caps, HFI_BUFFER_OUTPUT2, > + fmt); This could be unbroken Otherwise I think this lgtm Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > + if (found_ubwc && found) { > + /* > + * Hard-code DPB buffers to be 10bit UBWC > + * until V4L2 is able to expose compressed/tiled > + * formats to applications. > + */ > + *out_fmt = HFI_COLOR_FORMAT_YUV420_TP10_UBWC; > + *out2_fmt = fmt; > + return 0; > + } > + } > + > if (ubwc) { > ubwc_fmt = fmt | HFI_COLOR_FORMAT_UBWC_BASE; > found_ubwc = find_fmt_from_caps(caps, HFI_BUFFER_OUTPUT, > diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c > index 687d62e..69f7f6e 100644 > --- a/drivers/media/platform/qcom/venus/vdec.c > +++ b/drivers/media/platform/qcom/venus/vdec.c > @@ -701,6 +701,9 @@ static int vdec_set_work_route(struct venus_inst *inst) > } > > #define is_ubwc_fmt(fmt) (!!((fmt) & HFI_COLOR_FORMAT_UBWC_BASE)) > +#define is_10bit_ubwc_fmt(fmt) (!!((fmt) & HFI_COLOR_FORMAT_10_BIT_BASE & \ > + HFI_COLOR_FORMAT_UBWC_BASE)) > + > > static int vdec_output_conf(struct venus_inst *inst) > { > @@ -748,7 +751,7 @@ static int vdec_output_conf(struct venus_inst *inst) > inst->opb_fmt = out2_fmt; > inst->dpb_buftype = HFI_BUFFER_OUTPUT; > inst->dpb_fmt = out_fmt; > - } else if (is_ubwc_fmt(out2_fmt)) { > + } else if (is_ubwc_fmt(out2_fmt) || is_10bit_ubwc_fmt(out_fmt)) { > inst->opb_buftype = HFI_BUFFER_OUTPUT; > inst->opb_fmt = out_fmt; > inst->dpb_buftype = HFI_BUFFER_OUTPUT2;
This series includes the changes to - add V4L2_PIX_FMT_P010 as supported decoder format. - consider dpb color format while calculating buffer size for dpb buffers. - update dpb and opb color format when bit depth changes is detected, also update preferred color format to P010 in this case. With this series, divided the previous version [1] into multiple patches as suggested in review comments. [1] https://patchwork.linuxtv.org/project/linux-media/list/?series=10376 Dikshita Agarwal (4): venus: add support for V4L2_PIX_FMT_P010 color format venus: update calculation for dpb buffers venus: add handling of bit depth change from firmwar venus: return P010 as preferred format for 10 bit decode drivers/media/platform/qcom/venus/helpers.c | 24 ++++++++++++++++++++++ drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++ .../media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++- drivers/media/platform/qcom/venus/vdec.c | 16 +++++++++++++-- 4 files changed, 48 insertions(+), 3 deletions(-)