Re: [PATCH 0/2] DPU1 GC1.8 wiring-up

[Marijn Suijten]

On 2023-04-20 21:01:04, Dmitry Baryshkov wrote:
> On 20/04/2023 04:36, Konrad Dybcio wrote:
> > 
> > 
> > On 20.04.2023 03:28, Abhinav Kumar wrote:
> >>
> >>
> >> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
> >>>
> >>>
> >>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
> >>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
> >>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
> >>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
> >>>>> more, but none of them are supported upstream.
> >>>>>
> >>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
> >>>>
> >>>> Does this mean that we will see gamma_lut support soon?
> >>> No promises, my plate is not even full, it's beyond overflowing! :P
> >>>
> >>> Konrad
> >>
> >> So I think I wrote about this before during the catalog rework/fixes that the gc registers are not written to / programmed.
> >>
> >> If thats not done, is there any benefit to this series?
> > Completeness and preparation for the code itself, if nothing else?
> 
> The usual problem is that if something is not put to use, it quickly 
> rots or becomes misused for newer platforms. We have seen this with the 
> some of DPU features.
> 
> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we 
> have three options:
> - drop the unused GC from msm8998_sblk.
> - keep things as is, single unused GC entry
> - fill all the sblk with the correct information in hope that it stays 
> correct
> 
> Each of these options has its own drawbacks. I have slight bias towards 
> the last option, to have the information in place (as long as it is 
> accurate).

Normally I'm all for rigorously and completely defining the hardware,
porting the entire downstream DT in one go while looking at it anyway.
(And it leaves less room for error when looking at DT properties while
 having no clue where they should end up in the catalog, or why they
 wouldn't be there)

In this case though, as you say, it's unused so there's no way to test
and validate anything, especially future changes we **might** make to
the looks and layout of the catalog.

What's worse, this series shows zero efforts towards at the very least
explaining that GC is the Gamma Correction block, what the benefits are
in defining/having it, and that it is currently not used by the DSPP
driver block at all.  That's my major reason for NAK'ing this.

- Marijn