Age | Commit message (Collapse) | Author | Files | Lines |
|
struct intel_dsb can be an opaque type, hidden in intel_dsb.c. Make it
so. Reduce related includes while at it.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220908165702.973854-1-jani.nikula@intel.com
|
|
Move display color functions under drm_i915_private display sub-struct.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/36d6e9327582c94c1962c10ebd1581c3d15a5743.1661346845.git.jani.nikula@intel.com
|
|
The LUTs are a display feature so move the details into
the display portion of the device info.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220623130900.26078-10-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
|
|
Eliminate the PIPECONF RMWs from .comit_commit() so
that we can finally declare the whole vblank evade part
(and the noarm() part) of the pipe commit free of register
reads. Or at least I hope that's the last read...
Only the i9xx/ilk codepaths need this for now, but let's
add the same thing for hsw+ just in case we want to start
calling that during fastsets at some point (eg. to change
dithering settings/etc.).
Should open up the way to start experimenting with
different DSB usage approaches for pipe commits.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220413192607.27533-1-ville.syrjala@linux.intel.com
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
|
|
Spelling mistakes (triple letters) in comments.
Detected with the help of Coccinelle.
Signed-off-by: Julia Lawall <Julia.Lawall@inria.fr>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220521111145.81697-90-Julia.Lawall@inria.fr
|
|
Move most of the pipe+output CSC programming to the
.color_commit_noarm() hook which runs before vblank evasion.
Only PIPE_CSC_MODE (the arming register) needs to remain in
inside the critical section.
A test case that just updates the CTM in a loop produces
the following i915_update_info numbers on ilk (w/o lockdep):
old new
Updates: 10012 Updates: 10008
| |
1us |** 1us |**********
|************* |*************
4us |********* 4us |*
|* |**
16us | 16us |
| |
66us | 66us |
| |
262us | 262us |
| |
1ms | 1ms |
| |
4ms | 4ms |
| |
17ms | 17ms |
| |
Min update: 1345ns Min update: 1268ns
Max update: 16672ns Max update: 15656ns
Average update: 3914ns Average update: 2185ns
Overruns > 100us: 0 Overruns > 100us: 0
And here is tgl (forced to update both pipe CSC and
output CSC, and with lockdep enabled):
old new
Updates: 10012 Updates: 10012
| |
1us | 1us |
| |
4us |* 4us |**
|** |**********
16us |************* 16us |*************
|* |
66us | 66us |
| |
262us | 262us |
| |
1ms | 1ms |
| |
4ms | 4ms |
| |
17ms | 17ms |
| |
Min update: 5204ns Min update: 5176ns
Max update: 176038ns Max update: 186685ns
Average update: 23931ns Average update: 16654ns
Overruns > 250us: 0 Overruns > 250us: 0
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220224165103.15682-5-ville.syrjala@linux.intel.com
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
|
|
To reduce the amount of registers written during the vblank evade
critical section let's also split the .color_commit() hook to
noarm+arm pair. The noarm hook runs before the vblank evasion
with the arm hook staying inside the critical section.
Just the framework here, actually moving stuff out into the noarm
hook will follow.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220224165103.15682-4-ville.syrjala@linux.intel.com
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
|
|
The CHV CGM CSC registers are single buffered and so we
may have to write them from the vblank worker, which
imposes very tight dealines. Drop the pointless locking
for the register accessess to reduce the overhead.
All the other registers we bash from the vblank worker
(LUTs) were already made lockless earlier.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220202111616.1579-3-ville.syrjala@linux.intel.com
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
|
|
The pipe/output CSC register writes don't need to be locked
since all the registers are suitably isolated to their own
cachelines. So eliminate the locks to reduce the overhead
during the vblank evade critical section.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220202111616.1579-2-ville.syrjala@linux.intel.com
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
|
|
The struct is only needed in intel_color.c, move it there.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/68cf44fab55c047253c3ed87f8afcf0a86fa157a.1643896905.git.jani.nikula@intel.com
|
|
Enable Pipe Degamma for XE_LPD. Extend the legacy implementation
to incorparate the extended lut size for XE_LPD.
v2: Added a helper for degamma lut size (Ville)
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20211207071135.3660332-3-uma.shankar@intel.com
|
|
Enable pipe color support for Display 13 platforms. Currently
limit to just 10bit gamma and later extend it for logarithmic
gamma, once the new UAPI is agreed by community and implemented
by a userspace consumer.
v2: Updated dev_priv to i915 (Ville)
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20211207071135.3660332-2-uma.shankar@intel.com
|
|
Follow the convention of corresponding .h for .c.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20211122111504.223248-2-jani.nikula@intel.com
|
|
We have to bash in a lot of registers to load the higher
precision LUT modes. The locking overhead is significant, especially
as we have to get this done as quickly as possible during vblank.
So let's switch to unlocked accesses for these. Fortunately the LUT
registers are mostly spread around such that two pipes do not have
any registers on the same cacheline. So as long as commits on the
same pipe are serialized (which they are) we should get away with
this without angering the hardware.
The only exceptions are the PREC_PIPEGCMAX registers on ilk/snb which
we don't use atm as they are only used in the 12bit gamma mode. If/when
we add support for that we may need to remember to still serialize
those registers, though I'm not sure ilk/snb are actually affected
by the same cacheline issue. I think ivb/hsw at least were, but they
use a different set of registers for the precision LUT.
I have a test case which is updating the LUTs on two pipes from a
single atomic commit. Running that in a loop for a minute I get the
following worst case with the locks in place:
intel_crtc_vblank_work_start: pipe B, frame=10037, scanline=1081
intel_crtc_vblank_work_start: pipe A, frame=12274, scanline=769
intel_crtc_vblank_work_end: pipe A, frame=12274, scanline=58
intel_crtc_vblank_work_end: pipe B, frame=10037, scanline=74
And here's the worst case with the locks removed:
intel_crtc_vblank_work_start: pipe B, frame=5869, scanline=1081
intel_crtc_vblank_work_start: pipe A, frame=7616, scanline=769
intel_crtc_vblank_work_end: pipe B, frame=5869, scanline=1096
intel_crtc_vblank_work_end: pipe A, frame=7616, scanline=777
The test was done on a snb using the 10bit 1024 entry LUT mode.
The vtotals for the two displays are 793 and 1125. So we can
see that with the locks ripped out the LUT updates are pretty
nicely confined within the vblank, whereas with the locks in
place we're routinely blasting past the vblank end which causes
visual artifacts near the top of the screen.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20211020223339.669-5-ville.syrjala@linux.intel.com
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
|
|
Keep the functionality and the assert code together.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/0a5fa9b8d4d4615d4e6503b6bb33541c0bccffbb.1632992608.git.jani.nikula@intel.com
|
|
Keep the functionality and the assert code together.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/0229659fb8af6c91c774408c6f7bb8c4ff8735e3.1632992608.git.jani.nikula@intel.com
|
|
This clarifies quite well what functions get used on what platforms
instead of having to decipher the old tree.
v2: fixed IVB mistake (Jani)
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/49e46e61206d4fdcf08fb5dc1978da3fce702134.1632869550.git.jani.nikula@intel.com
|
|
These are only used internally in the color module
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/084a31362f1621d2f556069bb2bc47d362a63823.1632869550.git.jani.nikula@intel.com
|
|
The only real platform with DISPLAY_VER == 10 is GLK, so we don't need
any checks and supporting code for CNL. For DISPLAY_VER >= 11,
ilk_load_csc_matrix() is not used, so make it handle GLK only.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210728215946.1573015-4-lucas.demarchi@intel.com
|
|
Hoist the intel_de.h include from intel_display_types.h one
level up. I need this in order to untangle the include order
so that I can add tracepoints into intel_de.h.
This little cocci script did most of the work for me:
@find@
@@
(
intel_de_read(...)
|
intel_de_read_fw(...)
|
intel_de_write(...)
|
intel_de_write_fw(...)
)
@has_include@
@@
(
#include "intel_de.h"
|
#include "display/intel_de.h"
)
@depends on find && !has_include@
@@
+ #include "intel_de.h"
#include "intel_display_types.h"
@depends on find && !has_include@
@@
+ #include "display/intel_de.h"
#include "display/intel_display_types.h"
Cc: Cooper Chiou <cooper.chiou@intel.com>
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210430143945.6776-1-ville.syrjala@linux.intel.com
|
|
While converting the rest of the driver to use GRAPHICS_VER() and
MEDIA_VER(), following what was done for display, some discussions went
back on what we did for display:
1) Why is the == comparison special that deserves a separate
macro instead of just getting the version and comparing directly
like is done for >, >=, <=?
2) IS_DISPLAY_RANGE() is weird in that it omits the "_VER" for
brevity. If we remove the current users of IS_DISPLAY_VER(), we
could actually repurpose it for a range check
With (1) there could be an advantage if we used gen_mask since multiple
conditionals be combined by the compiler in a single and instruction and
check the result. However a) INTEL_GEN() doesn't use the mask since it
would make the code bigger everywhere else and b) in the cases it made
sense, it also made sense to convert to the _RANGE() variant.
So here we repurpose IS_DISPLAY_VER() to work with a [ from, to ] range
like was the IS_DISPLAY_RANGE() and convert the current IS_DISPLAY_VER()
users to use == and != operators. Aside from the definition changes,
this was done by the following semantic patch:
@@ expression dev_priv, E1; @@
- !IS_DISPLAY_VER(dev_priv, E1)
+ DISPLAY_VER(dev_priv) != E1
@@ expression dev_priv, E1; @@
- IS_DISPLAY_VER(dev_priv, E1)
+ DISPLAY_VER(dev_priv) == E1
@@ expression dev_priv, from, until; @@
- IS_DISPLAY_RANGE(dev_priv, from, until)
+ IS_DISPLAY_VER(dev_priv, from, until)
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
[Jani: Minor conflict resolve while applying.]
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20210413051002.92589-4-lucas.demarchi@intel.com
|
|
GLK has always been a bit of a special case since it reports INTEL_GEN()
as 9, but has version 10 display IP. Now we can properly represent the
display version as 10 and simplify the display generation tests
throughout the display code.
Aside from manually adding the version to the glk_info structure, the
rest of this patch is generated with a Coccinelle semantic patch. Note
that we also need to switch any code that matches gen10 today but *not*
GLK to be CNL-specific:
@@ expression dev_priv; @@
- DISPLAY_VER(dev_priv) > 9
+ DISPLAY_VER(dev_priv) >= 10
@@ expression dev_priv, E; @@
(
- DISPLAY_VER(dev_priv) >= 10 && E
+ (DISPLAY_VER(dev_priv) >= 11 || IS_CANNONLAKE(dev_priv)) && E
|
- DISPLAY_VER(dev_priv) >= 10
+ DISPLAY_VER(dev_priv) >= 11 || IS_CANNONLAKE(dev_priv)
|
- IS_DISPLAY_RANGE(dev_priv, 10, E)
+ IS_DISPLAY_RANGE(dev_priv, 11, E) || IS_CANNONLAKE(dev_priv)
)
@@ expression dev_priv, E, E2; @@
(
- (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ IS_DISPLAY_VER(dev_priv, 10)
|
- E || IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)
+ E || IS_DISPLAY_VER(dev_priv, 10)
|
- (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv))
+ IS_DISPLAY_VER(dev_priv, 10)
|
- IS_GEMINILAKE(dev_priv) || E || IS_CANNONLAKE(dev_priv)
+ E || IS_DISPLAY_VER(dev_priv, 10)
|
- E || IS_GEMINILAKE(dev_priv) || E2 || IS_CANNONLAKE(dev_priv)
+ E || E2 || IS_DISPLAY_VER(dev_priv, 10)
|
- (IS_DISPLAY_VER(dev_priv, 10) || IS_GEMINILAKE(dev_priv))
+ IS_DISPLAY_VER(dev_priv, 10)
|
- (IS_GEMINILAKE(dev_priv) || IS_DISPLAY_VER(dev_priv, 10))
+ IS_DISPLAY_VER(dev_priv, 10)
)
@@ expression dev_priv; @@
- (IS_DISPLAY_VER(dev_priv, 9) && !IS_GEMINILAKE(dev_priv))
+ IS_DISPLAY_VER(dev_priv, 9)
@@ expression dev_priv; @@
(
- !(DISPLAY_VER(dev_priv) >= 11 || IS_DISPLAY_VER(dev_priv, 10))
+ DISPLAY_VER(dev_priv) < 10
|
- (DISPLAY_VER(dev_priv) >= 11 || IS_DISPLAY_VER(dev_priv, 10))
+ DISPLAY_VER(dev_priv) >= 10
)
@@ expression dev_priv, E; @@
- E || DISPLAY_VER(dev_priv) >= 11 || IS_DISPLAY_VER(dev_priv, 10)
+ E || DISPLAY_VER(dev_priv) >= 10
@@ expression dev_priv, E; @@
- (IS_DISPLAY_RANGE(dev_priv, 11, E) || IS_DISPLAY_VER(dev_priv, 10))
+ IS_DISPLAY_RANGE(dev_priv, 10, E)
@@ expression dev_priv; @@
(
- DISPLAY_VER(dev_priv) >= 11 || IS_CANNONLAKE(dev_priv) || IS_GEN9_LP(dev_priv)
+ DISPLAY_VER(dev_priv) >= 10 || IS_GEN9_LP(dev_priv)
|
- IS_GEN9_LP(dev_priv) || DISPLAY_VER(dev_priv) >= 11 || IS_CANNONLAKE(dev_priv)
+ IS_GEN9_LP(dev_priv) || DISPLAY_VER(dev_priv) >= 10
)
@@ expression dev_priv, E; @@
- !(DISPLAY_VER(dev_priv) >= E)
+ DISPLAY_VER(dev_priv) < E
v2:
- Convert gen10 conditions that don't include GLK into CNL conditions.
(Ville)
v3:
- Rework coccinelle rules so that "ver>=10" turns into "ver>=11||is_cnl." (Ville)
v3.1:
- Manually re-add the ".display.version = 10" to glk_info after
regenerating patch via Coccinelle.
v4:
- Also apply cocci rules to intel_pm.c and i915_irq.c! (CI)
Cc: Ville Syrjälä <ville.syrjala@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210322233840.4056851-1-matthew.d.roper@intel.com
|
|
Use Coccinelle to convert most of the usage of INTEL_GEN() and IS_GEN()
in the display code to use DISPLAY_VER() comparisons instead. The
following semantic patch was used:
@@ expression dev_priv, E; @@
- INTEL_GEN(dev_priv) == E
+ IS_DISPLAY_VER(dev_priv, E)
@@ expression dev_priv; @@
- INTEL_GEN(dev_priv)
+ DISPLAY_VER(dev_priv)
@@ expression dev_priv; expression E; @@
- IS_GEN(dev_priv, E)
+ IS_DISPLAY_VER(dev_priv, E)
@@
expression dev_priv;
expression from, until;
@@
- IS_GEN_RANGE(dev_priv, from, until)
+ IS_DISPLAY_RANGE(dev_priv, from, until)
There are still some display-related uses of INTEL_GEN() in intel_pm.c
(watermark code) and i915_irq.c. Those will be updated separately.
v2:
- Use new IS_DISPLAY_RANGE and IS_DISPLAY_VER helpers. (Jani)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210320044245.3920043-4-matthew.d.roper@intel.com
|
|
There are two CSC on pipeline on gen11 and later platform.
User space application is allowed to enable CTM and RGB
to YCbCr coversion at the same time now.
v2: check csc capability in {}_color_check function.
v3: can't support two CSC at the same time in {ivb,glk}_color_check.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Shankar Uma <uma.shankar@intel.com>
Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210118022753.8798-1-shawn.c.lee@intel.com
|
|
Since gamma_mode can have more than two values on ilk+
let's use switch statements when interpreting them.
v2: Fix typo (Uma)
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200925131656.10022-10-ville.syrjala@linux.intel.com
|
|
Since bdw_read_lut_10() uses the auto-increment mode we must
have an equal number of entries in the software LUT and the
hardware LUT. WARN if that is not the case.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200925131656.10022-9-ville.syrjala@linux.intel.com
|
|
Move chv_cgm_gamma_pack() next to the other CGM gamma functions.
Right now it's stuck in the middle of the CGM degamma functions.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200925131656.10022-7-ville.syrjala@linux.intel.com
|
|
Just for some extra consistency let's reset the glk degamma LUT
index back to 0 after we're dong trawling the LUT.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200925131656.10022-6-ville.syrjala@linux.intel.com
|
|
glk_read_lut_10() works just fine for all bdw+ platforms, so
rename it.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200925131656.10022-5-ville.syrjala@linux.intel.com
|
|
Pre-allocate command buffer in atomic_commit using intel_dsb_prepare
function which also includes pinning and map in cpu domain.
No functional change is dsb write/commit functions.
Now dsb get/put function is removed and ref-count mechanism is
not needed. Below dsb api added to do respective job mentioned
below.
intel_dsb_prepare - Allocate, pin and map the buffer.
intel_dsb_cleanup - Unpin and release the gem object.
RFC: Initial patch for design review.
v2: included _init() part in _prepare(). [Daniel, Ville]
v3: dsb_cleanup called after cleanup_planes. [Daniel]
v4: dsb structure is moved to intel_crtc_state from intel_crtc. [Maarten]
v5: dsb get/put/ref-count mechanism removed. [Maarten]
v6: Based on review feedback following changes are added,
- replaced intel_dsb structure by pointer in intel_crtc_state. [Maarten]
- passing intel_crtc_state to dsp-api to simplify the code. [Maarten]
- few dsb functions prototype modified to simplify code.
v7: added few cosmetic changes suggested by Jani and null check for
crtc_state in dsb_cleanup removed as suggested by Maarten.
v8: changed the function parameter to intel_crtc_state* of
ivb_load_lut_ext_max() from intel_crtc. [Maarten]
v9: error handling improved in _write() and prepare(). [Maarten]
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200520130737.11240-1-animesh.manna@intel.com
|
|
For icl+, have hw read out to create hw blob of gamma
lut values. icl+ platforms supports multi segmented gamma
mode by default, add hw lut creation for this mode.
This will be used to validate gamma programming using dsb
(display state buffer) which is a tgl specific feature.
v2: -readout code for multisegmented gamma has to come
up with some intermediate entries that aren't preserved
in hardware (Jani N)
-linear interpolation (Ville)
-moved common code to check gamma_enable to specific funcs,
since icl doesn't support that
v3: -use u16 instead of __u16 [Jani N]
-used single lut [Jani N]
-improved and more readable for loops [Jani N]
-read values directly to actual locations and then fill gaps [Jani N]
-moved cleaning to patch 1 [Jani N]
-renamed icl_read_lut_multi_seg() to icl_read_lut_multi_segment to
make it similar to icl_load_luts()
-renamed icl_compute_interpolated_gamma_blob() to
icl_compute_interpolated_gamma_lut_values() more sensible, I guess
v4: -removed interpolated func for creating gamma lut values
-removed readouts of fine and coarse segments, failure to read PAL_PREC_DATA
correctly
v5: -added gamma_enable check inside read_luts()
v6: -renamed intel_color_lut_entry_equal() to intel_color_lut_entries_equal() [Ville]
-changed if-else to switch [Ville]
-removed intel_color_lut_entry_multi_equal() [Ville]
v7: -checkpatch warnings
v8: -rebased
v9: -rebased, aligned with Ville's style of gamma cleanup
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200317135736.14305-1-swati2.sharma@intel.com
|
|
The low level read_lut() functions don't need the entire crtc state
as they know exactly what they're reading. Just need to pass in the
crtc to get at the pipe. This now neatly mirrors the load_lut()
direction.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200303173313.28117-10-ville.syrjala@linux.intel.com
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
|
|
PIPEGCMAX is a 11.6 (or 1.16 if you will) value. Ie. it can
represent a value of 1.0 when the maximum we can store in the
software LUT is 0.ffff. Clamp the value so that it gets
saturated to the max the uapi supports.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200303173313.28117-9-ville.syrjala@linux.intel.com
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
|
|
Extract all the 'hw value -> LUT entry' stuff into small helpers
to make the main 'read out the entire LUT' loop less bogged down
by such mundane details.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200303173313.28117-8-ville.syrjala@linux.intel.com
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
|
|
A variable called 'i' having an unsigned type is just looking for
trouble, and using a sized type generally makes no sense either.
Change all of them to just plain old int. And do the same for some
'lut_size' variables which generally provide the loop end codition
for 'i'.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200303173313.28117-7-ville.syrjala@linux.intel.com
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
|
|
chv_read_cgm_lut() specifically reads the CGM _gamma_ LUT so
let's rename it to reflect that fact. This also mirrors
the other direction's chv_load_cgm_gamma().
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200303173313.28117-6-ville.syrjala@linux.intel.com
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
|
|
We're talking about LUT contents here so let's call the thing
'lut' rather than 'blob_data'. This is the name the load_lut()
code used before already.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200303173313.28117-5-ville.syrjala@linux.intel.com
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
|
|
To mirror the load_luts path let's clone an ilk+ version
from i9xx_read_lut_8(). I guess the extra branch isn't a huge
issue but feels better to make a clean split.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200303173313.28117-4-ville.syrjala@linux.intel.com
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
|
|
Split i9xx_load_luts_internal() into neat gmch vs. ilk+ chunks.
Avoids at least one branch in the inner loop, and makes life
a bit less confusing.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200303173313.28117-3-ville.syrjala@linux.intel.com
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
|
|
Only load the CGM CSC based on the cgm_mode bit like we
do with the gamma/degamma LUTs. And make the function
naming and arguments consistent as well.
TODO: the code to convert the coefficients look totally
bogus. IIRC CHV uses two's complement format but the code
certainly doesn't generate that, so probably negative
coefficients are totally busted.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200303173313.28117-2-ville.syrjala@linux.intel.com
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
|
|
It irks me to use crtc_state_is_legacy_gamma() inside the guts
of the CHV color management code. Let's get rid of it and instead
just consult cgm_mode to figure out if we want to enable the pipe
gamma or the CGM gamma.
Also CHV display engine is based on i965/g4x so we should fall back
to the i965 path when the CGM gamma is not used.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191107151725.10507-3-ville.syrjala@linux.intel.com
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
|
|
We have a nice little helper to compute a single LUT entry
for everything except the 8bpc legacy gamma mode. Let's
complete the set.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191107151725.10507-5-ville.syrjala@linux.intel.com
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
|
|
Initial conversion of the straightforward printk based logging macros to
the struct drm_device based logging macros in
i915/display/intel_color.c.
Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200206080014.13759-5-wambui.karugax@gmail.com
|
|
drm specific WARN* calls include device information in the
backtrace, so we know what device the warnings originate from.
Covert all the calls of WARN* with device specific drm_WARN*
variants in functions where drm_device or drm_i915_private struct
pointer is readily available.
The conversion was done automatically with below coccinelle semantic
patch. checkpatch errors/warnings are fixed manually.
@rule1@
identifier func, T;
@@
func(...) {
...
struct drm_device *T = ...;
<...
(
-WARN(
+drm_WARN(T,
...)
|
-WARN_ON(
+drm_WARN_ON(T,
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(T,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(T,
...)
)
...>
}
@rule2@
identifier func, T;
@@
func(struct drm_device *T,...) {
<...
(
-WARN(
+drm_WARN(T,
...)
|
-WARN_ON(
+drm_WARN_ON(T,
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(T,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(T,
...)
)
...>
}
@rule3@
identifier func, T;
@@
func(...) {
...
struct drm_i915_private *T = ...;
<+...
(
-WARN(
+drm_WARN(&T->drm,
...)
|
-WARN_ON(
+drm_WARN_ON(&T->drm,
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(&T->drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(&T->drm,
...)
)
...+>
}
@rule4@
identifier func, T;
@@
func(struct drm_i915_private *T,...) {
<+...
(
-WARN(
+drm_WARN(&T->drm,
...)
|
-WARN_ON(
+drm_WARN_ON(&T->drm,
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(&T->drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(&T->drm,
...)
)
...+>
}
Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200128181603.27767-20-pankaj.laxminarayan.bharadiya@intel.com
|
|
The implicit "dev_priv" local variable use has been a long-standing pain
point in the register access macros I915_READ(), I915_WRITE(),
POSTING_READ(), I915_READ_FW(), and I915_WRITE_FW().
Replace them with the corresponding new display engine register
accessors intel_de_read(), intel_de_write(), intel_de_posting_read(),
intel_de_read_fw(), and intel_de_write_fw().
No functional changes.
Generated using the following semantic patch:
@@
expression REG, OFFSET;
@@
- I915_READ(REG)
+ intel_de_read(dev_priv, REG)
@@
expression REG, OFFSET;
@@
- POSTING_READ(REG)
+ intel_de_posting_read(dev_priv, REG)
@@
expression REG, OFFSET;
@@
- I915_WRITE(REG, OFFSET)
+ intel_de_write(dev_priv, REG, OFFSET)
@@
expression REG;
@@
- I915_READ_FW(REG)
+ intel_de_read_fw(dev_priv, REG)
@@
expression REG, OFFSET;
@@
- I915_WRITE_FW(REG, OFFSET)
+ intel_de_write_fw(dev_priv, REG, OFFSET)
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/80d2cac864eb3f964587e74fbb004940889a2928.1579871655.git.jani.nikula@intel.com
|
|
The LUTs are single buffered so in order to program them without
tearing we'd have to do it during vblank (actually to be 100%
effective it has to happen between start of vblank and frame start).
We have no proper mechanism for that at the moment so we just
defer loading them after the vblank waits have happened. That
is not quite sufficient (especially when committing multiple pipes
whose vblanks don't line up) so the LUT load will often leak into
the following frame causing tearing.
However in case the hardware wasn't previously using the LUT we
can preload it before setting the enable bit (which is double
buffered so won't tear). Let's determine if we can do such
preloading and make it happen. Slight variation between the
hardware requires some platforms specifics in the checks.
Hans is seeing ugly colored flash on VLV/CHV macchines (GPD win
and Asus T100HA) when the gamma LUT gets loaded for the first
time as the BIOS has left some junk in the LUT memory.
v2: Deal with uapi vs. hw crtc state split
s/GCM/CGM/ typo fix
Cc: Hans de Goede <hdegoede@redhat.com>
Fixes: 051a6d8d3ca0 ("drm/i915: Move LUT programming to happen after vblank waits")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191030190815.7359-1-ville.syrjala@linux.intel.com
Tested-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
|
|
Split up crtc_state->base to uapi. This is done using the following patch,
ran after the previous commit that splits out any hw references:
@@
struct intel_crtc_state *T;
@@
-T->base
+T->uapi
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191031112610.27608-5-maarten.lankhorst@linux.intel.com
|
|
Split up crtc_state->base to hw where appropriate. This is done using the following patch:
@@
struct intel_crtc_state *T;
identifier x =~ "^(active|enable|degamma_lut|gamma_lut|ctm|mode|adjusted_mode)$";
@@
-T->base.x
+T->hw.x
@@
struct drm_crtc_state *T;
identifier x =~ "^(active|enable|degamma_lut|gamma_lut|ctm|mode|adjusted_mode)$";
@@
-to_intel_crtc_state(T)->base.x
+to_intel_crtc_state(T)->hw.x
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191031112610.27608-4-maarten.lankhorst@linux.intel.com
|
|
Moved common code to check gamma_enable to specific funcs per platform
in bit_precision func. icl doesn't support that and chv has separate
enable knob for CGM LUT.
v2:
-Simplified chv_gamma_precision() [Ville]
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191009065542.27415-3-swati2.sharma@intel.com
|
|
Premature gamma lut prepration and loading which was getting
reflected in first modeset causing different colors on
screen during boot.
Issue: In BIOS, gamma is disabled by default. However, legacy read_luts()
was setting crtc_state->base.gamma_lut and gamma_lut was programmed
with junk values which led to visual artifacts (different
colored screens instead of usual black during boot).
Fix: Calling read_luts() only when gamma is enabled which will happen
after first modeset.
This fix is independent from the revert 1b8588741fdc ("Revert
"drm/i915/color: Extract icl_read_luts()"") and should fix different colors
on screen in legacy platforms too.
v2:
-Added gamma_enable checks inside read_luts() [Ville/Jani N]
-Corrected gamma enable check for CHV [Ville]
v3:
-Added check in ilk_read_luts() [Ville]
-Simplified gamma enable check for CHV [Ville]
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111809
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111885
Tested-by: Jani Saarinen <jani.saarinen@intel.com>
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191009065542.27415-2-swati2.sharma@intel.com
|