On Thu, Aug 06, 2015 at 10:08:24PM +0530, Shashank Sharma wrote:
> From: Kausal Malladi <kausalmalladi@xxxxxxxxx>
>
> 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.sharma@xxxxxxxxx>
> Signed-off-by: Kausal Malladi <kausalmalladi@xxxxxxxxx>
> ---
> 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_VERSION 1
> #define CHV_10BIT_GAMMA_MAX_VALS 257
> --
> 1.9.1
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
