mbox series

[PATCHv10,00/10] Display Global Histogram

Message ID 20241209162504.2146697-1-arun.r.murthy@intel.com (mailing list archive)
Headers show
Series Display Global Histogram | expand

Message

Murthy, Arun R Dec. 9, 2024, 4:24 p.m. UTC
Display histogram is a hardware functionality where a statistics for 'x'
number of frames is generated to form a histogram data. This is notified
to the user via histogram event. Compositor will then upon sensing the
histogram event will read the histogram data from KMD via crtc property.
A library can be developed to take this generated histogram as an
input and apply some algorithm to generate an Image EnhancemenT(IET).
This is further fed back to the KMD via crtc property. KMD will use this
IET as a multiplicand factor to multiply with the incoming pixels at the
end of the pipe which is then pushed onto the display.

One such library Global Histogram Enhancement(GHE) will take the histogram
as input and applied the algorithm to enhance the density and then
return the enhanced factor. This library can be located @
https://github.com/intel/ghe

The corresponding mutter changes to enable/disable histogram, read the
histogram data, communicate with the library and write the enhanced data
back to the KMD is also pushed for review at https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873
and https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873/diffs?commit_id=270808ca7c8be48513553d95b4a47541f5d40206
The IGT changes for validating the histogram event and reading the
histogram is also pushed for review at https://patchwork.freedesktop.org/series/135789/

Test-with: 20240705091333.328322-1-mohammed.thasleem@intel.com

Arun R Murthy (10):
  drm/crtc: Add histogram properties
  drm/crtc: Expose API to create drm crtc property for histogram
  drm/i915/histogram: Define registers for histogram
  drm/i915/histogram: Add support for histogram
  drm/xe: Add histogram support to Xe builds
  drm/i915/histogram: histogram interrupt handling
  drm/i915/display: handle drm-crtc histogram property updates
  drm/i915/histogram: histogram delay counter doesnt reset
  drm/i915/histogram: Histogram changes for Display 20+
  drm/i915/histogram: Enable pipe dithering

 drivers/gpu/drm/drm_atomic_state_helper.c     |   6 +
 drivers/gpu/drm/drm_atomic_uapi.c             |  17 +
 drivers/gpu/drm/drm_crtc.c                    |  30 ++
 drivers/gpu/drm/i915/Makefile                 |   1 +
 drivers/gpu/drm/i915/display/intel_atomic.c   |   1 +
 drivers/gpu/drm/i915/display/intel_crtc.c     |   7 +
 drivers/gpu/drm/i915/display/intel_display.c  |  17 +
 .../gpu/drm/i915/display/intel_display_irq.c  |   6 +-
 .../drm/i915/display/intel_display_types.h    |   4 +
 .../gpu/drm/i915/display/intel_histogram.c    | 380 ++++++++++++++++++
 .../gpu/drm/i915/display/intel_histogram.h    |  40 ++
 .../drm/i915/display/intel_histogram_regs.h   |  75 ++++
 drivers/gpu/drm/i915/i915_reg.h               |   5 +-
 drivers/gpu/drm/xe/Makefile                   |   1 +
 include/drm/drm_crtc.h                        |  49 +++
 include/uapi/drm/drm_mode.h                   |  11 +
 16 files changed, 647 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_histogram.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_histogram.h
 create mode 100644 drivers/gpu/drm/i915/display/intel_histogram_regs.h

Comments

Matt Roper Dec. 9, 2024, 4:57 p.m. UTC | #1
On Mon, Dec 09, 2024 at 09:54:54PM +0530, Arun R Murthy wrote:
> Display histogram is a hardware functionality where a statistics for 'x'
> number of frames is generated to form a histogram data. This is notified
> to the user via histogram event. Compositor will then upon sensing the
> histogram event will read the histogram data from KMD via crtc property.
> A library can be developed to take this generated histogram as an
> input and apply some algorithm to generate an Image EnhancemenT(IET).
> This is further fed back to the KMD via crtc property. KMD will use this
> IET as a multiplicand factor to multiply with the incoming pixels at the
> end of the pipe which is then pushed onto the display.
> 
> One such library Global Histogram Enhancement(GHE) will take the histogram
> as input and applied the algorithm to enhance the density and then
> return the enhanced factor. This library can be located @
> https://github.com/intel/ghe
> 
> The corresponding mutter changes to enable/disable histogram, read the
> histogram data, communicate with the library and write the enhanced data
> back to the KMD is also pushed for review at https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873
> and https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873/diffs?commit_id=270808ca7c8be48513553d95b4a47541f5d40206
> The IGT changes for validating the histogram event and reading the
> histogram is also pushed for review at https://patchwork.freedesktop.org/series/135789/

I think other people have already asked this on previous postings of
these patches, but please don't try to manually hack the version numbers
within a series.  What you just posted has "PATCHv10" on the cover
letter, "PATCHv2" on one patch, "PATCHv3" on three patches, and the rest
are unversioned "PATCH."  The general expectation these days is that
versioning in the subject applies to the series as a whole, not the
individual patches, so this causes a lot of confusion.  Posting like you
did here also wrecks havoc on a lot of the tools people use to manage
and compare series like the "b4" tool.

When generating and sending a new series, you should just do something
like "git format-patch -v10 ..." so that the proper "v10" numbering is
automatically applied to all the patches and we don't wind up with this
strange jumble.


Matt

> 
> Test-with: 20240705091333.328322-1-mohammed.thasleem@intel.com
> 
> Arun R Murthy (10):
>   drm/crtc: Add histogram properties
>   drm/crtc: Expose API to create drm crtc property for histogram
>   drm/i915/histogram: Define registers for histogram
>   drm/i915/histogram: Add support for histogram
>   drm/xe: Add histogram support to Xe builds
>   drm/i915/histogram: histogram interrupt handling
>   drm/i915/display: handle drm-crtc histogram property updates
>   drm/i915/histogram: histogram delay counter doesnt reset
>   drm/i915/histogram: Histogram changes for Display 20+
>   drm/i915/histogram: Enable pipe dithering
> 
>  drivers/gpu/drm/drm_atomic_state_helper.c     |   6 +
>  drivers/gpu/drm/drm_atomic_uapi.c             |  17 +
>  drivers/gpu/drm/drm_crtc.c                    |  30 ++
>  drivers/gpu/drm/i915/Makefile                 |   1 +
>  drivers/gpu/drm/i915/display/intel_atomic.c   |   1 +
>  drivers/gpu/drm/i915/display/intel_crtc.c     |   7 +
>  drivers/gpu/drm/i915/display/intel_display.c  |  17 +
>  .../gpu/drm/i915/display/intel_display_irq.c  |   6 +-
>  .../drm/i915/display/intel_display_types.h    |   4 +
>  .../gpu/drm/i915/display/intel_histogram.c    | 380 ++++++++++++++++++
>  .../gpu/drm/i915/display/intel_histogram.h    |  40 ++
>  .../drm/i915/display/intel_histogram_regs.h   |  75 ++++
>  drivers/gpu/drm/i915/i915_reg.h               |   5 +-
>  drivers/gpu/drm/xe/Makefile                   |   1 +
>  include/drm/drm_crtc.h                        |  49 +++
>  include/uapi/drm/drm_mode.h                   |  11 +
>  16 files changed, 647 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_histogram.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_histogram.h
>  create mode 100644 drivers/gpu/drm/i915/display/intel_histogram_regs.h
> 
> -- 
> 2.25.1
>
Raag Jadav Dec. 9, 2024, 5:43 p.m. UTC | #2
On Mon, Dec 09, 2024 at 08:57:56AM -0800, Matt Roper wrote:
> On Mon, Dec 09, 2024 at 09:54:54PM +0530, Arun R Murthy wrote:
> > Display histogram is a hardware functionality where a statistics for 'x'
> > number of frames is generated to form a histogram data. This is notified
> > to the user via histogram event. Compositor will then upon sensing the
> > histogram event will read the histogram data from KMD via crtc property.
> > A library can be developed to take this generated histogram as an
> > input and apply some algorithm to generate an Image EnhancemenT(IET).
> > This is further fed back to the KMD via crtc property. KMD will use this
> > IET as a multiplicand factor to multiply with the incoming pixels at the
> > end of the pipe which is then pushed onto the display.
> > 
> > One such library Global Histogram Enhancement(GHE) will take the histogram
> > as input and applied the algorithm to enhance the density and then
> > return the enhanced factor. This library can be located @
> > https://github.com/intel/ghe
> > 
> > The corresponding mutter changes to enable/disable histogram, read the
> > histogram data, communicate with the library and write the enhanced data
> > back to the KMD is also pushed for review at https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873
> > and https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873/diffs?commit_id=270808ca7c8be48513553d95b4a47541f5d40206
> > The IGT changes for validating the histogram event and reading the
> > histogram is also pushed for review at https://patchwork.freedesktop.org/series/135789/
> 
> I think other people have already asked this on previous postings of
> these patches, but please don't try to manually hack the version numbers
> within a series.  What you just posted has "PATCHv10" on the cover
> letter, "PATCHv2" on one patch, "PATCHv3" on three patches, and the rest
> are unversioned "PATCH."  The general expectation these days is that
> versioning in the subject applies to the series as a whole, not the
> individual patches, so this causes a lot of confusion.  Posting like you
> did here also wrecks havoc on a lot of the tools people use to manage
> and compare series like the "b4" tool.
> 
> When generating and sending a new series, you should just do something
> like "git format-patch -v10 ..." so that the proper "v10" numbering is
> automatically applied to all the patches and we don't wind up with this
> strange jumble.

Isn't that the starting point?

https://kernelnewbies.org/FirstKernelPatch -> "Versioning patchsets"

Raag
Matt Roper Dec. 9, 2024, 6:18 p.m. UTC | #3
On Mon, Dec 09, 2024 at 07:43:55PM +0200, Raag Jadav wrote:
> On Mon, Dec 09, 2024 at 08:57:56AM -0800, Matt Roper wrote:
> > On Mon, Dec 09, 2024 at 09:54:54PM +0530, Arun R Murthy wrote:
> > > Display histogram is a hardware functionality where a statistics for 'x'
> > > number of frames is generated to form a histogram data. This is notified
> > > to the user via histogram event. Compositor will then upon sensing the
> > > histogram event will read the histogram data from KMD via crtc property.
> > > A library can be developed to take this generated histogram as an
> > > input and apply some algorithm to generate an Image EnhancemenT(IET).
> > > This is further fed back to the KMD via crtc property. KMD will use this
> > > IET as a multiplicand factor to multiply with the incoming pixels at the
> > > end of the pipe which is then pushed onto the display.
> > > 
> > > One such library Global Histogram Enhancement(GHE) will take the histogram
> > > as input and applied the algorithm to enhance the density and then
> > > return the enhanced factor. This library can be located @
> > > https://github.com/intel/ghe
> > > 
> > > The corresponding mutter changes to enable/disable histogram, read the
> > > histogram data, communicate with the library and write the enhanced data
> > > back to the KMD is also pushed for review at https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873
> > > and https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873/diffs?commit_id=270808ca7c8be48513553d95b4a47541f5d40206
> > > The IGT changes for validating the histogram event and reading the
> > > histogram is also pushed for review at https://patchwork.freedesktop.org/series/135789/
> > 
> > I think other people have already asked this on previous postings of
> > these patches, but please don't try to manually hack the version numbers
> > within a series.  What you just posted has "PATCHv10" on the cover
> > letter, "PATCHv2" on one patch, "PATCHv3" on three patches, and the rest
> > are unversioned "PATCH."  The general expectation these days is that
> > versioning in the subject applies to the series as a whole, not the
> > individual patches, so this causes a lot of confusion.  Posting like you
> > did here also wrecks havoc on a lot of the tools people use to manage
> > and compare series like the "b4" tool.
> > 
> > When generating and sending a new series, you should just do something
> > like "git format-patch -v10 ..." so that the proper "v10" numbering is
> > automatically applied to all the patches and we don't wind up with this
> > strange jumble.
> 
> Isn't that the starting point?
> 
> https://kernelnewbies.org/FirstKernelPatch -> "Versioning patchsets"

That section is explaining that the descriptive changelogs for updated
series can either be placed in the cover letter or in the individual
patches; I don't see anything about going back and editing the "[PATCH"
prefix of the subject line that was generated.  You generate the new
copy of all the patches (and the cover letter) with one execution of
"git format-patch," so whatever version number is generated should be
consistent and correct as a series-wide version without editing.  Also
note that even though that site suggests using "--subject-prefix" to
specify the version number, these days git's format-patch has a "-v"
option that's a bit more convenient for this purpose since it also
updates the filename of the patches generated and knows how to combine
the version number with other subject prefix rules for projects that use
them (e.g., IGT where patches use [PATCH i-g-t]).

Although sites like the one you linked can help with getting started,
it's worth noting that different kernel subsystems sometimes use
slightly different conventions so it's best to always check how things
are done in the area you're submitting patches to.  For example, unlike
many (most?) parts of the kernel, in the DRM subsystem we generally
prefer to place the per-patch changelogs directly into the commit
message rather than supplying them below the "---" line where they'd be
dropped when the patch is applied (i.e., our DRM convention runs counter
to what's stated earlier on the page you linked).  When in doubt it's
always best to read through other series on the mailing list to get a
sense of how other developers are crafting their series.


Matt

> 
> Raag
Raag Jadav Dec. 9, 2024, 6:57 p.m. UTC | #4
On Mon, Dec 09, 2024 at 10:18:42AM -0800, Matt Roper wrote:
> On Mon, Dec 09, 2024 at 07:43:55PM +0200, Raag Jadav wrote:
> > On Mon, Dec 09, 2024 at 08:57:56AM -0800, Matt Roper wrote:
> > > On Mon, Dec 09, 2024 at 09:54:54PM +0530, Arun R Murthy wrote:
> > > > Display histogram is a hardware functionality where a statistics for 'x'
> > > > number of frames is generated to form a histogram data. This is notified
> > > > to the user via histogram event. Compositor will then upon sensing the
> > > > histogram event will read the histogram data from KMD via crtc property.
> > > > A library can be developed to take this generated histogram as an
> > > > input and apply some algorithm to generate an Image EnhancemenT(IET).
> > > > This is further fed back to the KMD via crtc property. KMD will use this
> > > > IET as a multiplicand factor to multiply with the incoming pixels at the
> > > > end of the pipe which is then pushed onto the display.
> > > > 
> > > > One such library Global Histogram Enhancement(GHE) will take the histogram
> > > > as input and applied the algorithm to enhance the density and then
> > > > return the enhanced factor. This library can be located @
> > > > https://github.com/intel/ghe
> > > > 
> > > > The corresponding mutter changes to enable/disable histogram, read the
> > > > histogram data, communicate with the library and write the enhanced data
> > > > back to the KMD is also pushed for review at https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873
> > > > and https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873/diffs?commit_id=270808ca7c8be48513553d95b4a47541f5d40206
> > > > The IGT changes for validating the histogram event and reading the
> > > > histogram is also pushed for review at https://patchwork.freedesktop.org/series/135789/
> > > 
> > > I think other people have already asked this on previous postings of
> > > these patches, but please don't try to manually hack the version numbers
> > > within a series.  What you just posted has "PATCHv10" on the cover
> > > letter, "PATCHv2" on one patch, "PATCHv3" on three patches, and the rest
> > > are unversioned "PATCH."  The general expectation these days is that
> > > versioning in the subject applies to the series as a whole, not the
> > > individual patches, so this causes a lot of confusion.  Posting like you
> > > did here also wrecks havoc on a lot of the tools people use to manage
> > > and compare series like the "b4" tool.
> > > 
> > > When generating and sending a new series, you should just do something
> > > like "git format-patch -v10 ..." so that the proper "v10" numbering is
> > > automatically applied to all the patches and we don't wind up with this
> > > strange jumble.
> > 
> > Isn't that the starting point?
> > 
> > https://kernelnewbies.org/FirstKernelPatch -> "Versioning patchsets"
> 
> That section is explaining that the descriptive changelogs for updated
> series can either be placed in the cover letter or in the individual
> patches; I don't see anything about going back and editing the "[PATCH"
> prefix of the subject line that was generated.  You generate the new
> copy of all the patches (and the cover letter) with one execution of
> "git format-patch," so whatever version number is generated should be
> consistent and correct as a series-wide version without editing.  Also
> note that even though that site suggests using "--subject-prefix" to
> specify the version number, these days git's format-patch has a "-v"
> option that's a bit more convenient for this purpose since it also
> updates the filename of the patches generated and knows how to combine
> the version number with other subject prefix rules for projects that use
> them (e.g., IGT where patches use [PATCH i-g-t]).
> 
> Although sites like the one you linked can help with getting started,
> it's worth noting that different kernel subsystems sometimes use
> slightly different conventions so it's best to always check how things
> are done in the area you're submitting patches to.  For example, unlike
> many (most?) parts of the kernel, in the DRM subsystem we generally
> prefer to place the per-patch changelogs directly into the commit
> message rather than supplying them below the "---" line where they'd be
> dropped when the patch is applied (i.e., our DRM convention runs counter
> to what's stated earlier on the page you linked).  When in doubt it's
> always best to read through other series on the mailing list to get a
> sense of how other developers are crafting their series.

Which is why it's always useful to have some documentation up (especially
for the counter ones which are subsystem specific) which can serve as a
good starting point.

Raag
Raag Jadav Dec. 9, 2024, 8:17 p.m. UTC | #5
On Mon, Dec 09, 2024 at 10:18:42AM -0800, Matt Roper wrote:
> On Mon, Dec 09, 2024 at 07:43:55PM +0200, Raag Jadav wrote:
> > On Mon, Dec 09, 2024 at 08:57:56AM -0800, Matt Roper wrote:
> > > On Mon, Dec 09, 2024 at 09:54:54PM +0530, Arun R Murthy wrote:
> > > > Display histogram is a hardware functionality where a statistics for 'x'
> > > > number of frames is generated to form a histogram data. This is notified
> > > > to the user via histogram event. Compositor will then upon sensing the
> > > > histogram event will read the histogram data from KMD via crtc property.
> > > > A library can be developed to take this generated histogram as an
> > > > input and apply some algorithm to generate an Image EnhancemenT(IET).
> > > > This is further fed back to the KMD via crtc property. KMD will use this
> > > > IET as a multiplicand factor to multiply with the incoming pixels at the
> > > > end of the pipe which is then pushed onto the display.
> > > > 
> > > > One such library Global Histogram Enhancement(GHE) will take the histogram
> > > > as input and applied the algorithm to enhance the density and then
> > > > return the enhanced factor. This library can be located @
> > > > https://github.com/intel/ghe
> > > > 
> > > > The corresponding mutter changes to enable/disable histogram, read the
> > > > histogram data, communicate with the library and write the enhanced data
> > > > back to the KMD is also pushed for review at https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873
> > > > and https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873/diffs?commit_id=270808ca7c8be48513553d95b4a47541f5d40206
> > > > The IGT changes for validating the histogram event and reading the
> > > > histogram is also pushed for review at https://patchwork.freedesktop.org/series/135789/
> > > 
> > > I think other people have already asked this on previous postings of
> > > these patches, but please don't try to manually hack the version numbers
> > > within a series.  What you just posted has "PATCHv10" on the cover
> > > letter, "PATCHv2" on one patch, "PATCHv3" on three patches, and the rest
> > > are unversioned "PATCH."  The general expectation these days is that
> > > versioning in the subject applies to the series as a whole, not the
> > > individual patches, so this causes a lot of confusion.  Posting like you
> > > did here also wrecks havoc on a lot of the tools people use to manage
> > > and compare series like the "b4" tool.
> > > 
> > > When generating and sending a new series, you should just do something
> > > like "git format-patch -v10 ..." so that the proper "v10" numbering is
> > > automatically applied to all the patches and we don't wind up with this
> > > strange jumble.
> > 
> > Isn't that the starting point?
> > 
> > https://kernelnewbies.org/FirstKernelPatch -> "Versioning patchsets"
> 
> That section is explaining that the descriptive changelogs for updated
> series can either be placed in the cover letter or in the individual
> patches; I don't see anything about going back and editing the "[PATCH"
> prefix of the subject line that was generated.  You generate the new
> copy of all the patches (and the cover letter) with one execution of
> "git format-patch," so whatever version number is generated should be
> consistent and correct as a series-wide version without editing.

Yes, that's what I meant above (which is explained with an example).

Raag
Murthy, Arun R Dec. 10, 2024, 8:43 a.m. UTC | #6
> On Mon, Dec 09, 2024 at 09:54:54PM +0530, Arun R Murthy wrote:
> > Display histogram is a hardware functionality where a statistics for 'x'
> > number of frames is generated to form a histogram data. This is
> > notified to the user via histogram event. Compositor will then upon
> > sensing the histogram event will read the histogram data from KMD via crtc
> property.
> > A library can be developed to take this generated histogram as an
> > input and apply some algorithm to generate an Image EnhancemenT(IET).
> > This is further fed back to the KMD via crtc property. KMD will use
> > this IET as a multiplicand factor to multiply with the incoming pixels
> > at the end of the pipe which is then pushed onto the display.
> >
> > One such library Global Histogram Enhancement(GHE) will take the
> > histogram as input and applied the algorithm to enhance the density
> > and then return the enhanced factor. This library can be located @
> > https://github.com/intel/ghe
> >
> > The corresponding mutter changes to enable/disable histogram, read the
> > histogram data, communicate with the library and write the enhanced
> > data back to the KMD is also pushed for review at
> > https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3873
> > and
> > https://gitlab.gnome.org/GNOME/mutter/-
> /merge_requests/3873/diffs?comm
> > it_id=270808ca7c8be48513553d95b4a47541f5d40206
> > The IGT changes for validating the histogram event and reading the
> > histogram is also pushed for review at
> > https://patchwork.freedesktop.org/series/135789/
> 
> I think other people have already asked this on previous postings of these
> patches, but please don't try to manually hack the version numbers within a
> series.  What you just posted has "PATCHv10" on the cover letter, "PATCHv2" on
> one patch, "PATCHv3" on three patches, and the rest are unversioned "PATCH."
> The general expectation these days is that versioning in the subject applies to
> the series as a whole, not the individual patches, so this causes a lot of
> confusion.  Posting like you did here also wrecks havoc on a lot of the tools
> people use to manage and compare series like the "b4" tool.
> 
> When generating and sending a new series, you should just do something like
> "git format-patch -v10 ..." so that the proper "v10" numbering is automatically
> applied to all the patches and we don't wind up with this strange jumble.
> 
> 
Will use b4 in future.

Thanks and Regards,
Arun R Murthy
--------------------