| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <E92BA18FDE0A5B43B7B3DA7FCA031286057B474F@BGSMSX107.gar.corp.intel.com>
Date: Fri, 13 Mar 2020 08:45:35 +0000
From: "Laxminarayan Bharadiya, Pankaj"
<pankaj.laxminarayan.bharadiya@...el.com>
To: Ville Syrjälä <ville.syrjala@...ux.intel.com>
CC: "jani.nikula@...ux.intel.com" <jani.nikula@...ux.intel.com>,
"daniel@...ll.ch" <daniel@...ll.ch>,
"intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"airlied@...ux.ie" <airlied@...ux.ie>,
"maarten.lankhorst@...ux.intel.com"
<maarten.lankhorst@...ux.intel.com>,
"tzimmermann@...e.de" <tzimmermann@...e.de>,
"mripard@...nel.org" <mripard@...nel.org>,
"mihail.atanassov@....com" <mihail.atanassov@....com>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
"Vivi, Rodrigo" <rodrigo.vivi@...el.com>,
Chris Wilson <chris@...is-wilson.co.uk>,
"Souza, Jose" <jose.souza@...el.com>,
"De Marchi, Lucas" <lucas.demarchi@...el.com>,
"Roper, Matthew D" <matthew.d.roper@...el.com>,
"Deak, Imre" <imre.deak@...el.com>,
"Shankar, Uma" <uma.shankar@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Nautiyal, Ankit K" <ankit.k.nautiyal@...el.com>
Subject: RE: [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based
integer scaling support
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@...ux.intel.com>
> Sent: 12 March 2020 19:25
> To: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@...el.com>
> Cc: jani.nikula@...ux.intel.com; daniel@...ll.ch; intel-
> gfx@...ts.freedesktop.org; dri-devel@...ts.freedesktop.org; airlied@...ux.ie;
> maarten.lankhorst@...ux.intel.com; tzimmermann@...e.de;
> mripard@...nel.org; mihail.atanassov@....com; Joonas Lahtinen
> <joonas.lahtinen@...ux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@...el.com>;
> Chris Wilson <chris@...is-wilson.co.uk>; Souza, Jose <jose.souza@...el.com>;
> De Marchi, Lucas <lucas.demarchi@...el.com>; Roper, Matthew D
> <matthew.d.roper@...el.com>; Deak, Imre <imre.deak@...el.com>; Shankar,
> Uma <uma.shankar@...el.com>; linux-kernel@...r.kernel.org; Nautiyal, Ankit K
> <ankit.k.nautiyal@...el.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@...ux.intel.com>
> > > Sent: 10 March 2020 21:47
> > > To: Laxminarayan Bharadiya, Pankaj
> > > <pankaj.laxminarayan.bharadiya@...el.com>
> > > Cc: jani.nikula@...ux.intel.com; daniel@...ll.ch; intel-
> > > gfx@...ts.freedesktop.org; dri-devel@...ts.freedesktop.org;
> > > airlied@...ux.ie; maarten.lankhorst@...ux.intel.com;
> > > tzimmermann@...e.de; mripard@...nel.org; mihail.atanassov@....com;
> > > Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>; Vivi, Rodrigo
> > > <rodrigo.vivi@...el.com>; Chris Wilson <chris@...is-wilson.co.uk>;
> > > Souza, Jose <jose.souza@...el.com>; De Marchi, Lucas
> > > <lucas.demarchi@...el.com>; Roper, Matthew D
> > > <matthew.d.roper@...el.com>; Deak, Imre <imre.deak@...el.com>;
> > > Shankar, Uma <uma.shankar@...el.com>; linux- kernel@...r.kernel.org;
> > > Nautiyal, Ankit K <ankit.k.nautiyal@...el.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@...el.com>
> > > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@...el.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?
> {
> 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
Powered by blists - more mailing lists