Re: [Intel-gfx] Adding custom bugzilla fields
On Tue, 30 Jun 2015, Ander Conselvan De Oliveira wrote: > On Mon, 2015-06-29 at 14:31 +0300, Ander Conselvan De Oliveira wrote: >> On Fri, 2015-06-26 at 18:28 +0300, Ander Conselvan De Oliveira wrote: >> > Hi all, >> > >> > I've been looking into creating custom fields in Bugzilla to help sort >> > our bugs in a more manageable way. >> >> [...] >> >> > So I would like to hear what other people think about this. Specially, >> > about what should be in the features field. The values can change >> > overtime, but would be good to have a good list from the start. >> >> So here's the list after including Daniel's and Ville's feedback. If >> there's no more suggestions/objections, I'd like to create those fields >> already tomorrow. We can edit the list entries afterwards if needed. > > Thank you, everyone, for the input. The fields are now created. There are some additional feature fields that I've found might be useful: display/adapter (or display/dongle?) display/hotplug display/multi-gpu GEM/prime (or GEM/dma-buf?) other/IOMMU (or GEM/IOMMU?) other/BIOS other/module-reload (?) performance? BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: LVDS pixel clock check
On Tue, Aug 18, 2015 at 02:37:00PM +0300, Mika Kahola wrote:
> It is possible the we request to have a mode that has
> higher pixel clock than our HW can support. This patch
> checks if requested pixel clock is lower than the one
> supported by the HW. The requested mode is discarded
> if we cannot support the requested pixel clock.
>
> This patch applies to LVDS.
>
> V2:
> - removed computation for max pixel clock
>
> V3:
> - cleanup by removing unnecessary lines
>
> V4:
> - moved supported dotclock check from mode_valid() to intel_lvds_init()
>
> V5:
> - dotclock check moved back to mode_valid() function
> - dotclock check for fixed mode
>
> Signed-off-by: Mika Kahola
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_lvds.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c
> b/drivers/gpu/drm/i915/intel_lvds.c
> index 881b5d1..0794dc8 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -289,11 +289,14 @@ intel_lvds_mode_valid(struct drm_connector *connector,
> {
> struct intel_connector *intel_connector = to_intel_connector(connector);
> struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
> + int max_pixclk = to_i915(connector->dev)->max_dotclk_freq;
>
> if (mode->hdisplay > fixed_mode->hdisplay)
> return MODE_PANEL;
> if (mode->vdisplay > fixed_mode->vdisplay)
> return MODE_PANEL;
> + if (fixed_mode->clock > max_pixclk)
> + return MODE_CLOCK_HIGH;
>
> return MODE_OK;
> }
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 1/4] drm/i915: Store max dotclock
On Tue, Aug 18, 2015 at 02:36:59PM +0300, Mika Kahola wrote:
> Store max dotclock into dev_priv structure so we are able
> to filter out the modes that are not supported by our
> platforms.
>
> V2:
> - limit the max dot clock frequency to max CD clock frequency
> for the gen9 and above
> - limit the max dot clock frequency to 90% of the max CD clock
> frequency for the older gens
> - for Cherryview the max dot clock frequency is limited to 95%
> of the max CD clock frequency
> - for gen2 and gen3 the max dot clock limit is set to 90% of the
> 2X max CD clock frequency
>
> V3:
> - max_dotclk variable renamed as max_dotclk_freq in i915_drv.h
> - in intel_compute_max_dotclk() the rounding method changed from
> round up to round down when computing max dotclock
>
> V4:
> - Haswell and Broadwell supports now dot clocks up to max CD clock
> frequency
>
> Signed-off-by: Mika Kahola
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 20
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e0f3f05..4696685 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1792,6 +1792,7 @@ struct drm_i915_private {
> unsigned int fsb_freq, mem_freq, is_ddr3;
> unsigned int skl_boot_cdclk;
> unsigned int cdclk_freq, max_cdclk_freq;
> + unsigned int max_dotclk_freq;
> unsigned int hpll_freq;
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f604ce1..a0f790d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5271,6 +5271,21 @@ static void modeset_update_crtc_power_domains(struct
> drm_atomic_state *state)
> modeset_put_power_domains(dev_priv, put_domains[i]);
> }
>
> +static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> +{
> + int max_cdclk_freq = dev_priv->max_cdclk_freq;
> +
> + if (INTEL_INFO(dev_priv)->gen >= 9 ||
> + IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> + return max_cdclk_freq;
> + else if (IS_CHERRYVIEW(dev_priv))
> + return max_cdclk_freq*95/100;
> + else if (INTEL_INFO(dev_priv)->gen < 4)
> + return 2*max_cdclk_freq*90/100;
> + else
> + return max_cdclk_freq*90/100;
> +}
> +
> static void intel_update_max_cdclk(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5310,8 +5325,13 @@ static void intel_update_max_cdclk(struct drm_device
> *dev)
> dev_priv->max_cdclk_freq = dev_priv->cdclk_freq;
> }
>
> + dev_priv->max_dotclk_freq = intel_compute_max_dotclk(dev_priv);
> +
> DRM_DEBUG_DRIVER("Max CD clock rate: %d kHz\n",
>dev_priv->max_cdclk_freq);
> +
> + DRM_DEBUG_DRIVER("Max dotclock rate: %d kHz\n",
> + dev_priv->max_dotclk_freq);
> }
>
> static void intel_update_cdclk(struct drm_device *dev)
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 3/4] drm/i915: DSI pixel clock check
On Tue, Aug 18, 2015 at 02:37:01PM +0300, Mika Kahola wrote:
> It is possible the we request to have a mode that has
> higher pixel clock than our HW can support. This patch
> checks if requested pixel clock is lower than the one
> supported by the HW. The requested mode is discarded
> if we cannot support the requested pixel clock.
>
> This patch applies to DSI.
>
> V2:
> - removed computation for max pixel clock
>
> V3:
> - cleanup by removing unnecessary lines
>
> V4:
> - max_pixclk variable renamed as max_dotclk
> - moved dot clock checking inside 'if (fixed_mode)'
>
> V5:
> - dot clock checked against fixed_mode clock
>
> Signed-off-by: Mika Kahola
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dsi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 4a601cf..781c267 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -654,6 +654,7 @@ intel_dsi_mode_valid(struct drm_connector *connector,
> {
> struct intel_connector *intel_connector = to_intel_connector(connector);
> struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
> + int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>
> DRM_DEBUG_KMS("\n");
>
> @@ -667,6 +668,8 @@ intel_dsi_mode_valid(struct drm_connector *connector,
> return MODE_PANEL;
> if (mode->vdisplay > fixed_mode->vdisplay)
> return MODE_PANEL;
> + if (fixed_mode->clock > max_dotclk)
> + return MODE_CLOCK_HIGH;
> }
>
> return MODE_OK;
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 4/4] drm/i915: DVO pixel clock check
On Tue, Aug 18, 2015 at 02:37:02PM +0300, Mika Kahola wrote:
> It is possible the we request to have a mode that has
> higher pixel clock than our HW can support. This patch
> checks if requested pixel clock is lower than the one
> supported by the HW. The requested mode is discarded
> if we cannot support the requested pixel clock.
>
> This patch applies to DVO.
>
> V2:
> - removed computation for max pixel clock
>
> V3:
> - cleanup by removing unnecessary lines
>
> V4:
> - clock check against max dotclock moved inside 'if (fixed_mode)'
>
> V5:
> - dot clock check against fixed_mode clock when available
>
> Signed-off-by: Mika Kahola
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dvo.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c
> b/drivers/gpu/drm/i915/intel_dvo.c
> index dc532bb..c80fe1f 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -201,6 +201,8 @@ intel_dvo_mode_valid(struct drm_connector *connector,
>struct drm_display_mode *mode)
> {
> struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
> + int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> + int target_clock = mode->clock;
>
> if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> return MODE_NO_DBLESCAN;
> @@ -212,8 +214,13 @@ intel_dvo_mode_valid(struct drm_connector *connector,
> return MODE_PANEL;
> if (mode->vdisplay > intel_dvo->panel_fixed_mode->vdisplay)
> return MODE_PANEL;
> +
> + target_clock = intel_dvo->panel_fixed_mode->clock;
> }
>
> + if (target_clock > max_dotclk)
> + return MODE_CLOCK_HIGH;
> +
> return intel_dvo->dev.dev_ops->mode_valid(&intel_dvo->dev, mode);
> }
>
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 4/4] drm/i915: DVO pixel clock check
On Fri, 2015-08-21 at 13:58 +0300, Ville Syrjälä wrote:
> On Tue, Aug 18, 2015 at 02:37:02PM +0300, Mika Kahola wrote:
> > It is possible the we request to have a mode that has
> > higher pixel clock than our HW can support. This patch
> > checks if requested pixel clock is lower than the one
> > supported by the HW. The requested mode is discarded
> > if we cannot support the requested pixel clock.
> >
> > This patch applies to DVO.
> >
> > V2:
> > - removed computation for max pixel clock
> >
> > V3:
> > - cleanup by removing unnecessary lines
> >
> > V4:
> > - clock check against max dotclock moved inside 'if (fixed_mode)'
> >
> > V5:
> > - dot clock check against fixed_mode clock when available
> >
> > Signed-off-by: Mika Kahola
>
> Reviewed-by: Ville Syrjälä
>
Thanks Ville for the reviews!
-Mika-
> > ---
> > drivers/gpu/drm/i915/intel_dvo.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dvo.c
> > b/drivers/gpu/drm/i915/intel_dvo.c
> > index dc532bb..c80fe1f 100644
> > --- a/drivers/gpu/drm/i915/intel_dvo.c
> > +++ b/drivers/gpu/drm/i915/intel_dvo.c
> > @@ -201,6 +201,8 @@ intel_dvo_mode_valid(struct drm_connector *connector,
> > struct drm_display_mode *mode)
> > {
> > struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
> > + int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> > + int target_clock = mode->clock;
> >
> > if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> > return MODE_NO_DBLESCAN;
> > @@ -212,8 +214,13 @@ intel_dvo_mode_valid(struct drm_connector *connector,
> > return MODE_PANEL;
> > if (mode->vdisplay > intel_dvo->panel_fixed_mode->vdisplay)
> > return MODE_PANEL;
> > +
> > + target_clock = intel_dvo->panel_fixed_mode->clock;
> > }
> >
> > + if (target_clock > max_dotclk)
> > + return MODE_CLOCK_HIGH;
> > +
> > return intel_dvo->dev.dev_ops->mode_valid(&intel_dvo->dev, mode);
> > }
> >
> > --
> > 1.9.1
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Enabling RC6 immediately during init/resume
Since RC6 enabling does not involve PCU communication overhead,
it can be enabled immediately during the resume time.
This will help save additional power & meet power requirements
for active Idle KPI where power is evaluated over
number of transitions of suspend/resume.
Signed-off-by: Namrta Salonie
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/intel_pm.c | 94 ++-
1 file changed, 63 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fff0c22..f1164c0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5468,14 +5468,13 @@ static void valleyview_cleanup_gt_powersave(struct
drm_device *dev)
valleyview_cleanup_pctx(dev);
}
-static void cherryview_enable_rps(struct drm_device *dev)
+static void cherryview_enable_rc6(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *ring;
- u32 gtfifodbg, val, rc6_mode = 0, pcbr;
+ u32 gtfifodbg, rc6_mode = 0, pcbr;
int i;
- WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
gtfifodbg = I915_READ(GTFIFODBG);
if (gtfifodbg) {
@@ -5486,9 +5485,9 @@ static void cherryview_enable_rps(struct drm_device *dev)
cherryview_check_pctx(dev_priv);
- /* 1a & 1b: Get forcewake during program sequence. Although the driver
-* hasn't enabled a state yet where we need forcewake, BIOS may have.*/
- intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+ /* 1: Get forcewake during program sequence. Although the driver
+ * hasn't enabled a state yet where we need forcewake, BIOS may have.*/
+intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
/* Disable RC states. */
I915_WRITE(GEN6_RC_CONTROL, 0);
@@ -5520,8 +5519,21 @@ static void cherryview_enable_rps(struct drm_device *dev)
rc6_mode = GEN7_RC_CTL_TO_MODE;
I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
+ intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+}
- /* 4 Program defaults and thresholds for RPS*/
+static void cherryview_enable_rps(struct drm_device *dev)
+{
+struct drm_i915_private *dev_priv = dev->dev_private;
+u32 val;
+
+ WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
+
+/* 1: Get forcewake during program sequence. As Driver would have
enabled RC6
+* by now before Turbo enabling sequence */
+intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+
+ /* 2: Program defaults and thresholds for RPS*/
I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 100);
I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);
I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 245000);
@@ -5530,7 +5542,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
- /* 5: Enable RPS */
+ /* 3: Enable RPS */
I915_WRITE(GEN6_RP_CONTROL,
GEN6_RP_MEDIA_HW_NORMAL_MODE |
GEN6_RP_MEDIA_IS_GFX |
@@ -5538,7 +5550,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
GEN6_RP_UP_BUSY_AVG |
GEN6_RP_DOWN_IDLE_AVG);
- /* Setting Fixed Bias */
+ /* 4: Setting Fixed Bias */
val = VLV_OVERRIDE_EN |
VLV_SOC_TDP_EN |
CHV_BIAS_CPU_50_SOC_50;
@@ -5546,7 +5558,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
- /* RPS code assumes GPLL is used */
+ /* 5: RPS code assumes GPLL is used */
WARN_ONCE((val & GPLLENABLE) == 0, "GPLL not enabled\n");
DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
@@ -5566,14 +5578,13 @@ static void cherryview_enable_rps(struct drm_device
*dev)
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
}
-static void valleyview_enable_rps(struct drm_device *dev)
+static void valleyview_enable_rc6(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *ring;
- u32 gtfifodbg, val, rc6_mode = 0;
+ u32 gtfifodbg, rc6_mode = 0;
int i;
- WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
valleyview_check_pctx(dev_priv);
@@ -5583,28 +5594,12 @@ static void valleyview_enable_rps(struct drm_device
*dev)
I915_WRITE(GTFIFODBG, gtfifodbg);
}
- /* If VLV, Forcewake all wells, else re-direct to regular path */
- intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+/* If VLV, Forcewake all wells */
+intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
/* Disable RC states. */
I915_WRITE(GEN6_RC_CONTROL, 0);
- I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 100);
- I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);
- I915
Re: [Intel-gfx] [PATCH] drm/i915: Enabling RC6 immediately during init/resume
On Sat, Aug 22, 2015 at 02:19:48AM +0530, Namrta Salonie wrote: > Since RC6 enabling does not involve PCU communication overhead, > it can be enabled immediately during the resume time. > This will help save additional power & meet power requirements > for active Idle KPI where power is evaluated over > number of transitions of suspend/resume. > > Signed-off-by: Namrta Salonie > Signed-off-by: Sagar Arun Kamble You can pull out gen9 rc6 as well, and apply a similar transformation to gen6-8. So instead of putting the if-chain in intel_enable_gt_powersave(), add intel_enable_rc6() and start placing the ready functions there. Reviewing the comments we only need the rpm lock until after rc6 enabling and as you keep that wakelock, you are not getting the full improvement you seek. If you keep refactoring the remaining two rc6 functions, you can then drop the wakelock. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] Revert "tests/gem_ctx_param_basic: fix invalid params"
On Fri, 2015-08-07 at 15:53 +0300, David Weinehall wrote:
> On Thu, Aug 06, 2015 at 11:33:00PM +0200, Daniel Vetter wrote:
> > This reverts commit 0b45b0746f45deea11670a8b2c949776bbbef55c.
> >
> > The point of testing for LAST_FLAG + 1 is to catch abi extensions -
> > despite our best efforts we really suck at properly reviewing for test
> > coverage when extending ABI.
> >
> > The real bug here is that David Weinhall hasn't submitted updated igts
> > for the NO_ZEROMAP feature yet. Imo the right course of action is to
> > revert that feature if the testcase don't show up within a few days.
>
> The reason I never submitted it was probably because of Chris's strong
> opposition to the feature in the first place; I've had the testcase
> laying around on my computer for quite a while.
>
> Anyhow, here's a slightly modified version of that test -- hopefully
> not breaking anything.
>
>
> Signed-off-by: David Weinehall
>
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index bc5d4bd827cf..f4deca6bd79e 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -102,6 +102,7 @@ struct local_i915_gem_context_param {
> uint32_t size;
> uint64_t param;
> #define LOCAL_CONTEXT_PARAM_BAN_PERIOD 0x1
> +#define LOCAL_CONTEXT_PARAM_NO_ZEROMAP 0x2
> uint64_t value;
> };
> void gem_context_require_ban_period(int fd);
> diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
> index b44b37cf0538..1e7e8ff40703 100644
> --- a/tests/gem_ctx_param_basic.c
> +++ b/tests/gem_ctx_param_basic.c
> @@ -57,7 +57,7 @@ igt_main
> ctx = gem_context_create(fd);
> }
>
> - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> + ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
>
> igt_subtest("basic") {
> ctx_param.context = ctx;
> @@ -98,21 +98,31 @@ igt_main
[...]
> - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
> + ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
>
> - igt_subtest("non-root-set") {
> + igt_subtest("non-root-set-no-zeromap") {
> igt_fork(child, 1) {
> igt_drop_root();
ctx_param.context = ctx;
TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL);
TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
ctx_param.value--;
TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
(I've added the code missing from the context)
The code in i915_gem_context_setparam_ioctl() that handles
CONTEXT_PARAM_NO_ZEROMAP never returns
EPERM, so this test always fails.
Cheers,
Ander
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH libdrm] intel: Serialize drmPrimeFDToHandle with struct_mutex
On Fri, Jul 24, 2015 at 11:51:01AM +0100, Chris Wilson wrote: > On Fri, Jul 24, 2015 at 11:22:34AM +0200, Michał Winiarski wrote: > > From: Rafał Sapała > > > > It is possible to hit a race condition in create_from_prime, when trying > > to import a BO that's currently being freed. In case of prime sharing > > we'll succesfully get a handle, but fail on get_tiling call, potentially > > confusing the caller (and requiring different locking scheme than with > > sharing using flink). Wrap fd_to_handle with struct_mutex to force > > a more consistent behaviour between prime/flink, convert fprintf to DBG > > when handling errors. > > The race is that the kernel returns us the same file-private handle as > the first thread, but that first thread is about to call gem_close > (thereby removing the handle from the file completely) and does so > between us acquiring the handle and taking the mutex. If we take > the mutex, then we acquire the refcnt on the bo prior to the first > thread completing its unref (and so preventing the early close). Or we > acquire the handle after the earlier close, in which case we are the new > owner. > > Reviewed-by: Chris Wilson Thanks for the patch & review, pushed. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/1] vbt child device size fix
Since I had to revert David's commit from v4.2, we'll have to add it back to drm-intel-next-fixes for v4.3. So it's a good occasion to reflect on it. I think it's problematic to be so strict with the VBT child device size. We generally go for really verbose error and debug messages in the dmesg and plunge on even when almost all hope is lost; it seems rather silly to effectively stop trying to get displays lit up if the VBT child device size is larger than expected. Hey, new stuff gets added at the end all the time. So log what happened, and move on. Here's a modified version of David's patch to do just that. BR, Jani. David Weinehall (1): drm/i915: Allow parsing of variable size child device entries from VBT drivers/gpu/drm/i915/intel_bios.c | 37 + drivers/gpu/drm/i915/intel_bios.h | 6 -- 2 files changed, 37 insertions(+), 6 deletions(-) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] gem_storedw_loop: Skip test if device doesn't have requested ring
The VEBOX ring is not available in generations before Haswell, so make
tests that use it skip instead of fail in previous gens.
Signed-off-by: Ander Conselvan de Oliveira
---
tests/gem_storedw_loop.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tests/gem_storedw_loop.c b/tests/gem_storedw_loop.c
index ee2f518..2263397 100644
--- a/tests/gem_storedw_loop.c
+++ b/tests/gem_storedw_loop.c
@@ -166,11 +166,15 @@ igt_main
}
for (i = 0; i < ARRAY_SIZE(rings); i++) {
- igt_subtest_f("basic-%s", rings[i].name)
+ igt_subtest_f("basic-%s", rings[i].name) {
+ gem_require_ring(fd, rings[i].id);
store_test(rings[i].id, 16*1024);
+ }
- igt_subtest_f("long-%s", rings[i].name)
+ igt_subtest_f("long-%s", rings[i].name) {
+ gem_require_ring(fd, rings[i].id);
store_test(rings[i].id, 1024*1024);
+ }
}
close(fd);
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/1] drm/i915: Allow parsing of variable size child device entries from VBT
From: David Weinehall
VBT version 196 increased the size of common_child_dev_config. The
parser code assumed that the size of this structure would not change.
The modified code now copies the amount needed based on the VBT version,
and emits a debug message if the VBT version is unknown (too new); since
the struct config block won't shrink in newer versions it should be
harmless to copy the maximum known size in such cases, so that's what we
do, but emitting the warning is probably sensible anyway.
In the longer run it might make sense to modify the parser code to use a
version/feature mapping, rather than hardcoding things like this, but
for now the variants are fairly managable.
This fixes a regression introduced in
commit 75067ddecf21271631bc018d2fb23ddd09b66aae
Author: Antti Koskipaa
Date: Fri Jul 10 14:10:55 2015 +0300
drm/i915: Per-DDI I_boost override
since that commit changed the child device config size without updating
the checks and memcpy.
v2: Stricter size checks
v3 by Jani:
- Keep the checks strict, and warnigns verbose, but keep going anyway.
- Take care to copy the max amount of child device config we can.
- Fix the messages.
Signed-off-by: David Weinehall
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 37 +
drivers/gpu/drm/i915/intel_bios.h | 6 --
2 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 64e5b15ae0b6..be83b77aa018 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1051,17 +1051,39 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
const union child_device_config *p_child;
union child_device_config *child_dev_ptr;
int i, child_device_num, count;
- u16 block_size;
+ u8 expected_size;
+ u16 block_size;
p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
if (!p_defs) {
DRM_DEBUG_KMS("No general definition block is found, no devices
defined.\n");
return;
}
- if (p_defs->child_dev_size < sizeof(*p_child)) {
- DRM_ERROR("General definiton block child device size is too
small.\n");
+ if (bdb->version < 195) {
+ expected_size = sizeof(struct old_child_dev_config);
+ } else if (bdb->version == 195) {
+ expected_size = 37;
+ } else if (bdb->version <= 197) {
+ expected_size = 38;
+ } else {
+ expected_size = 38;
+ BUILD_BUG_ON(sizeof(*p_child) < 38);
+ DRM_DEBUG_DRIVER("Expected child device config size for VBT
version %u not known; assuming %u\n",
+bdb->version, expected_size);
+ }
+
+ /* The legacy sized child device config is the minimum we need. */
+ if (p_defs->child_dev_size < sizeof(struct old_child_dev_config)) {
+ DRM_ERROR("Child device config size %u is too small.\n",
+ p_defs->child_dev_size);
return;
}
+
+ /* Flag an error for unexpected size, but continue anyway. */
+ if (p_defs->child_dev_size != expected_size)
+ DRM_ERROR("Unexpected child device config size %u (expected %u
for VBT version %u)\n",
+ p_defs->child_dev_size, expected_size, bdb->version);
+
/* get the block size of general definitions */
block_size = get_blocksize(p_defs);
/* get the number of child device */
@@ -1106,7 +1128,14 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
child_dev_ptr = dev_priv->vbt.child_dev + count;
count++;
- memcpy(child_dev_ptr, p_child, sizeof(*p_child));
+
+ /*
+* Copy as much as we know (sizeof) and is available
+* (child_dev_size) of the child device. Accessing the data must
+* depend on VBT version.
+*/
+ memcpy(child_dev_ptr, p_child,
+ min_t(size_t, p_defs->child_dev_size, sizeof(*p_child)));
}
return;
}
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index 6d909efbf43f..06d0dbde2be6 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -203,9 +203,11 @@ struct bdb_general_features {
#define DEVICE_PORT_DVOB 0x01
#define DEVICE_PORT_DVOC 0x02
-/* We used to keep this struct but without any version control. We should avoid
+/*
+ * We used to keep this struct but without any version control. We should avoid
* using it in the future, but it should be safe to keep using it in the old
- * code. */
+ * code. Do not change; we rely on its size.
+ */
struct old_child_dev_config {
u16 handle;
u16 device_type;
--
2.1.4
Re: [Intel-gfx] [PATCH 0/7] More PSR patches
Em Sex, 2015-08-21 às 01:04 +, Rodrigo Vivi escreveu: > This patch series brings stability to PSR on VLV, CHV, HSW and BDW. > > It fixes issues Matthew Garrett without causing the blank and frozen > screens Ivan Mitev was facing. > > It also move the VLV/CHV workaround of that big delay from re-enable > to the first enable after modeset that was the real issue. > With this besides the stability this series brings more PSR > residency to these platforms. I hate to be "that guy", but: no new IGT tests for anything? At least on my machine, all PSR tests pass without your series, so maybe we could write some new tests. Some commits mention bugs, but they don't even teach us how to reproduce the bugs. > > However the enable by default is still out of this series and will > come > only after an intensive QA. > > Thanks, > Rodrigo. > > Rodrigo Vivi (7): > drm/i915: Remove duplicated dpcd write on hsw_psr_enable_sink. > drm/i915: Fix PSR disable sequence on core platforms. > drm/i915: PSR: Let's rely more on frontbuffer tracking. > drm/i915: PSR: Mask LPSP hw tracking back again. > drm/i915: Delay first PSR activation. > drm/i915: Remove psr re-activation delay on HSW+. > drm/i915: Reduce PSR re-activation time for VLV/CHV. > > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_psr.c | 75 +- > -- > 2 files changed, 49 insertions(+), 27 deletions(-) > > -- > 2.4.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Allow parsing of variable size child device entries from VBT
On Fri, Aug 21, 2015 at 04:52:01PM +0300, Jani Nikula wrote:
> From: David Weinehall
>
> VBT version 196 increased the size of common_child_dev_config. The
> parser code assumed that the size of this structure would not change.
>
> The modified code now copies the amount needed based on the VBT version,
> and emits a debug message if the VBT version is unknown (too new); since
> the struct config block won't shrink in newer versions it should be
> harmless to copy the maximum known size in such cases, so that's what we
> do, but emitting the warning is probably sensible anyway.
>
> In the longer run it might make sense to modify the parser code to use a
> version/feature mapping, rather than hardcoding things like this, but
> for now the variants are fairly managable.
>
> This fixes a regression introduced in
>
> commit 75067ddecf21271631bc018d2fb23ddd09b66aae
> Author: Antti Koskipaa
> Date: Fri Jul 10 14:10:55 2015 +0300
>
> drm/i915: Per-DDI I_boost override
>
> since that commit changed the child device config size without updating
> the checks and memcpy.
>
> v2: Stricter size checks
>
> v3 by Jani:
> - Keep the checks strict, and warnigns verbose, but keep going anyway.
> - Take care to copy the max amount of child device config we can.
> - Fix the messages.
>
> Signed-off-by: David Weinehall
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_bios.c | 37 +
> drivers/gpu/drm/i915/intel_bios.h | 6 --
> 2 files changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 64e5b15ae0b6..be83b77aa018 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1051,17 +1051,39 @@ parse_device_mapping(struct drm_i915_private
> *dev_priv,
> const union child_device_config *p_child;
> union child_device_config *child_dev_ptr;
> int i, child_device_num, count;
> - u16 block_size;
> + u8 expected_size;
> + u16 block_size;
>
> p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
> if (!p_defs) {
> DRM_DEBUG_KMS("No general definition block is found, no devices
> defined.\n");
> return;
> }
> - if (p_defs->child_dev_size < sizeof(*p_child)) {
> - DRM_ERROR("General definiton block child device size is too
> small.\n");
> + if (bdb->version < 195) {
> + expected_size = sizeof(struct old_child_dev_config);
> + } else if (bdb->version == 195) {
> + expected_size = 37;
> + } else if (bdb->version <= 197) {
> + expected_size = 38;
> + } else {
> + expected_size = 38;
> + BUILD_BUG_ON(sizeof(*p_child) < 38);
> + DRM_DEBUG_DRIVER("Expected child device config size for VBT
> version %u not known; assuming %u\n",
> + bdb->version, expected_size);
> + }
> +
> + /* The legacy sized child device config is the minimum we need. */
> + if (p_defs->child_dev_size < sizeof(struct old_child_dev_config)) {
> + DRM_ERROR("Child device config size %u is too small.\n",
> + p_defs->child_dev_size);
> return;
> }
> +
> + /* Flag an error for unexpected size, but continue anyway. */
> + if (p_defs->child_dev_size != expected_size)
> + DRM_ERROR("Unexpected child device config size %u (expected %u
> for VBT version %u)\n",
> + p_defs->child_dev_size, expected_size, bdb->version);
> +
> /* get the block size of general definitions */
> block_size = get_blocksize(p_defs);
> /* get the number of child device */
> @@ -1106,7 +1128,14 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>
> child_dev_ptr = dev_priv->vbt.child_dev + count;
> count++;
> - memcpy(child_dev_ptr, p_child, sizeof(*p_child));
> +
> + /*
> + * Copy as much as we know (sizeof) and is available
> + * (child_dev_size) of the child device. Accessing the data must
> + * depend on VBT version.
> + */
> + memcpy(child_dev_ptr, p_child,
> +min_t(size_t, p_defs->child_dev_size, sizeof(*p_child)));
Looks good. Could maybe use a
BUILD_BUG_ON(sizeof(struct old_child_dev_config) != 33)
somewhere to make sure people don't make a mess of things.
Reviewed-by: Ville Syrjälä
> }
> return;
> }
> diff --git a/drivers/gpu/drm/i915/intel_bios.h
> b/drivers/gpu/drm/i915/intel_bios.h
> index 6d909efbf43f..06d0dbde2be6 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -203,9 +203,11 @@ struct bdb_general_features {
> #define DEVICE_PORT_DVOB 0x01
> #define DEVICE_PORT_DVOC 0x02
>
> -/* We used to keep this struct but without any version control. We
[Intel-gfx] [PATCH] drm/i915: Flush pipecontrol post-sync writes
In order to flush the results from in-batch pipecontrol writes (used for
example in glQuery) before declaring the batch complete (and so declaring
the query results coherent), we need to set the FlushEnable bit in our
flushing pipecontrol. The FlushEnable bit "waits until all previous
writes of immediate data from post-sync circles are complete before
executing the next command".
Signed-off-by: Chris Wilson
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/i915/intel_lrc.c| 1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 01cf0ca21990..e0c19d75b196 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1771,6 +1771,7 @@ static int gen8_emit_flush_render(struct
drm_i915_gem_request *request,
if (flush_domains) {
flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
+ flags |= PIPE_CONTROL_FLUSH_ENABLE;
}
if (invalidate_domains) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index c2392f6c4204..551af7399ca1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -330,6 +330,7 @@ gen7_render_ring_flush(struct drm_i915_gem_request *req,
if (flush_domains) {
flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
+ flags |= PIPE_CONTROL_FLUSH_ENABLE;
}
if (invalidate_domains) {
flags |= PIPE_CONTROL_TLB_INVALIDATE;
@@ -401,6 +402,7 @@ gen8_render_ring_flush(struct drm_i915_gem_request *req,
if (flush_domains) {
flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
+ flags |= PIPE_CONTROL_FLUSH_ENABLE;
}
if (invalidate_domains) {
flags |= PIPE_CONTROL_TLB_INVALIDATE;
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback
On Tue, Aug 18, 2015 at 02:51:52PM +0800, libin.y...@intel.com wrote:
> From: Libin Yang
>
> HDMI audio may not work at some frequencies
> with the HW provided N/CTS.
>
> This patch sets the proper N value for the
> given audio sample rate at the impacted frequencies.
> At other frequencies, it will use the N/CTS value
> which HW provides.
>
> Signed-off-by: Libin Yang
> ---
> drivers/gpu/drm/i915/intel_audio.c | 119
> +
> 1 file changed, 119 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index dc32cf4..96b97be 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -68,6 +68,31 @@ static const struct {
> { 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
> };
>
> +/* HDMI N/CTS table */
> +#define TMDS_297M 297000
> +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
I don't really like the defines.
> +static const struct {
> + int sample_rate;
> + int clock;
> + int n;
> + int cts;
> +} aud_ncts[] = {
> + { 44100, TMDS_296M, 4459, 234375 },
> + { 44100, TMDS_297M, 4704, 247500 },
> + { 48000, TMDS_296M, 5824, 281250 },
> + { 48000, TMDS_297M, 5120, 247500 },
> + { 32000, TMDS_296M, 5824, 421875 },
> + { 32000, TMDS_297M, 3072, 222750 },
> + { 88200, TMDS_296M, 8918, 234375 },
> + { 88200, TMDS_297M, 9408, 247500 },
> + { 96000, TMDS_296M, 11648, 281250 },
> + { 96000, TMDS_297M, 10240, 247500 },
> + { 176400, TMDS_296M, 17836, 234375 },
> + { 176400, TMDS_297M, 18816, 247500 },
> + { 44100, TMDS_296M, 23296, 281250 },
> + { 44100, TMDS_297M, 20480, 247500 },
> +};
Last two should be 192 kHz. All the other values seem to match the spec.
These do assume 8bpc, but as the spec doesn't even specify N/CTS
values for deep color modes @ 297MHz, so I suppose the expectations is
that the max TMDS clock will never be so high as to allow them.
If/when we start using manual programming for other TMDS clock rates
we'll need to consider 12bpc as well.
> +
> /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
> static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
> {
> @@ -90,6 +115,31 @@ static u32 audio_config_hdmi_pixel_clock(struct
> drm_display_mode *mode)
> return hdmi_audio_clock[i].config;
> }
>
> +static int audio_config_get_n(struct drm_display_mode *mode, int rate)
mode can be const.
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> + if ((rate == aud_ncts[i].sample_rate) &&
> + (mode->clock == aud_ncts[i].clock)) {
> + return aud_ncts[i].n;
> + }
> + }
> + return 0;
> +}
> +
> +/* check whether N/CTS/M need be set manually */
> +static bool audio_rate_need_prog(struct intel_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> + if (((mode->clock == TMDS_297M) ||
> + (mode->clock == TMDS_296M)) &&
> + intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> + return true;
> + else
> + return false;
> +}
> +
> static bool intel_eld_uptodate(struct drm_connector *connector,
> int reg_eldv, uint32_t bits_eldv,
> int reg_elda, uint32_t bits_elda,
> @@ -514,12 +564,81 @@ static int i915_audio_component_get_cdclk_freq(struct
> device *dev)
> return ret;
> }
>
> +static int i915_audio_component_sync_audio_rate(struct device *dev,
> + int port, int rate)
> +{
> + struct drm_i915_private *dev_priv = dev_to_i915(dev);
> + struct drm_device *drm_dev = dev_priv->dev;
> + struct intel_encoder *intel_encoder;
> + struct intel_digital_port *intel_dig_port;
> + struct intel_crtc *crtc;
> + struct drm_display_mode *mode;
> + enum pipe pipe = -1;
> + u32 tmp;
> + int n_low, n_up, n;
> +
> + /* 1. get the pipe */
> + for_each_intel_encoder(drm_dev, intel_encoder) {
> + if (intel_encoder->type != INTEL_OUTPUT_HDMI)
> + continue;
> + intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> + if (port == intel_dig_port->port) {
> + crtc = to_intel_crtc(intel_encoder->base.crtc);
This sort of thing would need some locking to safely start digging at
the modeset state.
I would probably not do that, and instead add some new lock(s) for this.
The modeset code would then fill out some relevant information protected
by that same lock, and this function could then go look at it without
having to worry about the rest of modeset locking/state.
> + if (!crtc) {
> + DRM_DEBUG_KMS("%s: crtc is NULL\n", __func__);
> + continue;
> + }
> + pipe = crtc->
Re: [Intel-gfx] [PATCH] drm/i915: Flush pipecontrol post-sync writes
On Fri, Aug 21, 2015 at 04:08:41PM +0100, Chris Wilson wrote:
> In order to flush the results from in-batch pipecontrol writes (used for
> example in glQuery) before declaring the batch complete (and so declaring
> the query results coherent), we need to set the FlushEnable bit in our
> flushing pipecontrol. The FlushEnable bit "waits until all previous
> writes of immediate data from post-sync circles are complete before
> executing the next command".
>
> Signed-off-by: Chris Wilson
> Cc: sta...@vger.kernel.org
Yeah makes as much sense as anything about pipecontrols.
Reviewed-by: Ville Syrjälä
Though the spec makes me thing it would be even more appropriate if
we did the seqno write using a post-sync operation and followed it
with such a pipecontrol flush. But I've not actually played around
with this stuff, so can't say for sure.
Oh and we're also lacking DC flushes everywhere, in case someone
cares.
> ---
> drivers/gpu/drm/i915/intel_lrc.c| 1 +
> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 01cf0ca21990..e0c19d75b196 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1771,6 +1771,7 @@ static int gen8_emit_flush_render(struct
> drm_i915_gem_request *request,
> if (flush_domains) {
> flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
> flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
> + flags |= PIPE_CONTROL_FLUSH_ENABLE;
> }
>
> if (invalidate_domains) {
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index c2392f6c4204..551af7399ca1 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -330,6 +330,7 @@ gen7_render_ring_flush(struct drm_i915_gem_request *req,
> if (flush_domains) {
> flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
> flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
> + flags |= PIPE_CONTROL_FLUSH_ENABLE;
> }
> if (invalidate_domains) {
> flags |= PIPE_CONTROL_TLB_INVALIDATE;
> @@ -401,6 +402,7 @@ gen8_render_ring_flush(struct drm_i915_gem_request *req,
> if (flush_domains) {
> flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
> flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
> + flags |= PIPE_CONTROL_FLUSH_ENABLE;
> }
> if (invalidate_domains) {
> flags |= PIPE_CONTROL_TLB_INVALIDATE;
> --
> 2.5.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 4/4] drm/i915: set proper N/CTS in modeset
On Tue, Aug 18, 2015 at 02:51:54PM +0800, libin.y...@intel.com wrote:
> From: Libin Yang
>
> When modeset occurs and the TMDS frequency is set to some
> speical values, the N/CTS need to be set manually if audio
> is playing.
>
> Signed-off-by: Libin Yang
> ---
> drivers/gpu/drm/i915/i915_reg.h| 8
> drivers/gpu/drm/i915/intel_audio.c | 40
> +-
> 2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6786e94..122b5bd 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7035,6 +7035,8 @@ enum skl_disp_power_wells {
> _HSW_AUD_MISC_CTRL_A, \
> _HSW_AUD_MISC_CTRL_B)
>
> +#define HSW_AUD_PIPE_CONN_SEL_CTRL 0x650ac
> +
> #define _HSW_AUD_DIP_ELD_CTRL_ST_A 0x650b4
> #define _HSW_AUD_DIP_ELD_CTRL_ST_B 0x651b4
> #define HSW_AUD_DIP_ELD_CTRL(pipe) _PIPE(pipe, \
> @@ -7049,6 +7051,12 @@ enum skl_disp_power_wells {
> _HSW_AUD_DIG_CNVT_2)
> #define DIP_PORT_SEL_MASK0x3
>
> +#define _HSW_AUD_STR_DESC_1 0x65084
> +#define _HSW_AUD_STR_DESC_2 0x65184
> +#define AUD_STR_DESC(pipe) _PIPE(pipe, \
> + _HSW_AUD_STR_DESC_1, \
> + _HSW_AUD_STR_DESC_2)
> +
> #define _HSW_AUD_EDID_DATA_A 0x65050
> #define _HSW_AUD_EDID_DATA_B 0x65150
> #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 96b97be..0dfdc77 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -140,6 +140,27 @@ static bool audio_rate_need_prog(struct intel_crtc *crtc,
> return false;
> }
>
> +static int audio_config_get_rate(struct drm_i915_private *dev_priv,
> + enum pipe pipe)
> +{
> + uint32_t tmp;
> + int cvt_idx;
> + int base_rate, mul, div, rate;
> +
> + tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL);
> + cvt_idx = (tmp >> (pipe * 8)) & 0xff;
> + tmp = I915_READ(AUD_STR_DESC(cvt_idx));
> + base_rate = tmp & (1 << 14);
> + if (base_rate == 0)
> + rate = 48000;
> + else
> + rate = 44100;
> + mul = (tmp & (0x7 << 11)) + 1;
> + div = (tmp & (0x7 << 8)) + 1;
> + rate = rate * mul / div;
> + return rate;
> +}
Given your new .sync_audio_rate() callback, the audio driver should have
already told us what the current sample rate is, and so we shouldn't
have to go dig it up from registers.
> +
> static bool intel_eld_uptodate(struct drm_connector *connector,
> int reg_eldv, uint32_t bits_eldv,
> int reg_elda, uint32_t bits_elda,
> @@ -261,6 +282,8 @@ static void hsw_audio_codec_enable(struct drm_connector
> *connector,
> const uint8_t *eld = connector->eld;
> uint32_t tmp;
> int len, i;
> + int n_low, n_up, n;
> + int rate;
>
> DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
> pipe_name(pipe), drm_eld_size(eld));
> @@ -296,12 +319,27 @@ static void hsw_audio_codec_enable(struct drm_connector
> *connector,
> /* Enable timestamps */
> tmp = I915_READ(HSW_AUD_CFG(pipe));
> tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> - tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
> if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
> tmp |= AUD_CONFIG_N_VALUE_INDEX;
> else
> tmp |= audio_config_hdmi_pixel_clock(mode);
> +
> + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> + if (audio_rate_eed_prog(intel_crtc, mode)) {
> + rate = audio_config_get_rate(dev_priv, pipe);
> + n = audio_config_get_n(mode, rate);
> + if (n != 0) {
> + n_low = n & 0xfff;
> + n_up = (n >> 12) & 0xff;
> + tmp &= ~(AUD_CONFIG_UPPER_N_MASK |
> + AUD_CONFIG_LOWER_N_MASK);
> + tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
> + (n_low << AUD_CONFIG_LOWER_N_SHIFT) |
> + AUD_CONFIG_N_PROG_ENABLE);
> + }
> + }
We should share the code to set this up with .sync_audio_rate() rather
than having two copies of essentically the same code.
> +
> I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> }
>
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Re: [Intel-gfx] [PATCH 0/7] More PSR patches
On Fri, Aug 21, 2015 at 7:06 AM Zanoni, Paulo R wrote: > > Em Sex, 2015-08-21 às 01:04 +, Rodrigo Vivi escreveu: > > This patch series brings stability to PSR on VLV, CHV, HSW and BDW. > > > > It fixes issues Matthew Garrett without causing the blank and frozen > > screens Ivan Mitev was facing. > > > > It also move the VLV/CHV workaround of that big delay from re-enable > > to the first enable after modeset that was the real issue. > > With this besides the stability this series brings more PSR > > residency to these platforms. > > I hate to be "that guy", no worries, you are absolutely right. > but: no new IGT tests for anything? Unfortunately no new test. I couldn't reproduce with igt neither of the 2 bugs this series fixes. > At least > on my machine, all PSR tests pass without your series, so maybe we > could write some new tests. > yes, it was passing all ltests and are still passing, but I couldn't reproduce with igt the new cases. > > Some commits mention bugs, but they don't even teach us how to > reproduce the bugs. > 1 is mentioned on the bugzilla, just right a new modeset when psr was running already it get unrecoverable lock screen. On igt the combination simple works, but on the distro flow, any screen off-on will trigger it. 2 is mentioned on the commit but also I coulnd't simulate that on igt, right after modeset after dpms off-on keep moving your mouse fast and you wont receive any screen update while you are moving your mouse... then you stop moving and when you move again the screen is updated... > > > > > However the enable by default is still out of this series and will > > come > > only after an intensive QA. > > > > Thanks, > > Rodrigo. > > > > Rodrigo Vivi (7): > > drm/i915: Remove duplicated dpcd write on hsw_psr_enable_sink. > > drm/i915: Fix PSR disable sequence on core platforms. > > drm/i915: PSR: Let's rely more on frontbuffer tracking. > > drm/i915: PSR: Mask LPSP hw tracking back again. > > drm/i915: Delay first PSR activation. > > drm/i915: Remove psr re-activation delay on HSW+. > > drm/i915: Reduce PSR re-activation time for VLV/CHV. > > > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > drivers/gpu/drm/i915/intel_psr.c | 75 +- > > -- > > 2 files changed, 49 insertions(+), 27 deletions(-) > > > > -- > > 2.4.3 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Fix some gcc warnings
From: Ville Syrjälä
Simple one:
drivers/gpu/drm/i915/i915_debugfs.c:2449:57: warning: Using plain integer as
NULL pointer
And something a bit more peculiar:
drivers/gpu/drm/i915/i915_debugfs.c:4953:18: warning: Variable length array is
used.
drivers/gpu/drm/i915/i915_debugfs.c:4953:32: warning: Variable length array is
used.
We pass a 'const int' as the array size which results in the warning,
dropping the const gets rid of the warning. Weird, but I think getting
rid of the warnings is better than holding on to the const.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 7a28de5..4088cb1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2446,7 +2446,7 @@ static int i915_guc_info(struct seq_file *m, void *data)
struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_guc guc;
- struct i915_guc_client client = { .client_obj = 0 };
+ struct i915_guc_client client = {};
struct intel_engine_cs *ring;
enum intel_ring_id i;
u64 total = 0;
@@ -4948,7 +4948,7 @@ static void cherryview_sseu_device_status(struct
drm_device *dev,
struct sseu_dev_status *stat)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- const int ss_max = 2;
+ int ss_max = 2;
int ss;
u32 sig1[ss_max], sig2[ss_max];
--
2.4.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Use ARRAY_SIZE() instead of hand rolling it
From: Ville Syrjälä
A couple of hand rolled ARRAY_SIZE()s caught my eye. Get rid of them.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_sdvo.c | 2 +-
drivers/gpu/drm/i915/intel_tv.c | 2 +-
drivers/gpu/drm/i915/intel_uncore.c | 3 +--
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
b/drivers/gpu/drm/i915/intel_sdvo.c
index c98098e..25d74d2 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -63,7 +63,7 @@ static const char *tv_format_names[] = {
"SECAM_60"
};
-#define TV_FORMAT_NUM (sizeof(tv_format_names) / sizeof(*tv_format_names))
+#define TV_FORMAT_NUM ARRAY_SIZE(tv_format_names)
struct intel_sdvo {
struct intel_encoder base;
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 0568ae6..cbe39dc 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1291,7 +1291,7 @@ static void intel_tv_find_better_format(struct
drm_connector *connector)
return;
- for (i = 0; i < sizeof(tv_modes) / sizeof(*tv_modes); i++) {
+ for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
tv_mode = tv_modes + i;
if ((intel_tv->type == DRM_MODE_CONNECTOR_Component) ==
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 9d3c2e4..ad4e421 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -52,8 +52,7 @@ static const char * const forcewake_domain_names[] = {
const char *
intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id)
{
- BUILD_BUG_ON((sizeof(forcewake_domain_names)/sizeof(const char *)) !=
-FW_DOMAIN_ID_COUNT);
+ BUILD_BUG_ON(ARRAY_SIZE(forcewake_domain_names) != FW_DOMAIN_ID_COUNT);
if (id >= 0 && id < FW_DOMAIN_ID_COUNT)
return forcewake_domain_names[id];
--
2.4.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Make some string arrays const
From: Ville Syrjälä
Most of our char* arrays are markes as const already, but a few slipped
through the cracks. Fix it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_sdvo.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
b/drivers/gpu/drm/i915/intel_sdvo.c
index 25d74d2..ca3dd7c 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -53,7 +53,7 @@
#define IS_DIGITAL(c) (c->output_flag & (SDVO_TMDS_MASK | SDVO_LVDS_MASK))
-static const char *tv_format_names[] = {
+static const char * const tv_format_names[] = {
"NTSC_M" , "NTSC_J" , "NTSC_443",
"PAL_B", "PAL_D" , "PAL_G" ,
"PAL_H", "PAL_I" , "PAL_M" ,
@@ -452,7 +452,7 @@ static void intel_sdvo_debug_write(struct intel_sdvo
*intel_sdvo, u8 cmd,
DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd, buffer);
}
-static const char *cmd_status_names[] = {
+static const char * const cmd_status_names[] = {
"Power on",
"Success",
"Not supported",
--
2.4.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Make some string arrays const
On Fri, Aug 21, 2015 at 08:45:29PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Most of our char* arrays are markes as const already, but a few slipped > through the cracks. Fix it. > > Signed-off-by: Ville Syrjälä All 3, Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Update DDI buffer translation programming.
Em Qua, 2015-08-05 às 14:59 -0700, Rodrigo Vivi escreveu:
> SKL-Y can now use the same programming for all VccIO values after an
> adjustment to I_boost.
> SKL-U DP table adjustments.
> 1. Remove SKL Y 0.95V from "SKL H and S" columns in all tables.
> The other SKL Y column removes the "0.85V VccIO" so it now applies
> to all voltages.
> 2. DP table changes SKL U 400mV+0db dword 0 value from 2016h to
> 201Bh.
> 3. DP table changes SKL U 600mv+0db dword 0 value from 2016h to
> 201Bh.
> 4. DP table increases I_boost to level 3 for SKL Y 400mv+9.5db.
>
> Reference: Graphics Spec Change r97962
> Cc: Arthur Runyan
> Signed-off-by: Rodrigo Vivi
drivers/gpu/drm/i915/intel_ddi.c: In function ‘skl_get_buf_trans_dp’:
drivers/gpu/drm/i915/intel_ddi.c:338:27: warning: unused variable
‘dev_priv’ [-Wunused-variable]
drivers/gpu/drm/i915/intel_ddi.c: In function ‘skl_get_buf_trans_hdmi’:
drivers/gpu/drm/i915/intel_ddi.c:394:27: warning: unused variable
‘dev_priv’ [-Wunused-variable]
With that fixed:
Reviewed-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 73 ++
> --
> 1 file changed, 25 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 9a40bfb..9e5a21b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -128,7 +128,7 @@ static const struct ddi_buf_trans
> bdw_ddi_translations_hdmi[] = {
> { 0x80FF, 0x001B0002, 0x0 },/* 9: 100010000 > */
>
> };
>
> -/* Skylake H, S, and Skylake Y with 0.95V VccIO */
> +/* Skylake H and S */
> static const struct ddi_buf_trans skl_ddi_translations_dp[] = {
> { 0x2016, 0x00A0, 0x0 },
> { 0x5012, 0x009B, 0x0 },
> @@ -143,23 +143,23 @@ static const struct ddi_buf_trans
> skl_ddi_translations_dp[] = {
>
> /* Skylake U */
> static const struct ddi_buf_trans skl_u_ddi_translations_dp[] = {
> - { 0x2016, 0x00A2, 0x0 },
> + { 0x201B, 0x00A2, 0x0 },
> { 0x5012, 0x0088, 0x0 },
> { 0x7011, 0x0087, 0x0 },
> - { 0x80009010, 0x00C7, 0x1 },/* Uses I_boost */
> - { 0x2016, 0x009D, 0x0 },
> + { 0x80009010, 0x00C7, 0x1 },/* Uses I_boost
> level 0x1 */
> + { 0x201B, 0x009D, 0x0 },
> { 0x5012, 0x00C7, 0x0 },
> { 0x7011, 0x00C7, 0x0 },
> { 0x2016, 0x0088, 0x0 },
> { 0x5012, 0x00C7, 0x0 },
> };
>
> -/* Skylake Y with 0.85V VccIO */
> -static const struct ddi_buf_trans skl_y_085v_ddi_translations_dp[] =
> {
> +/* Skylake Y */
> +static const struct ddi_buf_trans skl_y_ddi_translations_dp[] = {
> { 0x0018, 0x00A2, 0x0 },
> { 0x5012, 0x0088, 0x0 },
> { 0x7011, 0x0087, 0x0 },
> - { 0x80009010, 0x00C7, 0x1 },/* Uses I_boost */
> + { 0x80009010, 0x00C7, 0x3 },/* Uses I_boost
> level 0x3 */
> { 0x0018, 0x009D, 0x0 },
> { 0x5012, 0x00C7, 0x0 },
> { 0x7011, 0x00C7, 0x0 },
> @@ -168,7 +168,7 @@ static const struct ddi_buf_trans
> skl_y_085v_ddi_translations_dp[] = {
> };
>
> /*
> - * Skylake H and S, and Skylake Y with 0.95V VccIO
> + * Skylake H and S
> * eDP 1.4 low vswing translation parameters
> */
> static const struct ddi_buf_trans skl_ddi_translations_edp[] = {
> @@ -202,10 +202,10 @@ static const struct ddi_buf_trans
> skl_u_ddi_translations_edp[] = {
> };
>
> /*
> - * Skylake Y with 0.95V VccIO
> + * Skylake Y
> * eDP 1.4 low vswing translation parameters
> */
> -static const struct ddi_buf_trans skl_y_085v_ddi_translations_edp[]
> = {
> +static const struct ddi_buf_trans skl_y_ddi_translations_edp[] = {
> { 0x0018, 0x00A8, 0x0 },
> { 0x4013, 0x00AB, 0x0 },
> { 0x7011, 0x00A4, 0x0 },
> @@ -218,7 +218,7 @@ static const struct ddi_buf_trans
> skl_y_085v_ddi_translations_edp[] = {
> { 0x0018, 0x008A, 0x0 },
> };
>
> -/* Skylake H, S and U, and Skylake Y with 0.95V VccIO */
> +/* Skylake U, H and S */
> static const struct ddi_buf_trans skl_ddi_translations_hdmi[] = {
> { 0x0018, 0x00AC, 0x0 },
> { 0x5012, 0x009D, 0x0 },
> @@ -233,8 +233,8 @@ static const struct ddi_buf_trans
> skl_ddi_translations_hdmi[] = {
> { 0x0018, 0x00C7, 0x0 },
> };
>
> -/* Skylake Y with 0.85V VccIO */
> -static const struct ddi_buf_trans skl_y_085v_ddi_translations_hdmi[]
> = {
> +/* Skylake Y */
> +static const struct ddi_buf_trans skl_y_ddi_translations_hdmi[] = {
> { 0x0018, 0x00A1, 0x0 },
> { 0x5012, 0x00DF, 0x0 },
> { 0x7011, 0x0084, 0x0 },
> @@ -244,7 +244,7 @@ static const struct ddi_buf_trans
> skl_y_085v_ddi_translations_hdmi[] = {
> { 0x6013, 0x00C7, 0x0 },
> { 0x0018, 0x008A, 0x0 },
>
[Intel-gfx] [PATCH] scripts/kernel-doc: Improve Markdown results
Using pandoc as the Markdown engine cause some minor side effects as
pandoc includes main tags for almost everything.
Original Markdown support approach removes those main tags, but it caused
some inconsistencies when that tag is not the main one, like:
..
...
As kernel-doc was already including a tag, it causes the presence
of double tags (), which is not supported by DocBook
spec.
Html target gets away with it, so it causes no harm, although other
targets might not be so lucky (pdf as example).
We're now delegating the inclusion of the main tag to pandoc
only, as it knows when it's necessary or not.
That behavior causes a corner case, the only situation where we're
certainly that is not needed, which is the content.
For those cases, we're using a $output_markdown_nopara = 1 control var.
Signed-off-by: Danilo Cesar Lemes de Paula
Cc: Randy Dunlap
Cc: Daniel Vetter
Cc: Laurent Pinchart
Cc: Jonathan Corbet
Cc: Herbert Xu
Cc: Stephan Mueller
Cc: Michal Marek
Cc: linux-ker...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: intel-gfx
Cc: dri-devel
Cc: Graham Whaley
---
Thanks to Graham Whaley who helped me to debug this.
scripts/kernel-doc | 48 ++--
1 file changed, 34 insertions(+), 14 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 3850c1e..12a106c 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -288,6 +288,7 @@ my $use_markdown = 0;
my $verbose = 0;
my $output_mode = "man";
my $output_preformatted = 0;
+my $output_markdown_nopara = 0;
my $no_doc_sections = 0;
my @highlights = @highlights_man;
my $blankline = $blankline_man;
@@ -529,8 +530,11 @@ sub markdown_to_docbook {
close(CHLD_OUT);
close(CHLD_ERR);
- # pandoc insists in adding Main , we should remove them.
- $content =~ s:\A\s*\s*\n(.*)\n\Z$:$1:egsm;
+ if ($output_markdown_nopara) {
+ # pandoc insists in adding Main , sometimes we
+ # want to remove them.
+ $content =~ s:\A\s*\s*\n(.*)\n\Z$:$1:egsm;
+ }
return $content;
}
@@ -605,7 +609,7 @@ sub output_highlight {
$line =~ s/^\s*//;
}
if ($line eq ""){
- if (! $output_preformatted) {
+ if (! $output_preformatted && ! $use_markdown) {
print $lineprefix, local_unescape($blankline);
}
} else {
@@ -1026,7 +1030,7 @@ sub output_section_xml(%) {
# programlisting is already included by pandoc
print "\n" unless $use_markdown;
$output_preformatted = 1;
- } else {
+ } elsif (! $use_markdown) {
print "\n";
}
output_highlight($args{'sections'}{$section});
@@ -1034,7 +1038,7 @@ sub output_section_xml(%) {
if ($section =~ m/EXAMPLE/i) {
print "\n" unless $use_markdown;
print "\n";
- } else {
+ } elsif (! $use_markdown) {
print "\n";
}
print "\n";
@@ -1066,7 +1070,9 @@ sub output_function_xml(%) {
print " " . $args{'function'} . "\n";
print " \n";
print " ";
+$output_markdown_nopara = 1;
output_highlight ($args{'purpose'});
+$output_markdown_nopara = 0;
print " \n";
print "\n";
@@ -1104,10 +1110,12 @@ sub output_function_xml(%) {
$parameter_name =~ s/\[.*//;
print " \n
$parameter\n";
- print " \n\n";
+ print " \n";
+ print "\n" unless $use_markdown;
$lineprefix=" ";
output_highlight($args{'parameterdescs'}{$parameter_name});
- print "\n \n \n";
+ print "\n" unless $use_markdown;
+ print " \n \n";
}
print " \n";
} else {
@@ -1143,7 +1151,9 @@ sub output_struct_xml(%) {
print " " . $args{'type'} . " " . $args{'struct'} .
"\n";
print " \n";
print " ";
+$output_markdown_nopara = 1;
output_highlight ($args{'purpose'});
+$output_markdown_nopara = 0;
print " \n";
print "\n";
@@ -1196,9 +1206,11 @@ sub output_struct_xml(%) {
($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
print "";
print " $parameter\n";
- print " \n";
+ print " \n";
+ print " \n" unless $use_markdown;
output_highlight($args{'parameterdescs'}{$parameter_name});
- print " \n";
+ print " \n" unless $use_markdown;
+ print " \n";
print "\n";
}
print " \n";
@@ -1237,7 +1249,9 @@ sub output_enum_xml(%) {
print " enum " . $args{'enum'} . "\n";
print " \n";
print " ";
+$output_markdown_nopara = 1;
output_highlight ($args{'purpose'});
+$output_markdown_nopara = 0;
print " \n";
print "\n";
@@ -1267,9 +1281,11 @@ sub output_enum_xml(%) {
print "";
print " $parameter\n";
- print "
[Intel-gfx] [PATCH] drm/doc: Fixing xml documentation warning
"/**" should be used for kernel-doc documentation only.
It causes a warning with the new "in struct body" format.
Signed-off-by: Danilo Cesar Lemes de Paula
Cc: Randy Dunlap
Cc: Daniel Vetter
Cc: Laurent Pinchart
Cc: Jonathan Corbet
Cc: Herbert Xu
Cc: Stephan Mueller
Cc: Michal Marek
Cc: linux-ker...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: intel-gfx
Cc: dri-devel
Cc: Graham Whaley
---
include/drm/drm_modeset_lock.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 70595ff..c16a3ec 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -43,19 +43,19 @@ struct drm_modeset_acquire_ctx {
struct ww_acquire_ctx ww_ctx;
- /**
+ /*
* Contended lock: if a lock is contended you should only call
* drm_modeset_backoff() which drops locks and slow-locks the
* contended lock.
*/
struct drm_modeset_lock *contended;
- /**
+ /*
* list of held locks (drm_modeset_lock)
*/
struct list_head locked;
- /**
+ /*
* Trylock mode, use only for panic handlers!
*/
bool trylock_only;
@@ -70,12 +70,12 @@ struct drm_modeset_acquire_ctx {
* Used for locking CRTCs and other modeset resources.
*/
struct drm_modeset_lock {
- /**
+ /*
* modeset lock
*/
struct ww_mutex mutex;
- /**
+ /*
* Resources that are locked as part of an atomic update are added
* to a list (so we know what to unlock at the end).
*/
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Also call frontbuffer flip when disabling planes.
2015-07-24 20:38 GMT-03:00 Rodrigo Vivi : > We also need to call the frontbuffer flip to trigger proper > invalidations when disabling planes. Otherwise we will miss > screen updates when disabling sprites or cursor. > > It was caught with kms_psr_sink_crc sprite_plane_onoff > and cursor_plane_onoff subtests. > > This is probably a regression since I can also get this > with the manual test case, but with so many changes on atomic > modeset I couldn't track exactly when this was introduced. > > Signed-off-by: Rodrigo Vivi Per our conversation on IRC, you need to mention that this patch only affects the VLV family since on big core the HW tracking helps hide the problem. It would also be good to use the Testcase tags :) But it looks correct. So with the amended message: Reviewed-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/intel_display.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index af0bcfe..bb124cc 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11716,7 +11716,7 @@ int intel_plane_atomic_calc_changes(struct > drm_crtc_state *crtc_state, > intel_crtc->atomic.update_wm_pre = true; > } > > - if (visible) > + if (visible || was_visible) > intel_crtc->atomic.fb_bits |= > to_intel_plane(plane)->frontbuffer_bit; > > -- > 1.9.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/18] drm/i915: Add atomic get property interface for CRTC
On Thu, Aug 06, 2015 at 10:08:12PM +0530, Shashank Sharma wrote:
> From: Kausal Malladi
>
> This patch adds atomic get property interface for Intel CRTC. This
> interface will be used for get operation on any non-core DRM properties.
>
> Signed-off-by: Shashank Sharma
> Signed-off-by: Kausal Malladi
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 8
> drivers/gpu/drm/i915/intel_display.c | 1 +
> drivers/gpu/drm/i915/intel_drv.h | 4
> 3 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 922fecf..8d04ee8 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -327,3 +327,11 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
> DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
> return -EINVAL;
> }
> +
> +int intel_crtc_atomic_get_property(struct drm_crtc *crtc,
> +const struct drm_crtc_state *state,
> +struct drm_property *property,
> +uint64_t *val)
> +{
> + return 0;
I think this should be -EINVAL since it's an unrecognized property.
Probably add a DRM_DEBUG_KMS() message here like we have in
intel_plane_atomic_get_property() as well.
Matt
> +}
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 1fbf0ff..1412e21 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13353,6 +13353,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs =
> {
> .page_flip = intel_crtc_page_flip,
> .set_property = drm_atomic_helper_crtc_set_property,
> .atomic_set_property = intel_crtc_atomic_set_property,
> + .atomic_get_property = intel_crtc_atomic_get_property,
> .atomic_duplicate_state = intel_crtc_duplicate_state,
> .atomic_destroy_state = intel_crtc_destroy_state,
> };
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index c0ae529..b3dc138 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1426,6 +1426,10 @@ int intel_crtc_atomic_set_property(struct drm_crtc
> *plane,
> struct drm_crtc_state *state,
> struct drm_property *property,
> uint64_t val);
> +int intel_crtc_atomic_get_property(struct drm_crtc *plane,
> +const struct drm_crtc_state *state,
> +struct drm_property *property,
> +uint64_t *val);
>
> /* intel_atomic_plane.c */
> struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
> --
> 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
Re: [Intel-gfx] [PATCH 05/18] drm/i915: Initialize color manager and add gamma correction
On Thu, Aug 06, 2015 at 10:08:14PM +0530, Shashank Sharma wrote:
> From: Kausal Malladi
>
> As per Color Manager design, each driver is responsible to load its
> palette color correction and enhancement capabilities in the form of
> a DRM blob property, so that user space can query and read.
>
> This patch does the following:
> 1. Create new files intel_color_manager(.c/.h)
> 2. Attach CRTC Palette Capabilities property to CRTC
> 3. Load all CHV platform specific gamma color capabilities
> for CRTC into a blob that can be accessible by user space to
> query capabilities via DRM property interface.
>
> Signed-off-by: Shashank Sharma
> Signed-off-by: Kausal Malladi
> ---
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/intel_color_manager.c | 83
> ++
> drivers/gpu/drm/i915/intel_color_manager.h | 33
> drivers/gpu/drm/i915/intel_display.c | 2 +
> drivers/gpu/drm/i915/intel_drv.h | 4 ++
> 5 files changed, 124 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/i915/intel_color_manager.c
> create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 41fb8a9..303b903 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -60,7 +60,8 @@ i915-y += intel_audio.o \
> intel_overlay.o \
> intel_psr.o \
> intel_sideband.o \
> - intel_sprite.o
> + intel_sprite.o \
> + intel_color_manager.o
> i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o
> i915-$(CONFIG_DRM_I915_FBDEV)+= intel_fbdev.o
>
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
> b/drivers/gpu/drm/i915/intel_color_manager.c
> new file mode 100644
> index 000..1c9c477
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Shashank Sharma
> + * Kausal Malladi
> + */
> +
> +#include "intel_color_manager.h"
> +
> +int get_chv_pipe_gamma_capabilities(struct drm_device *dev,
> + struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
> +{
> + struct drm_property_blob *blob;
> +
> + /*
> + * This function exposes best capability for DeGamma and Gamma
> + * For CHV, the DeGamma LUT has 65 entries
> + * and the best Gamma capability has 257 entries (CGM unit)
> + */
> + palette_caps->version = CHV_PALETTE_STRUCT_VERSION;
> + palette_caps->num_samples_before_ctm =
> + CHV_DEGAMMA_MAX_VALS;
> + palette_caps->num_samples_after_ctm =
> + CHV_10BIT_GAMMA_MAX_VALS;
> +
> + blob = drm_property_create_blob(dev, sizeof(struct drm_palette_caps),
> + (const void *) palette_caps);
> +
> + if (blob)
> + return blob->base.id;
It's a corner case, but blob could be a non-NULL error code here (e.g.,
-ENOMEM). We should probably check for that before we try to
dereference it.
> +
> + 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);
> + return -EINVAL;
We only call this function in the IS_CHERRYVIEW case at the moment, so I
realize the EINVAL return is technically dead code, but going forward
would it make more sense to return a valid capabilities blob that
explicitly tells userspace we have no capabilities?
> +}
> +
> +void intel_attach_color_properties_to_crtc(struct drm_device *dev,
> + struct drm_mode_object *mode_obj)
> +{
> + struct drm_mode_config *
Re: [Intel-gfx] [PATCH 09/18] drm/i915: Pipe level Gamma correction for CHV/BSW
On Thu, Aug 06, 2015 at 10:08:18PM +0530, Shashank Sharma wrote:
> From: Kausal Malladi
>
> CHV/BSW platform supports two different pipe level gamma
> correction modes, which are:
> 1. Legacy 8-bit mode
> 2. 10-bit CGM (Color Gamut Mapping) mode
>
> This patch does the following:
> 1. Attaches Gamma property to CRTC
> 3. Adds the core Gamma correction function for CHV/BSW
> 4. Adds Gamma correction macros
>
> Signed-off-by: Shashank Sharma
> Signed-off-by: Kausal Malladi
> ---
> drivers/gpu/drm/i915/i915_reg.h| 12 +++
> drivers/gpu/drm/i915/intel_color_manager.c | 146
> +
> drivers/gpu/drm/i915/intel_color_manager.h | 20
> drivers/gpu/drm/i915/intel_display.c | 2 +
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 5 files changed, 182 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3a77678..4997ebd 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7921,4 +7921,16 @@ enum skl_disp_power_wells {
> #define GEN9_VEBOX_MOCS_00xcb00 /* Video MOCS base register*/
> #define GEN9_BLT_MOCS_0 0xcc00 /* Blitter MOCS base register*/
>
> +/* Color Management */
> +#define PIPEA_CGM_CONTROL(VLV_DISPLAY_BASE + 0x67A00)
> +#define PIPEB_CGM_CONTROL(VLV_DISPLAY_BASE + 0x69A00)
> +#define PIPEC_CGM_CONTROL(VLV_DISPLAY_BASE + 0x6BA00)
> +#define PIPEA_CGM_GAMMA (VLV_DISPLAY_BASE +
> 0x67000)
> +#define PIPEB_CGM_GAMMA (VLV_DISPLAY_BASE +
> 0x69000)
> +#define PIPEC_CGM_GAMMA (VLV_DISPLAY_BASE +
> 0x6B000)
> +#define _PIPE_CGM_CONTROL(pipe) \
> + (_PIPE3(pipe, PIPEA_CGM_CONTROL, PIPEB_CGM_CONTROL, PIPEC_CGM_CONTROL))
> +#define _PIPE_GAMMA_BASE(pipe) \
> + (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))
> +
> #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index 9a6126c..f8c8d26 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,149 @@
>
> #include "intel_color_manager.h"
>
> +int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob,
> + struct drm_crtc *crtc)
It looks like this function is only called from this file, right? We
can probably make it static in that case.
> +{
> + bool flag = false;
> + enum pipe pipe;
> + u8 red_int, blue_int, green_int;
> + u16 red_fract, green_fract, blue_fract;
> + u32 red, green, blue;
> + u32 cgm_control_reg = 0;
> + u32 cgm_gamma_reg = 0;
> + u32 count = 0, num_samples, word;
> + int ret = 0, length;
> + struct drm_r32g32b32 *correction_values = NULL;
> + struct drm_palette *gamma_data;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (!blob) {
> + DRM_ERROR("NULL Blob\n");
> + return -EINVAL;
> + }
The way the code stands right now, this should never be possible since
we don't even call this function if the blob is NULL, right? In that
case we usually just handle this as
if (WARN_ON(!blob))
return -EINVAL;
to make it a little more obvious that this is truly a "this can never
happen" type of assertion.
Also, see my comment near the bottom about whether this would be a more
intuitive way to disable the current gamma values.
> +
> + gamma_data = (struct drm_palette *)blob->data;
> +
> + if (gamma_data->version != CHV_GAMMA_DATA_STRUCT_VERSION) {
> + DRM_ERROR("Invalid Gamma Data struct version\n");
A user can trigger this on-demand (and thus spam your kernel log), so
this should probably be a DRM_DEBUG_KMS rather than a DRM_ERROR.
> + return -EINVAL;
> + }
> +
> + pipe = to_intel_crtc(crtc)->pipe;
> + num_samples = gamma_data->num_samples;
> + length = num_samples * sizeof(struct drm_r32g32b32);
> +
> + switch (num_samples) {
> + case 0:
> +
> + /* Disable Gamma functionality on Pipe - CGM Block */
> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
> + cgm_control_reg &= ~CGM_GAMMA_EN;
> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
> +
> + DRM_DEBUG_DRIVER("Gamma disabled on Pipe %c\n",
> + pipe_name(pipe));
> + ret = 0;
> + break;
> +
> + case CHV_8BIT_GAMMA_MAX_VALS:
> + case CHV_10BIT_GAMMA_MAX_VALS:
> +
> + count = 0;
> + cgm_gamma_reg = _PIPE_GAMMA_BASE(pipe);
> + correction_values = (struct drm_r32g32b32 *)&gamma_data->lut;
> +
> + while (count < num_samples) {
> + blue = correction_values[count].b32;
> + green = corre
Re: [Intel-gfx] [PATCH 08/18] drm/i915: Add pipe gamma correction handlers
On Thu, Aug 06, 2015 at 10:08:17PM +0530, Shashank Sharma wrote:
> From: Kausal Malladi
>
> I915 driver registers gamma correction as palette correction
> property with DRM layer. This patch adds set_property() and get_property()
> handlers for pipe level gamma correction.
>
> The set function attaches the Gamma correction blob to CRTC state, these
> values will be committed during atomic commit.
>
> Signed-off-by: Shashank Sharma
> Signed-off-by: Kausal Malladi
> ---
> drivers/gpu/drm/i915/intel_atomic.c| 14 ++
> drivers/gpu/drm/i915/intel_color_manager.c | 20
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> 3 files changed, 37 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 8d04ee8..9f55e6c 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -324,6 +324,13 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
> struct drm_property *property,
> uint64_t val)
> {
> + struct drm_device *dev = crtc->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (property == config->cm_palette_after_ctm_property)
> + return intel_color_manager_set_pipe_gamma(dev, state,
> + &crtc->base, val);
> +
> DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
> return -EINVAL;
> }
> @@ -333,5 +340,12 @@ int intel_crtc_atomic_get_property(struct drm_crtc *crtc,
> struct drm_property *property,
> uint64_t *val)
> {
> + struct drm_device *dev = crtc->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (property == config->cm_palette_after_ctm_property)
> + *val = (state->palette_after_ctm_blob) ?
> + state->palette_after_ctm_blob->base.id : 0;
> +
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index 1c9c477..9a6126c 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,26 @@
>
> #include "intel_color_manager.h"
>
> +int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
> + struct drm_crtc_state *crtc_state,
> + struct drm_mode_object *obj, uint32_t blob_id)
> +{
> + struct drm_property_blob *blob;
> +
> + blob = drm_property_lookup_blob(dev, blob_id);
> + if (!blob) {
> + DRM_ERROR("Invalid Blob ID\n");
A user can trigger this error on demand, so I think we want to keep this
as DRM_DEBUG_KMS (same on patches #10 and 13).
Matt
> + return -EINVAL;
> + }
> +
> + if (crtc_state->palette_after_ctm_blob)
> +
> drm_property_unreference_blob(crtc_state->palette_after_ctm_blob);
> +
> + /* Attach the blob to be commited in state */
> + crtc_state->palette_after_ctm_blob = blob;
> + return 0;
> +}
> +
> int get_chv_pipe_gamma_capabilities(struct drm_device *dev,
> struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
> {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index dee5f91..820ded7 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1441,5 +1441,8 @@ extern const struct drm_plane_helper_funcs
> intel_plane_helper_funcs;
> /* intel_color_manager.c */
> void intel_attach_color_properties_to_crtc(struct drm_device *dev,
> struct drm_mode_object *mode_obj);
> +int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
> + struct drm_crtc_state *crtc_state,
> + struct drm_mode_object *obj, uint32_t blob_id);
>
> #endif /* __INTEL_DRV_H__ */
> --
> 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
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
>
> 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
> Signed-off-by: Kausal Malladi
> ---
> 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
Re: [Intel-gfx] [PATCH 06/18] drm: Add color correction blobs in CRTC state
On Thu, Aug 06, 2015 at 10:08:15PM +0530, Shashank Sharma wrote:
> From: Kausal Malladi
>
> This patch adds new variables in CRTC state, to hold respective color
> correction blobs. These blobs will be required during the atomic commit
> for writing the color correction values in correction registers.
>
> Signed-off-by: Shashank Sharma
> Signed-off-by: Kausal Malladi
Since these are part of the state now, I believe you also need to add a
drm_property_reference_blob() call in
__drm_atomic_helper_crtc_duplicate_state and a
drm_property_unreference_blob() call in
__drm_atomic_helper_crtc_destroy_state so that we properly
increment/decrement the reference count as the state gets
duplicated/destroyed. I don't see that later in the series, so this
might be the best patch to add it to.
Matt
> ---
> include/drm/drm_crtc.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 3c59045..504539a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -304,6 +304,11 @@ struct drm_crtc_state {
> /* blob property to expose current mode to atomic userspace */
> struct drm_property_blob *mode_blob;
>
> + /* blob properties to hold the color properties' blobs */
> + struct drm_property_blob *palette_before_ctm_blob;
> + struct drm_property_blob *palette_after_ctm_blob;
> + struct drm_property_blob *ctm_blob;
> +
> struct drm_pending_vblank_event *event;
>
> struct drm_atomic_state *state;
> --
> 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
Re: [Intel-gfx] [PATCH 03/18] drm/i915: Add atomic get property interface for CRTC
Thanks for the review Matt.
Regards
Shashank
On 8/22/2015 4:10 AM, Matt Roper wrote:
On Thu, Aug 06, 2015 at 10:08:12PM +0530, Shashank Sharma wrote:
From: Kausal Malladi
This patch adds atomic get property interface for Intel CRTC. This
interface will be used for get operation on any non-core DRM properties.
Signed-off-by: Shashank Sharma
Signed-off-by: Kausal Malladi
---
drivers/gpu/drm/i915/intel_atomic.c | 8
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_drv.h | 4
3 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index 922fecf..8d04ee8 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -327,3 +327,11 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
return -EINVAL;
}
+
+int intel_crtc_atomic_get_property(struct drm_crtc *crtc,
+ const struct drm_crtc_state *state,
+ struct drm_property *property,
+ uint64_t *val)
+{
+ return 0;
I think this should be -EINVAL since it's an unrecognized property.
Probably add a DRM_DEBUG_KMS() message here like we have in
intel_plane_atomic_get_property() as well.
Matt
Got it.
+}
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1fbf0ff..1412e21 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13353,6 +13353,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
.page_flip = intel_crtc_page_flip,
.set_property = drm_atomic_helper_crtc_set_property,
.atomic_set_property = intel_crtc_atomic_set_property,
+ .atomic_get_property = intel_crtc_atomic_get_property,
.atomic_duplicate_state = intel_crtc_duplicate_state,
.atomic_destroy_state = intel_crtc_destroy_state,
};
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c0ae529..b3dc138 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1426,6 +1426,10 @@ int intel_crtc_atomic_set_property(struct drm_crtc
*plane,
struct drm_crtc_state *state,
struct drm_property *property,
uint64_t val);
+int intel_crtc_atomic_get_property(struct drm_crtc *plane,
+ const struct drm_crtc_state *state,
+ struct drm_property *property,
+ uint64_t *val);
/* intel_atomic_plane.c */
struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/18] drm/i915: Initialize color manager and add gamma correction
Regards
Shashank
On 8/22/2015 4:10 AM, Matt Roper wrote:
On Thu, Aug 06, 2015 at 10:08:14PM +0530, Shashank Sharma wrote:
From: Kausal Malladi
As per Color Manager design, each driver is responsible to load its
palette color correction and enhancement capabilities in the form of
a DRM blob property, so that user space can query and read.
This patch does the following:
1. Create new files intel_color_manager(.c/.h)
2. Attach CRTC Palette Capabilities property to CRTC
3. Load all CHV platform specific gamma color capabilities
for CRTC into a blob that can be accessible by user space to
query capabilities via DRM property interface.
Signed-off-by: Shashank Sharma
Signed-off-by: Kausal Malladi
---
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/intel_color_manager.c | 83 ++
drivers/gpu/drm/i915/intel_color_manager.h | 33
drivers/gpu/drm/i915/intel_display.c | 2 +
drivers/gpu/drm/i915/intel_drv.h | 4 ++
5 files changed, 124 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/intel_color_manager.c
create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 41fb8a9..303b903 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -60,7 +60,8 @@ i915-y += intel_audio.o \
intel_overlay.o \
intel_psr.o \
intel_sideband.o \
- intel_sprite.o
+ intel_sprite.o \
+ intel_color_manager.o
i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o
i915-$(CONFIG_DRM_I915_FBDEV) += intel_fbdev.o
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
b/drivers/gpu/drm/i915/intel_color_manager.c
new file mode 100644
index 000..1c9c477
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Shashank Sharma
+ * Kausal Malladi
+ */
+
+#include "intel_color_manager.h"
+
+int get_chv_pipe_gamma_capabilities(struct drm_device *dev,
+ struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
+{
+ struct drm_property_blob *blob;
+
+ /*
+* This function exposes best capability for DeGamma and Gamma
+* For CHV, the DeGamma LUT has 65 entries
+* and the best Gamma capability has 257 entries (CGM unit)
+*/
+ palette_caps->version = CHV_PALETTE_STRUCT_VERSION;
+ palette_caps->num_samples_before_ctm =
+ CHV_DEGAMMA_MAX_VALS;
+ palette_caps->num_samples_after_ctm =
+ CHV_10BIT_GAMMA_MAX_VALS;
+
+ blob = drm_property_create_blob(dev, sizeof(struct drm_palette_caps),
+ (const void *) palette_caps);
+
+ if (blob)
+ return blob->base.id;
It's a corner case, but blob could be a non-NULL error code here (e.g.,
-ENOMEM). We should probably check for that before we try to
dereference it.
Agree, will check it.
+
+ 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);
+ return -EINVAL;
We only call this function in the IS_CHERRYVIEW case at the moment, so I
realize the EINVAL return is technically dead code, but going forward
would it make more sense to return a valid capabilities blob that
explicitly tells userspace we have no capabilities?
This function is more or less a platform check wrapper, which checks if
color correction is called for a supported platform. In the next patch
sets, we have IS_GEN9() and IS_BDW() coming here, and if we fail to find
the right platform, we
Re: [Intel-gfx] [PATCH 06/18] drm: Add color correction blobs in CRTC state
Regards
Shashank
On 8/22/2015 4:10 AM, Matt Roper wrote:
On Thu, Aug 06, 2015 at 10:08:15PM +0530, Shashank Sharma wrote:
From: Kausal Malladi
This patch adds new variables in CRTC state, to hold respective color
correction blobs. These blobs will be required during the atomic commit
for writing the color correction values in correction registers.
Signed-off-by: Shashank Sharma
Signed-off-by: Kausal Malladi
Since these are part of the state now, I believe you also need to add a
drm_property_reference_blob() call in
__drm_atomic_helper_crtc_duplicate_state and a
drm_property_unreference_blob() call in
__drm_atomic_helper_crtc_destroy_state so that we properly
increment/decrement the reference count as the state gets
duplicated/destroyed. I don't see that later in the series, so this
might be the best patch to add it to.
Matt
Oops, agree. Will add this.
---
include/drm/drm_crtc.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3c59045..504539a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -304,6 +304,11 @@ struct drm_crtc_state {
/* blob property to expose current mode to atomic userspace */
struct drm_property_blob *mode_blob;
+ /* blob properties to hold the color properties' blobs */
+ struct drm_property_blob *palette_before_ctm_blob;
+ struct drm_property_blob *palette_after_ctm_blob;
+ struct drm_property_blob *ctm_blob;
+
struct drm_pending_vblank_event *event;
struct drm_atomic_state *state;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/18] drm/i915: Add pipe gamma correction handlers
Regards
Shashank
On 8/22/2015 4:10 AM, Matt Roper wrote:
On Thu, Aug 06, 2015 at 10:08:17PM +0530, Shashank Sharma wrote:
From: Kausal Malladi
I915 driver registers gamma correction as palette correction
property with DRM layer. This patch adds set_property() and get_property()
handlers for pipe level gamma correction.
The set function attaches the Gamma correction blob to CRTC state, these
values will be committed during atomic commit.
Signed-off-by: Shashank Sharma
Signed-off-by: Kausal Malladi
---
drivers/gpu/drm/i915/intel_atomic.c| 14 ++
drivers/gpu/drm/i915/intel_color_manager.c | 20
drivers/gpu/drm/i915/intel_drv.h | 3 +++
3 files changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index 8d04ee8..9f55e6c 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -324,6 +324,13 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
struct drm_property *property,
uint64_t val)
{
+ struct drm_device *dev = crtc->dev;
+ struct drm_mode_config *config = &dev->mode_config;
+
+ if (property == config->cm_palette_after_ctm_property)
+ return intel_color_manager_set_pipe_gamma(dev, state,
+ &crtc->base, val);
+
DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
return -EINVAL;
}
@@ -333,5 +340,12 @@ int intel_crtc_atomic_get_property(struct drm_crtc *crtc,
struct drm_property *property,
uint64_t *val)
{
+ struct drm_device *dev = crtc->dev;
+ struct drm_mode_config *config = &dev->mode_config;
+
+ if (property == config->cm_palette_after_ctm_property)
+ *val = (state->palette_after_ctm_blob) ?
+ state->palette_after_ctm_blob->base.id : 0;
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
b/drivers/gpu/drm/i915/intel_color_manager.c
index 1c9c477..9a6126c 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,26 @@
#include "intel_color_manager.h"
+int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
+ struct drm_crtc_state *crtc_state,
+ struct drm_mode_object *obj, uint32_t blob_id)
+{
+ struct drm_property_blob *blob;
+
+ blob = drm_property_lookup_blob(dev, blob_id);
+ if (!blob) {
+ DRM_ERROR("Invalid Blob ID\n");
A user can trigger this error on demand, so I think we want to keep this
as DRM_DEBUG_KMS (same on patches #10 and 13).
Agree.
Matt
+ return -EINVAL;
+ }
+
+ if (crtc_state->palette_after_ctm_blob)
+
drm_property_unreference_blob(crtc_state->palette_after_ctm_blob);
+
+ /* Attach the blob to be commited in state */
+ crtc_state->palette_after_ctm_blob = blob;
+ return 0;
+}
+
int get_chv_pipe_gamma_capabilities(struct drm_device *dev,
struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
{
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dee5f91..820ded7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1441,5 +1441,8 @@ extern const struct drm_plane_helper_funcs
intel_plane_helper_funcs;
/* intel_color_manager.c */
void intel_attach_color_properties_to_crtc(struct drm_device *dev,
struct drm_mode_object *mode_obj);
+int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
+ struct drm_crtc_state *crtc_state,
+ struct drm_mode_object *obj, uint32_t blob_id);
#endif /* __INTEL_DRV_H__ */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/18] drm/i915: Pipe level Gamma correction for CHV/BSW
Regards
Shashank
On 8/22/2015 4:11 AM, Matt Roper wrote:
On Thu, Aug 06, 2015 at 10:08:18PM +0530, Shashank Sharma wrote:
From: Kausal Malladi
CHV/BSW platform supports two different pipe level gamma
correction modes, which are:
1. Legacy 8-bit mode
2. 10-bit CGM (Color Gamut Mapping) mode
This patch does the following:
1. Attaches Gamma property to CRTC
3. Adds the core Gamma correction function for CHV/BSW
4. Adds Gamma correction macros
Signed-off-by: Shashank Sharma
Signed-off-by: Kausal Malladi
---
drivers/gpu/drm/i915/i915_reg.h| 12 +++
drivers/gpu/drm/i915/intel_color_manager.c | 146 +
drivers/gpu/drm/i915/intel_color_manager.h | 20
drivers/gpu/drm/i915/intel_display.c | 2 +
drivers/gpu/drm/i915/intel_drv.h | 2 +
5 files changed, 182 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3a77678..4997ebd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7921,4 +7921,16 @@ enum skl_disp_power_wells {
#define GEN9_VEBOX_MOCS_0 0xcb00 /* Video MOCS base register*/
#define GEN9_BLT_MOCS_0 0xcc00 /* Blitter MOCS base register*/
+/* Color Management */
+#define PIPEA_CGM_CONTROL (VLV_DISPLAY_BASE + 0x67A00)
+#define PIPEB_CGM_CONTROL (VLV_DISPLAY_BASE + 0x69A00)
+#define PIPEC_CGM_CONTROL (VLV_DISPLAY_BASE + 0x6BA00)
+#define PIPEA_CGM_GAMMA(VLV_DISPLAY_BASE +
0x67000)
+#define PIPEB_CGM_GAMMA(VLV_DISPLAY_BASE +
0x69000)
+#define PIPEC_CGM_GAMMA(VLV_DISPLAY_BASE +
0x6B000)
+#define _PIPE_CGM_CONTROL(pipe) \
+ (_PIPE3(pipe, PIPEA_CGM_CONTROL, PIPEB_CGM_CONTROL, PIPEC_CGM_CONTROL))
+#define _PIPE_GAMMA_BASE(pipe) \
+ (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))
+
#endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
b/drivers/gpu/drm/i915/intel_color_manager.c
index 9a6126c..f8c8d26 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,149 @@
#include "intel_color_manager.h"
+int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob,
+ struct drm_crtc *crtc)
It looks like this function is only called from this file, right? We
can probably make it static in that case.
Yeah, this was the plan. We kept this non-static initially so that we
can debug with KGDB when required, but then forgot to add static in the
end :). Will do it for all platform specific core functions, for all
properties.
+{
+ bool flag = false;
+ enum pipe pipe;
+ u8 red_int, blue_int, green_int;
+ u16 red_fract, green_fract, blue_fract;
+ u32 red, green, blue;
+ u32 cgm_control_reg = 0;
+ u32 cgm_gamma_reg = 0;
+ u32 count = 0, num_samples, word;
+ int ret = 0, length;
+ struct drm_r32g32b32 *correction_values = NULL;
+ struct drm_palette *gamma_data;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (!blob) {
+ DRM_ERROR("NULL Blob\n");
+ return -EINVAL;
+ }
The way the code stands right now, this should never be possible since
we don't even call this function if the blob is NULL, right? In that
case we usually just handle this as
if (WARN_ON(!blob))
return -EINVAL;
to make it a little more obvious that this is truly a "this can never
happen" type of assertion.
Also, see my comment near the bottom about whether this would be a more
intuitive way to disable the current gamma values.
Got it. Agree.
+
+ gamma_data = (struct drm_palette *)blob->data;
+
+ if (gamma_data->version != CHV_GAMMA_DATA_STRUCT_VERSION) {
+ DRM_ERROR("Invalid Gamma Data struct version\n");
A user can trigger this on-demand (and thus spam your kernel log), so
this should probably be a DRM_DEBUG_KMS rather than a DRM_ERROR.
We though we will keep this check only until the first revision of the
structure is used, and once we have the next possible version, we will
remove this. Do you think we can keep it by then ?
+ return -EINVAL;
+ }
+
+ pipe = to_intel_crtc(crtc)->pipe;
+ num_samples = gamma_data->num_samples;
+ length = num_samples * sizeof(struct drm_r32g32b32);
+
+ switch (num_samples) {
+ case 0:
+
+ /* Disable Gamma functionality on Pipe - CGM Block */
+ cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
+ cgm_control_reg &= ~CGM_GAMMA_EN;
+ I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
+
+ DRM_DEBUG_DRIVER("Gamma disabled on Pipe %c\n",
+ pipe_name(pipe));
+ ret = 0;
+
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
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
Signed-off-by: Kausal Malladi
---
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
