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

From: "Srinivas, Vidya" <vidya.srinivas@intel.com>
To: "Sharma, Shashank" <shashank.sharma@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 10/16] drm/i915: Set scaler mode for NV12
Date: Thu, 15 Feb 2018 09:28:17 +0000	[thread overview]
Message-ID: <F653A0A18852B74D88578FA2EB7094EAB682416F@BGSMSX108.gar.corp.intel.com> (raw)
In-Reply-To: <4d8cddc2-4cac-1653-b7f9-7f3b3821d986@intel.com>


[-- Attachment #1.1: Type: text/plain, Size: 4495 bytes --]

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@intel.com>; intel-gfx@lists.freedesktop.org
Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>; Konduru, Chandra <chandra.konduru@intel.com>; Maiti, Nabendu Bikash <nabendu.bikash.maiti@intel.com>
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@intel.com><mailto:chandra.konduru@intel.com>



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@intel.com><mailto:clinton.a.taylor@intel.com>

Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com><mailto:clinton.a.taylor@intel.com>

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com><mailto:chandra.konduru@intel.com>

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com><mailto:nabendu.bikash.maiti@intel.com>

Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com><mailto:vidya.srinivas@intel.com>

---

 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,


[-- Attachment #1.2: Type: text/html, Size: 11811 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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