> -----Original Message----- > From: Pekka Paalanen <pekka.paala...@haloniitty.fi> > Sent: Thursday, April 17, 2025 12:48 PM > To: Shankar, Uma <uma.shan...@intel.com> > Cc: Murthy, Arun R <arun.r.mur...@intel.com>; intel...@lists.freedesktop.org; > intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Kandpal, > Suraj > <suraj.kand...@intel.com>; harry.wentl...@amd.com; alex.h...@amd.com; > Vetter, Simona <simona.vet...@intel.com>; airl...@gmail.com > Subject: Re: [PATCH v8 01/14] drm: Define histogram structures exposed to user > > On Thu, 17 Apr 2025 06:31:21 +0000 > "Shankar, Uma" <uma.shan...@intel.com> wrote: > > > > -----Original Message----- > > > From: Intel-xe <intel-xe-boun...@lists.freedesktop.org> On Behalf Of > > > Murthy, Arun R > > > Sent: Friday, March 28, 2025 10:36 AM > > > To: Pekka Paalanen <pekka.paala...@haloniitty.fi> > > > Cc: intel...@lists.freedesktop.org; intel-...@lists.freedesktop.org; > > > dri- de...@lists.freedesktop.org; Kandpal, Suraj > > > <suraj.kand...@intel.com>; harry.wentl...@amd.com; > > > alex.h...@amd.com; Vetter, Simona <simona.vet...@intel.com>; > > > airl...@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.mur...@intel.com> wrote: > > > > > > > > > > On Wed, 19 Mar 2025 12:08:15 +0000 "Murthy, Arun R" > > > > > > <arun.r.mur...@intel.com> wrote: > > > > > > > > > > > > > > On Mon, 3 Mar 2025 13:23:42 +0530 "Murthy, Arun R" > > > > > > > > <arun.r.mur...@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.mur...@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.h...@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? Hi Pekka, I meant the option 2 as an interim approach to get the visibility to userspace of histogram blocks which can be extended further once we get the full crtc color pipeline in place. Regards, Uma Shankar > > 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 > >