Re: [PATCH v1 10/14] drm/msm/disp/dpu: add supports of DSC encoder v1.2 engine

From: Abhinav Kumar
Date: Mon Jan 30 2023 - 16:22:21 EST


Hi Marijn

On 1/30/2023 12:16 PM, Marijn Suijten wrote:
On 2023-01-24 15:52:46, Kuogee Hsieh wrote:

<snip>

If only replying to a small chunk somewhere in the middle of a diff
and/or large review, please cut out unnecessary bits to make your reply
easier to find :)

+ data = (dsc->flatness_min_qp & 0x1f);
+ data |= (dsc->flatness_max_qp & 0x1f) << 5;
+ data |= (dsc_info->det_thresh_flatness & 0xff) << 10;
dpu_hw_dsc.c computes this on the fly. After removing that, and
using initial_lines from the function parameters, only
dsc_info->num_active_ss_per_enc remains. Do you really need that
msm_display_dsc_info struct here, do you need it at all?

I ported these code from our down stream code base.

I make it work first, then clean it up will follow.

I submit it for review since it looks like you guy like to have code sooner.

Correct, I was looking forward to these patches albeit complete with the
promised DSI support from Jessica, which still seems to be pending.


DSI support is still being worked upon.

I dont think we promised DSC 1.2 will come with DSI together in the same series. It was always going to be DSC 1.2 + DP followed by another series from Jessica for DSI.

Lets set the expectations right.

Thanks

Abhinav
When sending patches to that extent, with the intent of getting quick
turnaround but knowing that they are not ready for prime time yet (or
were they, based on your "submit it for review" mention? Don't you mean
testing?), please annotate the series with an RFC tag accompanied with a
description what still needs to be done and why. That would have saved
a great deal of comments and review.

yes, eliminate msm_display_dsc_info is my next target and hope it can be
done.

Thank you. Again, if that was the intent from the get-go, that's
perfect material to put in an RFC series' cover letter.

- Marijn