Re: [PATCH 10/16] drm/i915: Set scaler mode for NV12

["Srinivas, Vidya" <vidya.srinivas@xxxxxxxxx>]

I addressed it with PS_SCALER_MODE_PLANAR (1 << 29) to keep it generic.

But as you mentioned that wouldn’t be the right way.

I need to keep GEN9 and GEN10+ separate.

Will fix it in the next push. Thank you.

 

Vidya

 

 

From: Sharma, Shashank
Sent: Thursday, February 15, 2018 12:03 PM
To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: maarten.lankhorst@xxxxxxxxxxxxxxx; Kamath, Sunil <sunil.kamath@xxxxxxxxx>; Shankar, Uma <uma.shankar@xxxxxxxxx>; Konduru, Chandra <chandra.konduru@xxxxxxxxx>; Maiti, Nabendu Bikash <nabendu.bikash.maiti@xxxxxxxxx>
Subject: Re: [PATCH 10/16] drm/i915: Set scaler mode for NV12

 

My previous review comments are not addressed on this patch.

I made a comment which is about programming bits 29:28 on PS_CTRL for (GEN>=9) blindly.

This is not valid for all GEN9 and above, in fact bit 29:28 as scalar mode is only valid for SKL.

Bit 28 is not supported for GLK, bspec clearly mentions do not program this.

Also from GEN10 onward bit 28 is NOT for scaler mode, its for adaptive filtering.

Regards
Shashank

On 2/14/2018 10:27 AM, Vidya Srinivas wrote:

From: Chandra Konduru <chandra.konduru@xxxxxxxxx>

This patch sets appropriate scaler mode for NV12 format.

In this mode, skylake scaler does either chroma-upsampling or

chroma-upsampling and resolution scaling

v2: Review comments from Ville addressed

NV12 case to be checked first for setting

the scaler

v3: Rebased (me)

v4: Rebased (me)

v5: Missed the Tested-by/Reviewed-by in the previous series

Adding the same to commit message in this version.

v6: Rebased (me)

v7: Rebased (me)

v8: Rebased (me)

Restricting the NV12 change for scaler to BXT and KBL

in this series.

v9: Rebased (me)

v10: As of now, NV12 has been tested on Gen9 and Gen10. However,

code is applicable to all GEN >= 9. Hence making

that change to keep it generic.

Comments under v8 is not valid anymore.

v11: Addressed review comments by Shashank Sharma.

For Gen10+, the scaler mode to be set it planar or normal

(single bit). Changed the code to be applicable to all

Gen.

Tested-by: Clinton Taylor <clinton.a.taylor@xxxxxxxxx>

Reviewed-by: Clinton Taylor <clinton.a.taylor@xxxxxxxxx>

Signed-off-by: Chandra Konduru <chandra.konduru@xxxxxxxxx>

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@xxxxxxxxx>

Signed-off-by: Vidya Srinivas <vidya.srinivas@xxxxxxxxx>

---

 drivers/gpu/drm/i915/i915_reg.h     | 1 +

 drivers/gpu/drm/i915/intel_atomic.c | 8 ++++++--

 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h

index f6afa5e..10edacc 100644

--- a/drivers/gpu/drm/i915/i915_reg.h

+++ b/drivers/gpu/drm/i915/i915_reg.h

@@ -6732,6 +6732,7 @@ enum {

 #define PS_SCALER_MODE_MASK (3 << 28)

 #define PS_SCALER_MODE_DYN  (0 << 28)

 #define PS_SCALER_MODE_HQ  (1 << 28)

+#define PS_SCALER_MODE_PLANAR (1 << 29)

 #define PS_PLANE_SEL_MASK  (7 << 25)

 #define PS_PLANE_SEL(plane) (((plane) + 1) << 25)

 #define PS_FILTER_MASK         (3 << 23)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c

index d452c32..7ae4f48 100644

--- a/drivers/gpu/drm/i915/intel_atomic.c

+++ b/drivers/gpu/drm/i915/intel_atomic.c

@@ -327,8 +327,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,

                }

                /* set scaler mode */

-               if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {

-                       scaler_state->scalers[*scaler_id].mode = 0;

+               if ((INTEL_GEN(dev_priv) >= 9) &&

+                   plane_state && plane_state->base.fb &&

+                   plane_state->base.fb->format->format ==

+                   DRM_FORMAT_NV12) {

+                       scaler_state->scalers[*scaler_id].mode =

+                              PS_SCALER_MODE_PLANAR;

                } else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {

                        /*

                         * when only 1 scaler is in use on either pipe A or B,

 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx