Re: [Intel-gfx] [PATCH 15/18] drm/i915: Initialize Gen8 pipe gamma correction
Regards
Shashank
On 8/22/2015 4:11 AM, Matt Roper wrote:
On Thu, Aug 06, 2015 at 10:08:24PM +0530, Shashank Sharma wrote:
From: Kausal Malladi kausalmall...@gmail.com
This patch initializes gamma color correction proeprty
typo
Oops !
for Gen8 and higher platforms.
I'd specifically say 'BDW and Gen9' here since we already have some gen8
support (CHV).
Agree. Will change it.
It does the following :
1. Load pipe Gamma color correction capabilities for BDW/SKL/BXT
2. Attach the color properties to CRTC
Signed-off-by: Shashank Sharma shashank.sha...@intel.com
Signed-off-by: Kausal Malladi kausalmall...@gmail.com
---
drivers/gpu/drm/i915/intel_color_manager.c | 30 +-
drivers/gpu/drm/i915/intel_color_manager.h | 3 +++
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
b/drivers/gpu/drm/i915/intel_color_manager.c
index 5fa575b..bc77ab5 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -475,11 +475,39 @@ int get_chv_pipe_gamma_capabilities(struct drm_device
*dev,
return 0;
}
+int get_gen9_pipe_gamma_capabilities(struct drm_device *dev,
+ struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
Calling this 'gen9' seems a little confusing to me given that it's also
used for BDW, which is a gen8 platform. The general pattern is that
functions get named after the first platform that works a specific way,
so I'd expect this to be called get_bdw_pipe_gamma_capabilities.
Yes, its a mistake. I will fix this and will be more specific.
+{
+ struct drm_property_blob *blob = NULL;
+
+ /*
+* This function exposes best capability for DeGamma and Gamma
+* For BXT, the DeGamma LUT has 512 entries
+* and the best Gamma capability has 512 entries
+*/
+ palette_caps-version = GEN9_PALETTE_STRUCT_VERSION;
+ palette_caps-num_samples_before_ctm =
+ GEN9_SPLITGAMMA_MAX_VALS;
+ palette_caps-num_samples_after_ctm =
+ GEN9_SPLITGAMMA_MAX_VALS;
+
+ blob = drm_property_create_blob(dev, sizeof(struct drm_palette_caps),
+ (const void *) palette_caps);
We're pretty much doing the same thing we did for CHV, but just filling
in different values. Could we just stick the number of samples in
INTEL_INFO(dev)-num_gamma_samples_{before/after}_ctm instead and then
have a single function that fills out your capability blob (or at least
the part of it that we have today) across all platforms? Or is this
something that we think might actually start to vary across the
different pipes of a single platform in the future?
Thanks, that's pretty good suggestion. Will do that.
+
+ if (blob)
+ return blob-base.id;
+
+ return 0;
+}
+
int get_pipe_gamma_capabilities(struct drm_device *dev,
struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
{
if (IS_CHERRYVIEW(dev))
return get_chv_pipe_gamma_capabilities(dev, palette_caps, crtc);
+ if (IS_BROADWELL(dev) || IS_GEN9(dev))
+ return get_gen9_pipe_gamma_capabilities(dev,
+ palette_caps, crtc);
return -EINVAL;
}
@@ -491,7 +519,7 @@ void intel_attach_color_properties_to_crtc(struct
drm_device *dev,
struct drm_crtc *crtc;
int capabilities_blob_id;
- if (IS_CHERRYVIEW(dev)) {
+ if (IS_CHERRYVIEW(dev) || IS_BROADWELL(dev) || IS_GEN9(dev)) {
'IS_CHERRYVIEW(dev) || IS_BROADWELL(dev)' could be simplified to just
'IS_GEN8(dev)' right?
yep, will do it.
Matt
crtc = obj_to_crtc(mode_obj);
palette_caps = kzalloc(sizeof(struct drm_palette_caps),
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h
b/drivers/gpu/drm/i915/intel_color_manager.h
index b2ee847..78de1a2 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -35,6 +35,9 @@
#define CHV_DEGAMMA_MAX_VALS 65
#define CHV_10BIT_GAMMA_MAX_VALS 257
+#define GEN9_PALETTE_STRUCT_VERSION1
+#define GEN9_SPLITGAMMA_MAX_VALS 512
+
/* Gamma correction */
#define CHV_GAMMA_DATA_STRUCT_VERSION 1
#define CHV_10BIT_GAMMA_MAX_VALS 257
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 15/18] drm/i915: Initialize Gen8 pipe gamma correction
On Thu, Aug 06, 2015 at 10:08:24PM +0530, Shashank Sharma wrote:
From: Kausal Malladi kausalmall...@gmail.com
This patch initializes gamma color correction proeprty
typo
for Gen8 and higher platforms.
I'd specifically say 'BDW and Gen9' here since we already have some gen8
support (CHV).
It does the following :
1. Load pipe Gamma color correction capabilities for BDW/SKL/BXT
2. Attach the color properties to CRTC
Signed-off-by: Shashank Sharma shashank.sha...@intel.com
Signed-off-by: Kausal Malladi kausalmall...@gmail.com
---
drivers/gpu/drm/i915/intel_color_manager.c | 30
+-
drivers/gpu/drm/i915/intel_color_manager.h | 3 +++
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
b/drivers/gpu/drm/i915/intel_color_manager.c
index 5fa575b..bc77ab5 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -475,11 +475,39 @@ int get_chv_pipe_gamma_capabilities(struct drm_device
*dev,
return 0;
}
+int get_gen9_pipe_gamma_capabilities(struct drm_device *dev,
+ struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
Calling this 'gen9' seems a little confusing to me given that it's also
used for BDW, which is a gen8 platform. The general pattern is that
functions get named after the first platform that works a specific way,
so I'd expect this to be called get_bdw_pipe_gamma_capabilities.
+{
+ struct drm_property_blob *blob = NULL;
+
+ /*
+ * This function exposes best capability for DeGamma and Gamma
+ * For BXT, the DeGamma LUT has 512 entries
+ * and the best Gamma capability has 512 entries
+ */
+ palette_caps-version = GEN9_PALETTE_STRUCT_VERSION;
+ palette_caps-num_samples_before_ctm =
+ GEN9_SPLITGAMMA_MAX_VALS;
+ palette_caps-num_samples_after_ctm =
+ GEN9_SPLITGAMMA_MAX_VALS;
+
+ blob = drm_property_create_blob(dev, sizeof(struct drm_palette_caps),
+ (const void *) palette_caps);
We're pretty much doing the same thing we did for CHV, but just filling
in different values. Could we just stick the number of samples in
INTEL_INFO(dev)-num_gamma_samples_{before/after}_ctm instead and then
have a single function that fills out your capability blob (or at least
the part of it that we have today) across all platforms? Or is this
something that we think might actually start to vary across the
different pipes of a single platform in the future?
+
+ if (blob)
+ return blob-base.id;
+
+ return 0;
+}
+
int get_pipe_gamma_capabilities(struct drm_device *dev,
struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
{
if (IS_CHERRYVIEW(dev))
return get_chv_pipe_gamma_capabilities(dev, palette_caps, crtc);
+ if (IS_BROADWELL(dev) || IS_GEN9(dev))
+ return get_gen9_pipe_gamma_capabilities(dev,
+ palette_caps, crtc);
return -EINVAL;
}
@@ -491,7 +519,7 @@ void intel_attach_color_properties_to_crtc(struct
drm_device *dev,
struct drm_crtc *crtc;
int capabilities_blob_id;
- if (IS_CHERRYVIEW(dev)) {
+ if (IS_CHERRYVIEW(dev) || IS_BROADWELL(dev) || IS_GEN9(dev)) {
'IS_CHERRYVIEW(dev) || IS_BROADWELL(dev)' could be simplified to just
'IS_GEN8(dev)' right?
Matt
crtc = obj_to_crtc(mode_obj);
palette_caps = kzalloc(sizeof(struct drm_palette_caps),
diff --git a/drivers/gpu/drm/i915/intel_color_manager.h
b/drivers/gpu/drm/i915/intel_color_manager.h
index b2ee847..78de1a2 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.h
+++ b/drivers/gpu/drm/i915/intel_color_manager.h
@@ -35,6 +35,9 @@
#define CHV_DEGAMMA_MAX_VALS 65
#define CHV_10BIT_GAMMA_MAX_VALS 257
+#define GEN9_PALETTE_STRUCT_VERSION 1
+#define GEN9_SPLITGAMMA_MAX_VALS 512
+
/* Gamma correction */
#define CHV_GAMMA_DATA_STRUCT_VERSION1
#define CHV_10BIT_GAMMA_MAX_VALS 257
--
1.9.1
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
