On Fri, Mar 13, 2020 at 08:45:35AM +0000, Laxminarayan Bharadiya, Pankaj wrote:
-----Original Message----- From: Ville Syrjälä ville.syrjala@linux.intel.com Sent: 12 March 2020 19:25 To: Laxminarayan Bharadiya, Pankaj pankaj.laxminarayan.bharadiya@intel.com Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel- gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airlied@linux.ie; maarten.lankhorst@linux.intel.com; tzimmermann@suse.de; mripard@kernel.org; mihail.atanassov@arm.com; Joonas Lahtinen joonas.lahtinen@linux.intel.com; Vivi, Rodrigo rodrigo.vivi@intel.com; Chris Wilson chris@chris-wilson.co.uk; Souza, Jose jose.souza@intel.com; De Marchi, Lucas lucas.demarchi@intel.com; Roper, Matthew D matthew.d.roper@intel.com; Deak, Imre imre.deak@intel.com; Shankar, Uma uma.shankar@intel.com; linux-kernel@vger.kernel.org; Nautiyal, Ankit K ankit.k.nautiyal@intel.com Subject: Re: [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support
On Thu, Mar 12, 2020 at 09:13:24AM +0000, Laxminarayan Bharadiya, Pankaj wrote:
-----Original Message----- From: Ville Syrjälä ville.syrjala@linux.intel.com Sent: 10 March 2020 21:47 To: Laxminarayan Bharadiya, Pankaj pankaj.laxminarayan.bharadiya@intel.com Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel- gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airlied@linux.ie; maarten.lankhorst@linux.intel.com; tzimmermann@suse.de; mripard@kernel.org; mihail.atanassov@arm.com; Joonas Lahtinen joonas.lahtinen@linux.intel.com; Vivi, Rodrigo rodrigo.vivi@intel.com; Chris Wilson chris@chris-wilson.co.uk; Souza, Jose jose.souza@intel.com; De Marchi, Lucas lucas.demarchi@intel.com; Roper, Matthew D matthew.d.roper@intel.com; Deak, Imre imre.deak@intel.com; Shankar, Uma uma.shankar@intel.com; linux- kernel@vger.kernel.org; Nautiyal, Ankit K ankit.k.nautiyal@intel.com Subject: Re: [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support
On Tue, Feb 25, 2020 at 12:35:45PM +0530, Pankaj Bharadiya wrote:
Integer scaling (IS) is a nearest-neighbor upscaling technique that simply scales up the existing pixels by an integer (i.e., whole number) multiplier.Nearest-neighbor (NN) interpolation works by filling in the missing color values in the upscaled image with that of the coordinate-mapped nearest source pixel value.
Both IS and NN preserve the clarity of the original image. Integer scaling is particularly useful for pixel art games that rely on sharp, blocky images to deliver their distinctive look.
Program the scaler filter coefficients to enable the NN filter if scaling filter property is set to DRM_SCALING_FILTER_NEAREST_NEIGHBOR and enable integer scaling.
Bspec: 49247
Signed-off-by: Pankaj Bharadiya pankaj.laxminarayan.bharadiya@intel.com Signed-off-by: Ankit Nautiyal ankit.k.nautiyal@intel.com
drivers/gpu/drm/i915/display/intel_display.c | 83 +++++++++++++++++++- drivers/gpu/drm/i915/display/intel_display.h +++++++++++++++++++| 2 + drivers/gpu/drm/i915/display/intel_sprite.c | 20 +++-- 3 files changed, 97 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index b5903ef3c5a0..6d5f59203258 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -6237,6 +6237,73 @@ void skl_scaler_disable(const struct
intel_crtc_state *old_crtc_state)
skl_detach_scaler(crtc, i);}
+/**
- Theory behind setting nearest-neighbor integer scaling:
- 17 phase of 7 taps requires 119 coefficients in 60 dwords per set.
- The letter represents the filter tap (D is the center tap)
+and the number
- represents the coefficient set for a phase (0-16).
+------------+------------------------+------------------------+
|Index value | Data value coeffient 1 | Data value coeffient 2 |
+------------+------------------------+------------------------+
| 00h | B0 | A0 |
+------------+------------------------+------------------------+
| 01h | D0 | C0 |
+------------+------------------------+------------------------+
| 02h | F0 | E0 |
+------------+------------------------+------------------------+
| 03h | A1 | G0 |
+------------+------------------------+------------------------+
| 04h | C1 | B1 |
+------------+------------------------+------------------------+
| ... | ... | ... |
+------------+------------------------+------------------------+
| 38h | B16 | A16 |
+------------+------------------------+------------------------+
| 39h | D16 | C16 |
+------------+------------------------+------------------------+
| 3Ah | F16 | C16 |
+------------+------------------------+------------------------+
| 3Bh | Reserved | G16 |
+------------+------------------------+------------------------+
- To enable nearest-neighbor scaling: program scaler
+coefficents with
- the center tap (Dxx) values set to 1 and all other values set
+to +0 as per
- SCALER_COEFFICIENT_FORMAT
- */
+void skl_setup_nearest_neighbor_filter(struct drm_i915_private
*dev_priv,
enum pipe pipe, int scaler_id)skl_scaler_...
+{
- int coeff = 0;
- int phase = 0;
- int tap;
- int val = 0;
Needlessly wide scope for most of these.
- /*enable the index auto increment.*/
- intel_de_write_fw(dev_priv, SKL_PS_COEF_INDEX_SET0(pipe,
scaler_id),
_PS_COEE_INDEX_AUTO_INC);- for (phase = 0; phase < 17; phase++) {
for (tap = 0; tap < 7; tap++) {coeff++;Can be part of the % check.
OK.
if (tap == 3)val = (phase % 2) ? (0x800) : (0x800 << 16);Parens overload.
OK. Will remove.
if (coeff % 2 == 0) {intel_de_write_fw(dev_priv,SKL_PS_COEF_DATA_SET0(pipe, scaler_id), val);
val = 0;Can drop this val=0 if you move the variable into tight scope and initialize there.
Moving val=0 initialization to the tight scope will not work here as we need to retain "val" and write only when 2 coefficients are ready (since 2 coefficients are packed in 1 dword).
e.g. for (12th , 11th) coefficients, coefficient reg value should be ( (0 << 16) |
0x800).
If we initialize val = 0 in tight loop, 0 will be written to coefficient register.
Hmm, right. I guess I'd try to rearrange this to iterate the registers directly instead of the phases and taps. Something like this perhaps:
static int cnl_coef_tap(int i) { return i % 7; }
static u16 cnl_coef(int t)
cnl_coef -> cnl_nearest_filter_coef. Right?
Right.
{ return t == 3 ? 0x0800 : 0x3000; }
static void cnl_program_nearest_filter_coefs(void) { int i;
for (i = 0; i < 17 * 7; i += 2) { uint32_t tmp; int t;
t = cnl_coef_tap(i); tmp = cnl_nearest_filter_coef(t); t = cnl_coef_tap(i + 1); tmp |= cnl_nearest_filter_coef(t) << 16; intel_de_write_fw(tmp);} }
More readable I think. The downside being all those modulo operations but hopefully that's all in the noise when it comes to performance.
Looks better, thanks for spending time on this. I will try this out.
Thanks, Pankaj
-- Ville Syrjälä Intel