Re: [PATCH v8 01/14] drm: Define histogram structures exposed to user

[Pekka Paalanen <pekka.paalanen@haloniitty.fi>]

[-- Attachment #1: Type: text/plain, Size: 5728 bytes --]

On Thu, 17 Apr 2025 06:31:21 +0000
"Shankar, Uma" <uma.shankar@intel.com> wrote:

> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Murthy,
> > Arun R
> > Sent: Friday, March 28, 2025 10:36 AM
> > To: Pekka Paalanen <pekka.paalanen@haloniitty.fi>
> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; dri-
> > devel@lists.freedesktop.org; Kandpal, Suraj <suraj.kandpal@intel.com>;
> > harry.wentland@amd.com; alex.hung@amd.com; Vetter, Simona
> > <simona.vetter@intel.com>; airlied@gmail.com
> > Subject: RE: [PATCH v8 01/14] drm: Define histogram structures exposed to user
> >   
> > > On Wed, 26 Mar 2025 06:03:27 +0000
> > > "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
> > >  
> > > > > On Wed, 19 Mar 2025 12:08:15 +0000 "Murthy, Arun R"
> > > > > <arun.r.murthy@intel.com> wrote:
> > > > >  
> > > > > > > On Mon, 3 Mar 2025 13:23:42 +0530 "Murthy, Arun R"
> > > > > > > <arun.r.murthy@intel.com> wrote:
> > > > > > >  
> > > > > > > > On 20-02-2025 21:20, Pekka Paalanen wrote:  
> > > > > > > > > On Wed, 19 Feb 2025 09:28:51 +0530 "Murthy, Arun R"
> > > > > > > > > <arun.r.murthy@intel.com> wrote:  
> > > > >
> > > > > ...
> > > > >  
> > > > > > > > Hi Pekka,
> > > > > > > > Sorry got getting back late on this.
> > > > > > > > I totally agree that the UAPI should not be any hardware
> > > > > > > > specific and have taken care to get rid of such if any.
> > > > > > > > Here we are just exposing the histogram data and what point
> > > > > > > > is the histogram generated is out of the scope.  
> > > > > > >
> > > > > > > It's not out of scope. Understanding exactly at what point the
> > > > > > > histogram is generated in the KMS pixel pipeline is paramount
> > > > > > > to being able to use the results correctly - how it relates to
> > > > > > > the  
> > > framebuffers'  
> > > > > > > contents and KMS pixel pipeline configuration.
> > > > > > >  
> > > > > >
> > > > > > While working around this comment, it looks to be quite
> > > > > > challenging to address this comment and agree that this will
> > > > > > have to be addressed and is important for the user to know at
> > > > > > which point in the pixel pipeline configuration the histogram is generated.
> > > > > > I can think of 2 options on addressing this.
> > > > > >
> > > > > > 1.  Have this documented in the driver. Since this can vary on
> > > > > > each vendor hardware, can have this documented in the drivers or
> > > > > > ReST  
> > > document.  
> > > > > >  
> > > > >
> > > > > Hi Arun,
> > > > >
> > > > > this is not acceptable in KMS, because it requires userspace to
> > > > > have code that depends on the hardware revision or identity. When
> > > > > new hardware appears, it will not be enough to update the drivers,
> > > > > one will also need to update userspace. There currently is no
> > > > > userspace "standard KMS library" to abstract all drivers further,
> > > > > like there is libcamera  
> > > and Mesa.  
> > > > >  
> > > > > > 2. Maybe have a bitmapping like we have it for histogram_mode.
> > > > > > Define user readable macros for that.
> > > > > > Ex: CC1_DEGAMMA_HIST_CC2
> > > > > > In this case histogram is between the two color conversion
> > > > > > hardware block in the pixel pipeline.
> > > > > > This macro will have to be defined on need basis and define a
> > > > > > u32 variable for this bit manipulation.  
> > > > >
> > > > > This one still has problems in that some hardware may not have all
> > > > > the non- HIST elements or not in the same order. It's a better
> > > > > abstraction than option 1, but it's still weak.
> > > > >
> > > > > I can see one solid solution, but it won't be usable for quite
> > > > > some time I
> > > > > suppose:
> > > > >
> > > > > https://lore.kernel.org/dri-devel/20241220043410.416867-5-
> > > > > alex.hung@amd.com/
> > > > >
> > > > > The current work on the color pipelines UAPI is concentrated on
> > > > > the per-plane operations. The idea is that once those are hashed
> > > > > out, the same design is applied to CRTCs as well, deprecating all
> > > > > existing CRTC color processing properties. A histogram processing
> > > > > element could be exposed as a colorop object, and its position in
> > > > > a CRTC color pipeline represents the point where the histogram is collected.
> > > > >
> > > > > That would be the best possible UAPI design on current knowledge
> > > > > in my opinion.
> > > > >  
> > > > Yes this would be the best design, but looking into the timeline for
> > > > this CRTC color pipeline can we proceed with this as in interim solution.
> > > > Once we have the CRTC color pipeline in drm, will rebase this
> > > > histogram to make use of the pipeline.
> > > > We can also take up the crtc color pipeline and will also start
> > > > contributing to get things faster but since there is not timeline
> > > > defined for that activity, would it be viable to go ahead with
> > > > option2 as in  
> > > interim solution?
> > >
> > > Hi Arun,
> > >
> > > I think that's something the DRM maintainers should chime in on.  
> 
> As a first step, I think we can expose the Histogram through the property.
> We can later hook this into the crtc color pipeline once we implement it.

Do you mean upstreamed as well?

How is that different from the original proposal here that I raised
concerns about?


Thanks,
pq

> A userspace implementation showing end to end benefit of the feature and
> usecase would be needed. Hope this is ok and no strong objection to this
> approach.
> 
> Regards,
> Uma Shankar
> 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]