RE: [Intel-gfx] [PATCH v5 1/5] drm: Introduce plane and CRTC scaling filter properties
> -Original Message-
> From: Shankar, Uma
> Sent: 19 August 2020 13:44
> To: Laxminarayan Bharadiya, Pankaj
> ; jani.nik...@linux.intel.com;
> dan...@ffwll.ch; intel-...@lists.freedesktop.org; dri-
> de...@lists.freedesktop.org; ville.syrj...@linux.intel.com;
> dani...@collabora.com; Lattannavar, Sameer
> ; Maarten Lankhorst
> ; Maxime Ripard ;
> Thomas Zimmermann ; David Airlie
> Subject: RE: [Intel-gfx] [PATCH v5 1/5] drm: Introduce plane and CRTC scaling
> filter properties
>
>
>
> > -Original Message-
> > From: Intel-gfx On Behalf Of
> > Pankaj Bharadiya
> > Sent: Monday, August 3, 2020 10:00 AM
> > To: jani.nik...@linux.intel.com; dan...@ffwll.ch;
> > intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > ville.syrj...@linux.intel.com; dani...@collabora.com; Lattannavar,
> > Sameer ; Maarten Lankhorst
> > ; Maxime Ripard
> > ; Thomas Zimmermann ;
> David
> > Airlie
> > Subject: [Intel-gfx] [PATCH v5 1/5] drm: Introduce plane and CRTC
> > scaling filter properties
> >
> > Introduce per-plane and per-CRTC scaling filter properties to allow
> > userspace to select the driver's default scaling filter or
> > Nearest-neighbor(NN) filter for upscaling operations on CRTC and plane.
> >
> > Drivers can set up this property for a plane by calling
> > drm_plane_create_scaling_filter() and for a CRTC by calling
> > drm_crtc_create_scaling_filter().
> >
> > NN filter works by filling in the missing color values in the upscaled
> > image with that of the coordinate-mapped nearest source pixel value.
> >
> > NN filter for integer multiple scaling can be particularly useful for
> > for pixel art games that rely on sharp, blocky images to deliver their
> > distinctive
> look.
> >
> > changes since v3:
> > * Refactor code, add new function for common code (Ville) changes since v2:
> > * Create per-plane and per-CRTC scaling filter property (Ville) changes
> > since
> v1:
> > * None
> > changes since RFC:
> > * Add separate properties for plane and CRTC (Ville)
> >
> > Signed-off-by: Pankaj Bharadiya
> >
> > ---
> > drivers/gpu/drm/drm_atomic_uapi.c | 8 +++
> > drivers/gpu/drm/drm_crtc.c | 48 +++
> > drivers/gpu/drm/drm_crtc_internal.h | 3 +
> > drivers/gpu/drm/drm_plane.c | 90 +
> > include/drm/drm_crtc.h | 16 +
> > include/drm/drm_plane.h | 21 +++
> > 6 files changed, 186 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 25c269bc4681..ef82009035e6 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -469,6 +469,8 @@ static int drm_atomic_crtc_set_property(struct
> > drm_crtc *crtc,
> > return -EFAULT;
> >
> > set_out_fence_for_crtc(state->state, crtc, fence_ptr);
> > + } else if (property == crtc->scaling_filter_property) {
> > + state->scaling_filter = val;
> > } else if (crtc->funcs->atomic_set_property) {
> > return crtc->funcs->atomic_set_property(crtc, state, property,
> > val);
> > } else {
> > @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> > *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> > else if (property == config->prop_out_fence_ptr)
> > *val = 0;
> > + else if (property == crtc->scaling_filter_property)
> > + *val = state->scaling_filter;
> > else if (crtc->funcs->atomic_get_property)
> > return crtc->funcs->atomic_get_property(crtc, state, property,
> > val);
> > else
> > @@ -585,6 +589,8 @@ static int drm_atomic_plane_set_property(struct
> > drm_plane *plane,
> > sizeof(struct drm_rect),
> > );
> > return ret;
> > + } else if (property == plane->scaling_filter_property) {
> > + state->scaling_filter = val;
> > } else if (plane->funcs->atomic_set_property) {
> > return plane->funcs->atomic_set_property(plane, state,
> > property, val);
> > @@ -643,6 +649,8 @@ drm_atomic_plane_get_property(struct drm_plane
> > *plane,
> > } else if (property == config->prop_fb_damage_clips) {
> >
RE: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over WARN*
> -Original Message-
> From: Jani Nikula
> Sent: 19 May 2020 19:12
> To: Laxminarayan Bharadiya, Pankaj
> ; dan...@ffwll.ch; intel-
> g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> David Airlie ; Ville Syrjälä
> ; Chris
> Wilson ; Deak, Imre ;
> Maarten Lankhorst
> Subject: RE: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over
> WARN*
>
> On Fri, 08 May 2020, "Laxminarayan Bharadiya, Pankaj"
>wrote:
> >> -Original Message-
> >> From: Jani Nikula
> >> Sent: 08 May 2020 12:19
> >> To: Laxminarayan Bharadiya, Pankaj
> >> ; dan...@ffwll.ch; intel-
> >> g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Joonas
> >> Lahtinen ; Vivi, Rodrigo
> >> ; David Airlie ; Ville
> >> Syrjälä ; Chris Wilson
> >> ; Deak, Imre ; Maarten
> >> Lankhorst ; Laxminarayan
> >> Bharadiya, Pankaj
> >> Subject: Re: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN*
> >> over
> >> WARN*
> >>
> >> On Mon, 04 May 2020, Pankaj Bharadiya
> >> wrote:
> >> > struct drm_device specific drm_WARN* macros include device
> >> > information in the backtrace, so we know what device the warnings
> originate from.
> >> >
> >> > Prefer drm_WARN* over WARN* calls.
> >> >
> >> > changes since v1:
> >> > - Added dev_priv local variable and used it in drm_WARN* calls
> >> > (Jani)
> >>
> >> In the earlier patches you're adding i915 local variable, here it's
> >> dev_priv. We're gradually transitioning from dev_priv to i915, so I'm
> >> not thrilled about adding new dev_priv.
> >
> > dev_priv name is being used throughout the file. So to be consistent
> > with rest of the code, I used dev_priv variable in this specific file.
> >
> > Shall I rename it to i915?
> >
> > I used i915 or dev_priv variable name based on what variable name
> > being already used for struct drm_i915_private pointer in a given file.
>
> I understand your reasoning. However with i915 I've preferred to switch when
> possible.
>
> Regardless, pushed the series now. Thanks for the patches, and sorry for the
> delay.
Thank you Jani.
Will you please review - https://patchwork.freedesktop.org/series/75265/#rev2
Thanks,
Pankaj
>
> BR,
> Jani.
>
>
>
> >
> > Thanks,
> > Pankaj
> >
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >>
> >> >
> >> > Signed-off-by: Pankaj Bharadiya
> >> >
> >> > ---
> >> > drivers/gpu/drm/i915/display/intel_sdvo.c | 21
> >> > ++---
> >> > 1 file changed, 14 insertions(+), 7 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> > b/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> > index bc6c26818e15..773523dcd107 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> > @@ -411,6 +411,7 @@ static const char *sdvo_cmd_name(u8 cmd)
> >> > static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
> >> > const void *args, int args_len) {
> >> > +struct drm_i915_private *dev_priv =
> >> > +to_i915(intel_sdvo->base.base.dev);
> >> > const char *cmd_name;
> >> > int i, pos = 0;
> >> > char buffer[64];
> >> > @@ -431,7 +432,7 @@ static void intel_sdvo_debug_write(struct
> >> > intel_sdvo
> >> *intel_sdvo, u8 cmd,
> >> > else
> >> > BUF_PRINT("(%02X)", cmd);
> >> >
> >> > -WARN_ON(pos >= sizeof(buffer) - 1);
> >> > +drm_WARN_ON(_priv->drm, pos >= sizeof(buffer) - 1);
> >> > #undef BUF_PRINT
> >> >
> >> > DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd,
> >> > buffer); @@ -533,6 +534,7 @@ static bool
> >> > intel_sdvo_write_cmd(struct intel_sdvo *intel_sdvo, u8 cmd, static
> >> > bool intel_sdvo_read_response(struct
> >> intel_sdvo *intel_sdvo,
> >> > void *response, int response_len)
> >> > {
> >> > +struct drm_i915_private *dev_priv =
> >>
RE: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over WARN*
> -Original Message-
> From: Jani Nikula
> Sent: 08 May 2020 12:19
> To: Laxminarayan Bharadiya, Pankaj
> ; dan...@ffwll.ch; intel-
> g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> David Airlie ; Ville Syrjälä
> ; Chris
> Wilson ; Deak, Imre ;
> Maarten Lankhorst ; Laxminarayan
> Bharadiya, Pankaj
> Subject: Re: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over
> WARN*
>
> On Mon, 04 May 2020, Pankaj Bharadiya
> wrote:
> > struct drm_device specific drm_WARN* macros include device information
> > in the backtrace, so we know what device the warnings originate from.
> >
> > Prefer drm_WARN* over WARN* calls.
> >
> > changes since v1:
> > - Added dev_priv local variable and used it in drm_WARN* calls (Jani)
>
> In the earlier patches you're adding i915 local variable, here it's dev_priv.
> We're
> gradually transitioning from dev_priv to i915, so I'm not thrilled about
> adding
> new dev_priv.
dev_priv name is being used throughout the file. So to be consistent with rest
of the
code, I used dev_priv variable in this specific file.
Shall I rename it to i915?
I used i915 or dev_priv variable name based on what variable name being
already used for struct drm_i915_private pointer in a given file.
Thanks,
Pankaj
>
> BR,
> Jani.
>
>
>
> >
> > Signed-off-by: Pankaj Bharadiya
> >
> > ---
> > drivers/gpu/drm/i915/display/intel_sdvo.c | 21 ++---
> > 1 file changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > index bc6c26818e15..773523dcd107 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > @@ -411,6 +411,7 @@ static const char *sdvo_cmd_name(u8 cmd) static
> > void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
> >const void *args, int args_len) {
> > + struct drm_i915_private *dev_priv =
> > +to_i915(intel_sdvo->base.base.dev);
> > const char *cmd_name;
> > int i, pos = 0;
> > char buffer[64];
> > @@ -431,7 +432,7 @@ static void intel_sdvo_debug_write(struct intel_sdvo
> *intel_sdvo, u8 cmd,
> > else
> > BUF_PRINT("(%02X)", cmd);
> >
> > - WARN_ON(pos >= sizeof(buffer) - 1);
> > + drm_WARN_ON(_priv->drm, pos >= sizeof(buffer) - 1);
> > #undef BUF_PRINT
> >
> > DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd,
> > buffer); @@ -533,6 +534,7 @@ static bool intel_sdvo_write_cmd(struct
> > intel_sdvo *intel_sdvo, u8 cmd, static bool intel_sdvo_read_response(struct
> intel_sdvo *intel_sdvo,
> > void *response, int response_len) {
> > + struct drm_i915_private *dev_priv =
> > +to_i915(intel_sdvo->base.base.dev);
> > const char *cmd_status;
> > u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
> > u8 status;
> > @@ -597,7 +599,7 @@ static bool intel_sdvo_read_response(struct
> intel_sdvo *intel_sdvo,
> > BUF_PRINT(" %02X", ((u8 *)response)[i]);
> > }
> >
> > - WARN_ON(pos >= sizeof(buffer) - 1);
> > + drm_WARN_ON(_priv->drm, pos >= sizeof(buffer) - 1);
> > #undef BUF_PRINT
> >
> > DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
> @@
> > -1081,6 +1083,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct
> intel_sdvo *intel_sdvo,
> > struct intel_crtc_state
> > *crtc_state,
> > struct drm_connector_state
> *conn_state) {
> > + struct drm_i915_private *dev_priv =
> > +to_i915(intel_sdvo->base.base.dev);
> > struct hdmi_avi_infoframe *frame = _state->infoframes.avi.avi;
> > const struct drm_display_mode *adjusted_mode =
> > _state->hw.adjusted_mode;
> > @@ -1106,7 +1109,7 @@ static bool
> intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo,
> >
> HDMI_QUANTIZATION_RANGE_FULL);
> >
> > ret = hdmi_avi_infoframe_check(frame);
> > - if (WARN_ON(ret))
> > + if (drm_WARN_ON(_priv->drm, ret))
> > return false;
> >
> > return true;
> > @@ -1115,6 +1118,7 @@ static bool
> > intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo, static bool
> intel_sdvo_set_avi_infoframe(struct intel_sdvo *in
RE: [PATCH] drm/todo: Add todo to make i915 WARN* calls drm device specific
> -Original Message- > From: Sam Ravnborg > Sent: 31 March 2020 00:57 > To: Laxminarayan Bharadiya, Pankaj > > Cc: jani.nik...@linux.intel.com; dan...@ffwll.ch; intel- > g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Maarten Lankhorst > ; Maxime Ripard ; > Thomas Zimmermann ; David Airlie ; > Jonathan Corbet > Subject: Re: [PATCH] drm/todo: Add todo to make i915 WARN* calls drm device > specific > > Hi Pankaj. > > On Tue, Mar 31, 2020 at 12:45:24AM +0530, Pankaj Bharadiya wrote: > > With below commit, we have new struct drm_device based WARN* macros, > > which include device specific information in the backtrace. > > > > commit dc1a73e50f9c63d4dd928df538082200467dc4b1 > > Author: Pankaj Bharadiya > > Date: Wed Jan 15 09:14:45 2020 +0530 > > > > drm/print: introduce new struct drm_device based WARN* macros > > > > Majority of the i915 WARN* are already converted to use struct > > drm_device specific drm_WARN* calls.Add new todo entry for > Add space after '.' > > > pending conversions. > > > > Signed-off-by: Pankaj Bharadiya > > > > --- > > Documentation/gpu/todo.rst | 14 +- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > index 37a3a023c114..0cb32df89784 100644 > > --- a/Documentation/gpu/todo.rst > > +++ b/Documentation/gpu/todo.rst > > @@ -575,6 +575,18 @@ See drivers/gpu/drm/amd/display/TODO for tasks. > > > > Contact: Harry Wentland, Alex Deucher > > > > +Make i915 WARN* Calls struct drm_device Specific > > + > > + > > +struct drm_device specific drm_WARN* macros include device > > +information in the backtrace, so we know what device the warnings > > +originate from. Convert all the calls of WARN* with drm_WARN* calls > > +in i915. While at it, remove WARN* which are not truly valid. > > + > > +Contact: Jani Nikula > > + > > +Level: Starter > > + > > Bootsplash > > == > > > > @@ -595,7 +607,7 @@ Level: Advanced > > Outside DRM > > === > > > > -Convert fbdev drivers to DRM > > +Convert fbdev drivers to > Unrelated change? > > Please fix and re-submit. Ah, my bad. Thanks for pointing out. Thanks, Pankaj > > Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v2 5/5] drm/i915: Enable scaling filter for plane and CRTC
> -Original Message-
> From: Ville Syrjälä
> Sent: 23 March 2020 20:18
> To: Laxminarayan Bharadiya, Pankaj
>
> Cc: Lattannavar, Sameer ;
> jani.nik...@linux.intel.com; dan...@ffwll.ch; intel-...@lists.freedesktop.org;
> dri-devel@lists.freedesktop.org; dani...@collabora.com; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> David Airlie ; Chris Wilson ;
> Maarten Lankhorst ; Souza, Jose
> ; Deak, Imre ; Shankar, Uma
>
> Subject: Re: [PATCH v2 5/5] drm/i915: Enable scaling filter for plane and CRTC
>
> On Thu, Mar 19, 2020 at 03:51:03PM +0530, Pankaj Bharadiya wrote:
> > GEN >= 10 hardware supports the programmable scaler filter.
> >
> > Attach scaling filter property for CRTC and plane for GEN >= 10
> > hardwares and program scaler filter based on the selected filter type.
> >
> > changes since v1:
> > * None
> > Changes since RFC:
> > * Enable properties for GEN >= 10 platforms (Ville)
> > * Do not round off the crtc co-ordinate (Danial Stone, Ville)
> > * Add new functions to handle scaling filter setup (Ville)
> > * Remove coefficient set 0 hardcoding.
> >
> > Signed-off-by: Shashank Sharma
> > Signed-off-by: Ankit Nautiyal
> > Signed-off-by: Pankaj Bharadiya
> >
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 32
> > ++-- drivers/gpu/drm/i915/display/intel_sprite.c |
> > 31 ++-
> > 2 files changed, 60 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 791dd908aa89..4b3387ee332e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6309,6 +6309,25 @@ void
> skl_scaler_setup_nearest_neighbor_filter(struct drm_i915_private *dev_priv,
> > }
> > }
> >
> > +static u32
> > +skl_scaler_crtc_setup_filter(struct drm_i915_private *dev_priv, enum pipe
> pipe,
> > + int id, int set, enum drm_crtc_scaling_filter filter)
> > {
> > + u32 scaler_filter_ctl = PS_FILTER_MEDIUM;
> > +
> > + if (filter == DRM_CRTC_SCALING_FILTER_NEAREST_NEIGHBOR) {
> > + skl_scaler_setup_nearest_neighbor_filter(dev_priv, pipe, id,
> > +set);
> > + scaler_filter_ctl = PS_FILTER_PROGRAMMED |
> > + PS_UV_VERT_FILTER_SELECT(set) |
> > + PS_UV_HORZ_FILTER_SELECT(set) |
> > + PS_Y_VERT_FILTER_SELECT(set) |
> > + PS_Y_HORZ_FILTER_SELECT(set);
> > +
> > + }
> > + return scaler_filter_ctl;
>
> This function does too many things.
I was thinking to have a common function which configures the filter and also
provides the register bits (ps_ctrl) to select a desired filter so that we need
not have extra condition to figure out filter select register bits where this
function is being called.
How about renaming this function to some better name like
skl_scaler_set_and_get_filter_select() or something else?
Or shall I breakdown this function into multiple functions?
Any suggestions?
>
> > +}
> > +
> > static void skl_pfit_enable(const struct intel_crtc_state
> > *crtc_state) {
> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > @@ -6316,12 +6335,14 @@ static void skl_pfit_enable(const struct
> intel_crtc_state *crtc_state)
> > enum pipe pipe = crtc->pipe;
> > const struct intel_crtc_scaler_state *scaler_state =
> > _state->scaler_state;
> > + const struct drm_crtc_state *state = _state->uapi;
>
> Pls don't add this kind of aliases. We're moving away from using the drm_
> types
> as much as possible.
OK.
>
> >
> > if (crtc_state->pch_pfit.enabled) {
> > u16 uv_rgb_hphase, uv_rgb_vphase;
> > int pfit_w, pfit_h, hscale, vscale;
> > unsigned long irqflags;
> > int id;
> > + int scaler_filter_ctl;
>
> It's a register value so u32. I'd also
Yes, I missed it. Thanks for pointing out.
> s/scaler_filter_ctl/filter_select/ or something like that.
>
> Alternatively we could just call it ps_ctrl and have it contain the full
> register
> value for that particular register.
ps_ctrl sounds better, will use this name.
>
> >
> > if (drm_WARN_ON(_priv->drm,
> > crtc_state->scaler_state.scaler_id < 0)) @@ -
> 6340,8 +636
RE: [PATCH v2 3/5] drm/i915: Introduce scaling filter related registers and bit fields.
> -Original Message-
> From: Ville Syrjälä
> Sent: 23 March 2020 20:09
> To: Laxminarayan Bharadiya, Pankaj
>
> Cc: Lattannavar, Sameer ;
> jani.nik...@linux.intel.com; dan...@ffwll.ch; intel-...@lists.freedesktop.org;
> dri-devel@lists.freedesktop.org; dani...@collabora.com; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> David Airlie
> Subject: Re: [PATCH v2 3/5] drm/i915: Introduce scaling filter related
> registers
> and bit fields.
>
> On Thu, Mar 19, 2020 at 03:51:01PM +0530, Pankaj Bharadiya wrote:
> > Introduce scaler registers and bit fields needed to configure the
> > scaling filter in prgrammed mode and configure scaling filter
> > coefficients.
> >
> > changes since v1:
> > * None
> > changes since RFC:
> > * Parametrize scaler coeffient macros by 'set' (Ville)
> >
> > Signed-off-by: Shashank Sharma
> > Signed-off-by: Ankit Nautiyal
> > Signed-off-by: Pankaj Bharadiya
> >
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 48
> > +
> > 1 file changed, 48 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 9c53fe918be6..d40f12d2a6b5
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7205,6 +7205,7 @@ enum {
> > #define PS_PLANE_SEL(plane) (((plane) + 1) << 25)
> > #define PS_FILTER_MASK (3 << 23)
> > #define PS_FILTER_MEDIUM (0 << 23)
> > +#define PS_FILTER_PROGRAMMED (1 << 23)
> > #define PS_FILTER_EDGE_ENHANCE (2 << 23)
> > #define PS_FILTER_BILINEAR (3 << 23)
> > #define PS_VERT3TAP(1 << 21)
> > @@ -7219,6 +7220,10 @@ enum {
> > #define PS_VADAPT_MODE_MOST_ADAPT (3 << 5) #define
> > PS_PLANE_Y_SEL_MASK (7 << 5) #define PS_PLANE_Y_SEL(plane) (((plane)
> > + 1) << 5)
> > +#define PS_Y_VERT_FILTER_SELECT(set) ((set) << 4)
> > +#define PS_Y_HORZ_FILTER_SELECT(set) ((set) << 3)
> > +#define PS_UV_VERT_FILTER_SELECT(set) ((set) << 2) #define
> > +PS_UV_HORZ_FILTER_SELECT(set) ((set) << 1)
> >
> > #define _PS_PWR_GATE_1A 0x68160
> > #define _PS_PWR_GATE_2A 0x68260
> > @@ -7281,6 +7286,25 @@ enum {
> > #define _PS_ECC_STAT_2B 0x68AD0
> > #define _PS_ECC_STAT_1C 0x691D0
> >
> > +#define _PS_COEF_SET0_INDEX_1A0x68198
> > +#define _PS_COEF_SET0_INDEX_2A0x68298
> > +#define _PS_COEF_SET0_INDEX_1B0x68998
> > +#define _PS_COEF_SET0_INDEX_2B0x68A98
> > +#define _PS_COEF_SET1_INDEX_1A0x681A0
> > +#define _PS_COEF_SET1_INDEX_2A0x682A0
> > +#define _PS_COEF_SET1_INDEX_1B0x689A0
> > +#define _PS_COEF_SET1_INDEX_2B0x68AA0
> > +#define PS_COEE_INDEX_AUTO_INC(1 << 10)
> > +
> > +#define _PS_COEF_SET0_DATA_1A 0x6819C
> > +#define _PS_COEF_SET0_DATA_2A 0x6829C
> > +#define _PS_COEF_SET0_DATA_1B 0x6899C
> > +#define _PS_COEF_SET0_DATA_2B 0x68A9C
> > +#define _PS_COEF_SET1_DATA_1A 0x681A4
> > +#define _PS_COEF_SET1_DATA_2A 0x682A4
> > +#define _PS_COEF_SET1_DATA_1B 0x689A4
> > +#define _PS_COEF_SET1_DATA_2B 0x68AA4
> > +
> > #define _ID(id, a, b) _PICK_EVEN(id, a, b)
> > #define SKL_PS_CTRL(pipe, id) _MMIO_PIPE(pipe,\
> > _ID(id, _PS_1A_CTRL, _PS_2A_CTRL), \
> > @@ -7310,6 +7334,30 @@ enum {
> > _ID(id, _PS_ECC_STAT_1A, _PS_ECC_STAT_2A), \
> > _ID(id, _PS_ECC_STAT_1B, _PS_ECC_STAT_2B))
> >
> > +#define _SKL_PS_COEF_INDEX_SET0(pipe, id) _ID(pipe,\
> > + _ID(id, _PS_COEF_SET0_INDEX_1A,
> _PS_COEF_SET0_INDEX_2A), \
> > + _ID(id, _PS_COEF_SET0_INDEX_1B,
> _PS_COEF_SET0_INDEX_2B))
> > +
> > +#define _SKL_PS_COEF_INDEX_SET1(pipe, id) _ID(pipe,\
> > + _ID(id, _PS_COEF_SET1_INDEX_1A,
> _PS_COEF_SET1_INDEX_2A), \
> > + _ID(id, _PS_COEF_SET1_INDEX_1B,
> _PS_COEF_SET1_INDEX_2B))
> > +
> > +#define _SKL_PS_COEF_DATA_SET0(pipe, id) _ID(pipe, \
> > + _ID(id, _PS_COEF_SET0_DATA_1A,
> _PS_COEF_SET0_DATA_2A), \
> > + _ID(id, _PS_COEF_SET0_DATA_1B,
> _PS_COEF_SET0_DATA_2B))
> > +
> > +#define _SKL_PS_COEF_DATA_SET1(pipe, id) _ID(pipe, \
> > + _ID(id, _PS_COEF_SET1_DATA_1A,
> _PS_COEF_SET1_DATA_2A), \
> >
RE: [PATCH v2 1/5] drm: Introduce plane and CRTC scaling filter properties
> -Original Message-
> From: Ville Syrjälä
> Sent: 23 March 2020 19:52
> To: Laxminarayan Bharadiya, Pankaj
>
> Cc: Lattannavar, Sameer ;
> jani.nik...@linux.intel.com; dan...@ffwll.ch; intel-...@lists.freedesktop.org;
> dri-devel@lists.freedesktop.org; dani...@collabora.com; Maarten Lankhorst
> ; Maxime Ripard ;
> Thomas Zimmermann ; David Airlie
> Subject: Re: [PATCH v2 1/5] drm: Introduce plane and CRTC scaling filter
> properties
>
> On Thu, Mar 19, 2020 at 03:50:59PM +0530, Pankaj Bharadiya wrote:
> > Introduce new plane and CRTC scaling filter properties to allow
> > userspace to select the driver's default scaling filter or
> > Nearest-neighbor(NN) filter for upscaling operations on CRTC and
> > plane.
> >
> > Drivers can set up this property for a plane by calling
> > drm_plane_enable_scaling_filter() and for a CRTC by calling
> > drm_crtc_enable_scaling_filter().
> >
> > NN filter works by filling in the missing color values in the upscaled
> > image with that of the coordinate-mapped nearest source pixel value.
> >
> > NN filter for integer multiple scaling can be particularly useful for
> > for pixel art games that rely on sharp, blocky images to deliver their
> > distinctive look.
> >
> > changes since v1:
> > * None
> > changes since RFC:
> > * Add separate properties for plane and CRTC (Ville)
>
> I actually meant the prop should be per-object, not just separate prop for
> planes
> and crtcs.
My bad, I misunderstood it ☹.
Will add drm_property in drm_crtc and drm_plane.
Thanks,
Pankaj
>
> >
> > Signed-off-by: Shashank Sharma
> > Signed-off-by: Ankit Nautiyal
> > Signed-off-by: Pankaj Bharadiya
> >
> > ---
> > drivers/gpu/drm/drm_atomic_uapi.c | 8
> > drivers/gpu/drm/drm_crtc.c| 33 +++
> > drivers/gpu/drm/drm_mode_config.c | 26
> > drivers/gpu/drm/drm_plane.c | 33 +++
> > include/drm/drm_crtc.h| 13
> > include/drm/drm_mode_config.h | 12 +++
> > include/drm/drm_plane.h | 13
> > 7 files changed, 138 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index a1e5e262bae2..3c72ab52ff62 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct
> drm_crtc *crtc,
> > return ret;
> > } else if (property == config->prop_vrr_enabled) {
> > state->vrr_enabled = val;
> > + } else if (property == config->crtc_scaling_filter_property) {
> > + state->scaling_filter = val;
> > } else if (property == config->degamma_lut_property) {
> > ret = drm_atomic_replace_property_blob_from_id(dev,
> > >degamma_lut,
> > @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> > *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> > else if (property == config->prop_out_fence_ptr)
> > *val = 0;
> > + else if (property == config->crtc_scaling_filter_property)
> > + *val = state->scaling_filter;
> > else if (crtc->funcs->atomic_get_property)
> > return crtc->funcs->atomic_get_property(crtc, state, property,
> val);
> > else
> > @@ -583,6 +587,8 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
> > sizeof(struct drm_rect),
> > );
> > return ret;
> > + } else if (property == config->plane_scaling_filter_property) {
> > + state->scaling_filter = val;
> > } else if (plane->funcs->atomic_set_property) {
> > return plane->funcs->atomic_set_property(plane, state,
> > property, val);
> > @@ -641,6 +647,8 @@ drm_atomic_plane_get_property(struct drm_plane
> *plane,
> > } else if (property == config->prop_fb_damage_clips) {
> > *val = (state->fb_damage_clips) ?
> > state->fb_damage_clips->base.id : 0;
> > + } else if (property == config->plane_scaling_filter_property) {
> > + *val = state->scaling_filter;
> > } else if (plane->funcs->atomic_get_property) {
> > return plane->fu
RE: [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support
> -Original Message- > From: Ville Syrjälä > Sent: 12 March 2020 19:25 > To: Laxminarayan Bharadiya, Pankaj > > Cc: jani.nik...@linux.intel.com; dan...@ffwll.ch; intel- > g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airl...@linux.ie; > maarten.lankho...@linux.intel.com; tzimmerm...@suse.de; > mrip...@kernel.org; mihail.atanas...@arm.com; Joonas Lahtinen > ; Vivi, Rodrigo ; > Chris Wilson ; Souza, Jose ; > De Marchi, Lucas ; Roper, Matthew D > ; Deak, Imre ; Shankar, > Uma ; linux-ker...@vger.kernel.org; Nautiyal, Ankit K > > 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 +, Laxminarayan Bharadiya, Pankaj > wrote: > > > > > > > -Original Message- > > > From: Ville Syrjälä > > > Sent: 10 March 2020 21:47 > > > To: Laxminarayan Bharadiya, Pankaj > > > > > > Cc: jani.nik...@linux.intel.com; dan...@ffwll.ch; intel- > > > g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; > > > airl...@linux.ie; maarten.lankho...@linux.intel.com; > > > tzimmerm...@suse.de; mrip...@kernel.org; mihail.atanas...@arm.com; > > > Joonas Lahtinen ; Vivi, Rodrigo > > > ; Chris Wilson ; > > > Souza, Jose ; De Marchi, Lucas > > > ; Roper, Matthew D > > > ; Deak, Imre ; > > > Shankar, Uma ; linux- ker...@vger.kernel.org; > > > Nautiyal, Ankit K > > > 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 > > > > > > > > Signed-off-by: Ankit Nautiyal > > > > --- > > > > 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 | > > > > + * > > > > ++-
RE: [RFC][PATCH 0/5] Introduce drm scaling filter property
> -Original Message- > From: Ville Syrjälä > Sent: 12 March 2020 19:35 > To: Laxminarayan Bharadiya, Pankaj > > Cc: jani.nik...@linux.intel.com; dan...@ffwll.ch; intel- > g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airl...@linux.ie; > maarten.lankho...@linux.intel.com; tzimmerm...@suse.de; > mrip...@kernel.org; mihail.atanas...@arm.com; linux- > ker...@vger.kernel.org; Nautiyal, Ankit K > Subject: Re: [RFC][PATCH 0/5] Introduce drm scaling filter property > > On Tue, Feb 25, 2020 at 12:35:40PM +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. In > > contrast, traditional upscaling algorithms, such as bilinear or > > bicubic interpolation, result in blurry upscaled images because they > > employ interpolation techniques that smooth out the transition from > > one pixel to another. Therefore, integer scaling is particularly > > useful for pixel art games that rely on sharp, blocky images to > > deliver their distinctive look. > > > > Many gaming communities have been asking for integer-mode scaling > > support, some links and background: > > > > https://software.intel.com/en-us/articles/integer-scaling-support-on-i > > ntel-graphics http://tanalin.com/en/articles/lossless-scaling/ > > https://community.amd.com/thread/209107 > > https://www.nvidia.com/en-us/geforce/forums/game-ready-drivers/13/1002 > > /feature-request-nonblurry-upscaling-at-integer-rat/ > > > > This patch series - > > - Introduces new scaling filter property to allow userspace to > > select the driver's default scaling filter or Nearest-neighbor(NN) > > filter for scaling operations on crtc/plane. > > - Implements and enable integer scaling for i915 > > > > Userspace patch series link: TBD. > > That needs to be done or this will go nowhere. Yes, Sameer is working on enabling this feature in Kodi. Sameer, please share link here once you post patches. Thanks, Pankaj > > > > > Thanks to Shashank for initiating this work. His initial RFC can be > > found here [1] > > > > [1] https://patchwork.freedesktop.org/patch/337082/ > > > > Modifications done in this series - > >- refactored code and incorporated initial review comments and > > added 2 scaling filter types (default and NN) to begin with. > >- added scaling filter property support for planes and new API > > helpers for drivers to setup this property. > >- rewrote code to enable integer scaling and NN filter for i915 > > > > > > Pankaj Bharadiya (5): > > drm: Introduce scaling filter property > > drm/drm-kms.rst: Add Scaling filter property documentation > > drm/i915: Enable scaling filter for plane and pipe > > drm/i915: Introduce scaling filter related registers and bit fields. > > drm/i915/display: Add Nearest-neighbor based integer scaling support > > > > Documentation/gpu/drm-kms.rst| 6 ++ > > drivers/gpu/drm/drm_atomic_uapi.c| 8 ++ > > drivers/gpu/drm/drm_crtc.c | 16 +++ > > drivers/gpu/drm/drm_mode_config.c| 13 +++ > > drivers/gpu/drm/drm_plane.c | 35 +++ > > drivers/gpu/drm/i915/display/intel_display.c | 100 ++- > > drivers/gpu/drm/i915/display/intel_display.h | 2 + > > drivers/gpu/drm/i915/display/intel_sprite.c | 32 -- > > drivers/gpu/drm/i915/i915_reg.h | 21 > > include/drm/drm_crtc.h | 10 ++ > > include/drm/drm_mode_config.h| 6 ++ > > include/drm/drm_plane.h | 14 +++ > > 12 files changed, 252 insertions(+), 11 deletions(-) > > > > -- > > 2.23.0 > > -- > Ville Syrjälä > Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support
> -Original Message- > From: Ville Syrjälä > Sent: 10 March 2020 21:47 > To: Laxminarayan Bharadiya, Pankaj > > Cc: jani.nik...@linux.intel.com; dan...@ffwll.ch; intel- > g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airl...@linux.ie; > maarten.lankho...@linux.intel.com; tzimmerm...@suse.de; > mrip...@kernel.org; mihail.atanas...@arm.com; Joonas Lahtinen > ; Vivi, Rodrigo ; > Chris Wilson ; Souza, Jose > ; De Marchi, Lucas ; > Roper, Matthew D ; Deak, Imre > ; Shankar, Uma ; linux- > ker...@vger.kernel.org; Nautiyal, Ankit K > 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 > > > > Signed-off-by: Ankit Nautiyal > > --- > > 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 cente
RE: [RFC][PATCH 3/5] drm/i915: Enable scaling filter for plane and pipe
> -Original Message-
> From: Ville Syrjälä
> Sent: 10 March 2020 21:36
> To: Laxminarayan Bharadiya, Pankaj
>
> Cc: jani.nik...@linux.intel.com; dan...@ffwll.ch; intel-
> g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airl...@linux.ie;
> maarten.lankho...@linux.intel.com; tzimmerm...@suse.de;
> mrip...@kernel.org; mihail.atanas...@arm.com; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> Chris Wilson ; Souza, Jose
> ; Juha-Pekka Heikkila
> ; linux-ker...@vger.kernel.org; Nautiyal,
> Ankit K
> Subject: Re: [RFC][PATCH 3/5] drm/i915: Enable scaling filter for plane and
> pipe
>
> On Tue, Feb 25, 2020 at 12:35:43PM +0530, Pankaj Bharadiya wrote:
> > Attach scaling filter property for crtc and plane and program the
> > scaler control register for the selected filter type.
> >
> > This is preparatory patch to enable Nearest-neighbor integer scaling.
> >
> > Signed-off-by: Pankaj Bharadiya
> >
> > Signed-off-by: Ankit Nautiyal
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 17 +++--
> > drivers/gpu/drm/i915/display/intel_sprite.c | 12 +++-
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > 3 files changed, 27 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 3031e64ee518..b5903ef3c5a0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6242,6 +6242,8 @@ static void skl_pfit_enable(const struct
> intel_crtc_state *crtc_state)
> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > enum pipe pipe = crtc->pipe;
> > + const struct drm_crtc_state *state = _state->uapi;
> > + u32 scaling_filter = PS_FILTER_MEDIUM;
> > const struct intel_crtc_scaler_state *scaler_state =
> > _state->scaler_state;
> >
> > @@ -6258,6 +6260,11 @@ static void skl_pfit_enable(const struct
> intel_crtc_state *crtc_state)
> > pfit_w = (crtc_state->pch_pfit.size >> 16) & 0x;
> > pfit_h = crtc_state->pch_pfit.size & 0x;
> >
> > + if (state->scaling_filter ==
> > + DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
> > + scaling_filter = PS_FILTER_PROGRAMMED;
> > + }
>
> Just make that a function that can be used all over.
> skl_scaler_filter(scaling_filter) or something.
>
> > +
> > hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> > vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
> >
> > @@ -6268,8 +6275,10 @@ static void skl_pfit_enable(const struct
> > intel_crtc_state *crtc_state)
> >
> > spin_lock_irqsave(_priv->uncore.lock, irqflags);
> >
> > - intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id),
> PS_SCALER_EN |
> > - PS_FILTER_MEDIUM | scaler_state-
> >scalers[id].mode);
> > + intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id),
> > + PS_SCALER_EN |
> > + scaling_filter |
> > + scaler_state->scalers[id].mode);
> > intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
> > PS_Y_PHASE(0) |
> PS_UV_RGB_PHASE(uv_rgb_vphase));
> > intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id), @@
> -16695,6
> > +16704,10 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv,
> enum pipe pipe)
> > dev_priv->plane_to_crtc_mapping[i9xx_plane] = crtc;
> > }
> >
> > +
> > + if (INTEL_GEN(dev_priv) >= 11)
>
> gen >= 10 actually. Even glk seems to have it but bspec says not to use it on
> glk. Supposedly not validated.
>
> ilk/snb/ivb pfits also has programmable coefficients actually. So IMO we
> should enable this on those as well.
OK. I need to explore bspec more for these platforms.
To begin with I would like to stick to gen >=10.
>
> The bigger problem will be how is userspace supposed to use this if it's a
> crtc
> property? Those will not get automagically exposed via xrandr.
>
> > + drm_crtc_enable_scaling_filter(>base);
> > +
> > intel_color_init(crtc);
> >
> > drm_WARN_ON(_priv->drm, drm_crtc_index(>base) !=
> > crtc->pipe); diff --git a/d
RE: [Intel-gfx] [PATCH v2 0/7] drm: drm_fb_helper cleanup.
> -Original Message- > From: Emil Velikov > Sent: 02 March 2020 23:51 > To: Laxminarayan Bharadiya, Pankaj > > Cc: Jani Nikula ; Daniel Vetter > ; Intel Graphics Development g...@lists.freedesktop.org>; ML dri-devel > Subject: Re: [Intel-gfx] [PATCH v2 0/7] drm: drm_fb_helper cleanup. > > Hi Pankaj, > > On Mon, 2 Mar 2020 at 16:33, Pankaj Bharadiya > wrote: > > > > This series addresses below drm_fb_helper tasks from > > Documentation/gpu/todo.rst. > > > > - The max connector argument for drm_fb_helper_init() isn't used > > anymore and can be removed. > > > > - The helper doesn't keep an array of connectors anymore so these can > > be removed: drm_fb_helper_single_add_all_connectors(), > > drm_fb_helper_add_one_connector() and > > drm_fb_helper_remove_one_connector(). > > > > Changes since v1: > >- Squashed warning fixes into the patch that introduced the > > warnings (into 5/7) (Laurent) > >- Fixed reflow in in 9/9 (Laurent) > > > For the future, include the changelog in the respective patches. This makes it > easier for reviewers... > Plus you're already changing the commit - might as well mention what/why :- > ) > > Also do include the R-B, Acked-by, other tags accumulated up-to that point, > when sending new revision. Noted, Thank you for the feedback. Will send new series with tags accumulated after 1-2 days. > > > That said, if you're interested in further cleaning this up, one can cleanup > the > drm_dp_mst_topology_cbs hooks. > In particular ::register_connector is identical across the board - create a > helper function using it directly in core, killing the hook. > > While for ::destroy_connector - there's some amdgpu specific code in > there... which I'm not sure if it should stay or not. > To be on the save side - create a helper which will be called for drivers > where > the hook is !=NULL (aka everyone but amdgpu). Will take a look. Thanks, Pankaj > > HTH > Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Intel-gfx] [PATCH v2 0/7] drm: drm_fb_helper cleanup.
> -Original Message- > From: Daniel Vetter > Sent: 03 March 2020 03:15 > To: Emil Velikov > Cc: Laxminarayan Bharadiya, Pankaj > ; Jani Nikula > ; Daniel Vetter ; Intel > Graphics Development ; ML dri-devel de...@lists.freedesktop.org> > Subject: Re: [Intel-gfx] [PATCH v2 0/7] drm: drm_fb_helper cleanup. > > On Mon, Mar 02, 2020 at 10:43:19PM +0100, Daniel Vetter wrote: > > On Mon, Mar 02, 2020 at 06:21:23PM +, Emil Velikov wrote: > > > Hi Pankaj, > > > > > > On Mon, 2 Mar 2020 at 16:33, Pankaj Bharadiya > > > wrote: > > > > > > > > This series addresses below drm_fb_helper tasks from > > > > Documentation/gpu/todo.rst. > > > > > > > > - The max connector argument for drm_fb_helper_init() isn't used > > > > anymore and can be removed. > > > > > > > > - The helper doesn't keep an array of connectors anymore so these can > > > > be removed: drm_fb_helper_single_add_all_connectors(), > > > > drm_fb_helper_add_one_connector() and > > > > drm_fb_helper_remove_one_connector(). > > > > > > > > Changes since v1: > > > >- Squashed warning fixes into the patch that introduced the > > > > warnings (into 5/7) (Laurent) > > > >- Fixed reflow in in 9/9 (Laurent) > > > > > > > For the future, include the changelog in the respective patches. > > > This makes it easier for reviewers... > > > Plus you're already changing the commit - might as well mention > > > what/why :-) > > > > > > Also do include the R-B, Acked-by, other tags accumulated up-to that > > > point, when sending new revision. > > > > Yup, can you pls resend that entire pile with all the ack/review tags > > from the earlier versions included? If you don't do that you waste > > reviewers time. Another one is that resend right away also kinda > > wastes peoples time, because there's a much bigger chance that someone > > will review the old version, instead of your new one. Better wait of > > at least 1-2 days or so. > > Hm just noticed that people are still reviewing v1. I'd say lets forget about > this v2 here, wait 1-2 days, and then resend with everything combined. > Hopefully not too many will re-review v2 here and waste time on stuff that's > reviewed already. Resending within hours is really not good on mailing lists > (with merge requests or whatever it doesn't matter, because everyone > always looks at the latest version). Noted . Thanks for the feedback. Will send the new series with tags accumulated after 1-2 days. Thanks, Pankaj > -Daniel > > > > > > That said, if you're interested in further cleaning this up, one can > > > cleanup the drm_dp_mst_topology_cbs hooks. > > > In particular ::register_connector is identical across the board - > > > create a helper function using it directly in core, killing the hook. > > > > > > While for ::destroy_connector - there's some amdgpu specific code in > > > there... which I'm not sure if it should stay or not. > > > To be on the save side - create a helper which will be called for > > > drivers where the hook is !=NULL (aka everyone but amdgpu). > > > > Yeah that stuff looks fishy. Smells a bit like old mst code developed > > before Lyude fixed the refcounting for real, it seems to manually shut > > down stuff that should be cleaned up with normal code paths (modeset > > and/or final kref_put on the connector). > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Intel-gfx] [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support
> -Original Message-
> From: Daniel Stone
> Sent: 25 February 2020 13:00
> To: Laxminarayan Bharadiya, Pankaj
>
> Cc: Jani Nikula ; Daniel Vetter
> ; intel-gfx ; dri-devel
> ; Ville Syrjälä
> ; David Airlie ; Maarten
> Lankhorst ; tzimmerm...@suse.de;
> Maxime Ripard ; mihail.atanas...@arm.com; Joonas
> Lahtinen ; Vivi, Rodrigo
> ; Chris Wilson ; Souza,
> Jose ; De Marchi, Lucas
> ; Roper, Matthew D
> ; Deak, Imre ;
> Shankar, Uma ; Nautiyal, Ankit K
> ; Linux Kernel Mailing List ker...@vger.kernel.org>
> Subject: Re: [Intel-gfx] [RFC][PATCH 5/5] drm/i915/display: Add Nearest-
> neighbor based integer scaling support
>
> Hi,
>
> On Tue, 25 Feb 2020 at 07:17, Pankaj Bharadiya
> wrote:
> > @@ -415,18 +415,26 @@ skl_program_scaler(struct intel_plane *plane,
> > u16 y_vphase, uv_rgb_vphase;
> > int hscale, vscale;
> > const struct drm_plane_state *state = _state->uapi;
> > + u32 src_w = drm_rect_width(_state->uapi.src) >> 16;
> > + u32 src_h = drm_rect_height(_state->uapi.src) >> 16;
> > u32 scaling_filter = PS_FILTER_MEDIUM;
> > + struct drm_rect dst;
> >
> > if (state->scaling_filter ==
> DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
> > scaling_filter = PS_FILTER_PROGRAMMED;
> > + skl_setup_nearest_neighbor_filter(dev_priv, pipe,
> > + scaler_id);
> > +
> > + /* Make the scaling window size to integer multiple of
> > source
> > +* TODO: Should userspace take desision to round scaling
> > window
> > +* to integer multiple?
> > +*/
> > + crtc_w = rounddown(crtc_w, src_w);
> > + crtc_h = rounddown(crtc_h, src_h);
>
> The kernel should absolutely not be changing the co-ordinates that
> userspace requested.
Thanks, Will get rid of this in V2.
Thanks,
Pankaj
>
> Cheers,
> Daniel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Intel-gfx][PATCH 01/10] drm/i915: Add i915 device based MISSING_CASE macro
> -Original Message- > From: Jani Nikula > Sent: 27 February 2020 14:00 > To: Laxminarayan Bharadiya, Pankaj > ; Chris Wilson wilson.co.uk> > Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; David > Airlie > ; Joonas Lahtinen ; Vivi, > Rodrigo ; dan...@ffwll.ch > Subject: RE: [Intel-gfx][PATCH 01/10] drm/i915: Add i915 device based > MISSING_CASE macro > > On Thu, 27 Feb 2020, "Laxminarayan Bharadiya, Pankaj" >wrote: > > Hi Chris, > > > >> -Original Message- > >> From: Chris Wilson > >> Sent: 25 February 2020 19:32 > >> To: David Airlie ; Joonas Lahtinen > >> ; Laxminarayan Bharadiya, Pankaj > >> ; Vivi, Rodrigo > >> ; dan...@ffwll.ch; > >> dri-devel@lists.freedesktop.org; intel-...@lists.freedesktop.org; > >> jani.nik...@linux.intel.com > >> Cc: Laxminarayan Bharadiya, Pankaj > >> > >> Subject: Re: [Intel-gfx][PATCH 01/10] drm/i915: Add i915 device based > >> MISSING_CASE macro > >> > >> Quoting Pankaj Bharadiya (2020-02-25 13:47:00) > >> > Now that we have struct drm_device based drm_WARN, introduce struct > >> > drm_i915_private based i915_MISSING_CASE macro which uses > >> drm_WARN so > >> > that device specific information will also get printed in backtrace. > >> > > >> > i915_MISSING_CASE macro should be preferred over MISSING_CASE, > >> > wherever possible. > >> > >> Whatever for? MISSING_CASE() itself should be a complete picture for > >> the forgotten code. > > > > Are you saying, no need to have a new device specific macro? > > > > We want convert all the calls of WARN* with device specific drm_WARN* > > in i915, hence I introduced new i915_MISSING_CASE macro. > > > > Jani, Will you please share your opinion on this? > > In general, many or most WARNs are device specific, and the device information > is useful. However MISSING_CASE is about the *code*. That was the intent > anyway. Perhaps there are cases where the device information might be useful, > but for most cases probably not. Thanks for clarification. Please ignore this patch series then. Thanks, Pankaj > > BR, > Jani. > > > -- > Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Intel-gfx][PATCH 01/10] drm/i915: Add i915 device based MISSING_CASE macro
Hi Chris, > -Original Message- > From: Chris Wilson > Sent: 25 February 2020 19:32 > To: David Airlie ; Joonas Lahtinen > ; Laxminarayan Bharadiya, Pankaj > ; Vivi, Rodrigo > ; dan...@ffwll.ch; dri-devel@lists.freedesktop.org; > intel-...@lists.freedesktop.org; jani.nik...@linux.intel.com > Cc: Laxminarayan Bharadiya, Pankaj > > Subject: Re: [Intel-gfx][PATCH 01/10] drm/i915: Add i915 device based > MISSING_CASE macro > > Quoting Pankaj Bharadiya (2020-02-25 13:47:00) > > Now that we have struct drm_device based drm_WARN, introduce struct > > drm_i915_private based i915_MISSING_CASE macro which uses > drm_WARN so > > that device specific information will also get printed in backtrace. > > > > i915_MISSING_CASE macro should be preferred over MISSING_CASE, > > wherever possible. > > Whatever for? MISSING_CASE() itself should be a complete picture for the > forgotten code. Are you saying, no need to have a new device specific macro? We want convert all the calls of WARN* with device specific drm_WARN* in i915, hence I introduced new i915_MISSING_CASE macro. Jani, Will you please share your opinion on this? Thanks, Pankaj > -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [RFC][PATCH 1/5] drm: Introduce scaling filter property
> -Original Message- > From: Jani Nikula > Sent: 25 February 2020 15:26 > To: Laxminarayan Bharadiya, Pankaj > ; dan...@ffwll.ch; intel- > g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; > ville.syrj...@linux.intel.com; airl...@linux.ie; > maarten.lankho...@linux.intel.com; tzimmerm...@suse.de; > mrip...@kernel.org; mihail.atanas...@arm.com > Cc: Laxminarayan Bharadiya, Pankaj > ; linux-ker...@vger.kernel.org; > Nautiyal, Ankit K > Subject: Re: [RFC][PATCH 1/5] drm: Introduce scaling filter property > > On Tue, 25 Feb 2020, Pankaj Bharadiya > wrote: > > Signed-off-by: Pankaj Bharadiya > > > > Signed-off-by: Shashank Sharma > > Signed-off-by: Ankit Nautiyal > > What did Shashank and Ankit do, should one or the other perhaps retain > authorship? I kind of refactored the code, added plane scaling support added new APIs & documentation and rewrote the scaling filter setting logic. Since I made significant changes (IMHO), I thought of changing the authorship. I spoke with Ankit regarding this during my initial discussion with him. Will you please review the present RFC and the older one and suggest. I have no issues with changing the authorship . > > In any case, when taking over code and submitting, you should add your > sign-off > *last*. Please see [1] for what Signed-off-by means. Was not aware, will do the needful. Thanks, Pankaj > > BR, > Jani. > > > [1] https://developercertificate.org/ > > -- > Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
