Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable for valleyview platform.
> -Original Message- > From: Jani Nikula [mailto:jani.nik...@linux.intel.com] > Sent: Tuesday, June 24, 2014 11:58 PM > To: Wang, Quanxian > Cc: intel-gfx@lists.freedesktop.org; Daniel Vetter > Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable > for valleyview platform. > > >> Per DP spec we need to read the SINK_COUNT. Perhaps your problem is > >> the hotplug irqs? > > [Wang, Quanxian] Hi, Jani > > The log event is about the transaction event instead of hotplug event. It is > general since they will use I2c to read byte one by one. It will generate many > transaction. > > > > But the root cause is really about hotplug(intel_hpd_irq_handler). When > we switch to console, there will be a hotplug event happens after a while. > After that, the system will continually get the hotplug event to switch sink > device on and off periodly. With DisplayPort 1.2 spec, '' This bit is used > for > either monitor hotplug/unplug or for notification of a sink event.", I am not > sure if it is notification of a sink event or real hotplug event. We have no > code to identify the difference between hotplug/unplug and notification of > a sink event. I check display port spec and also not found how to identify > them differently. > > > > Question is: > > In function intel_dp_detect_dpcd, before checking SINK_COUNT, we will > use intel_dp_get_dpcd to get the dpcd. Could we think there is an active sink > device attached to branch device if dpcd content is not null. > > According to the display port spec, only sink device has dpcd, if we get > > one, > there must be a sink device attached to branch device or source device. Right? > > No. From your logs, the DPCD has bit 0 set in address 5h, which means > downstream port present, which is only allowed in branch devices. [Wang, Quanxian] Ok. Currently I have some founds. Sink device attached to branch device will be idle without operation after a while. And kernel will get the 'fake dpms off' event and called intel_encoder_dpms to disable the connector and dp link will be disabled(See the 1st line of log list below, intel_encoder_dpms will be called with DPMS off). At that time, kernel thought connector had no any sink device attached when SINK_COUNT return 0. Status will be changed from connected to disconnected. Encoder will be deleted after that. Whatever for console terminal or other apps which depends on connector will not work. In this case, The system is in black status even if you press any key or others. From ssh terminal, we could find kernel gets hpd event continually, however at that time, encoder is deleted, connector is not be restored. Always get '[CRTC:6] [NOFB ]'. So SINK_COUNT is not the only way to check the alive status for connector. If we got one or more, we can confirm that connector is alive. However if we get 0, we should not say connector is not alive. We should try another way to make sure if it is alive. I have tried with DDC checking if SINK_COUNT is 0. It works for DP with branch device. But if disconnect the sink device from Branch device, it will not work. So I *plan*: 1) when we get SINK_COUNT to be 0, don't return disconnected. We will continue to check DDC. 2) or firstly check OUI to make sure if there is branch device, if there is, then check DDC if SINK_COUNT is 0. Any suggestion for this process? [ 1334.170715] [drm:intel_encoder_dpms], mode:3 [ 1334.170721] [drm:intel_crtc_update_dpms], [ 1334.170726] [drm:i9xx_crtc_disable], [ 1334.170730] [drm:intel_disable_dp], [ 1334.170735] [drm:intel_dp_aux_native_write], [ 1334.170741] [drm:intel_dp_aux_native_write], retry 0 [ 1334.201477] [drm:intel_post_disable_dp], [ 1334.201483] [drm:intel_dp_link_down], [ 1334.253549] [drm:g4x_wait_for_vblank], vblank wait timed out [ 1334.256743] [drm:valleyview_update_wm], Setting FIFO watermarks - A: plane=2, cursor=2, B: plane=2, cursor=2, SR: plane=0, cursor=0 [ 1334.256761] [drm:check_encoder_state], [ENCODER:10:DAC-10] [ 1334.256769] [drm:check_encoder_state], [ENCODER:11:TMDS-11] [ 1334.256777] [drm:check_encoder_state], [ENCODER:15:TMDS-15] [ 1334.256784] [drm:check_crtc_state], [CRTC:3] [ 1334.256791] [drm:check_crtc_state], [CRTC:6] [ 1340.097288] [drm:valleyview_irq_handler], hotplug event happens 0x2002 & 0x007e08c0??? [ 1340.097311] [drm:intel_hpd_irq_handler], Received HPD interrupt on PIN 4 - cnt: 0 [ 1340.097417] [drm:i915_hotplug_work_func], running encoder hotplug functions [ 1340.097436] [drm:i915_hotplug_work_func], Connector HDMI-A-1 (pin 4) received hotplug event. [ 1340.097450] [drm:i915_hotplug_work_func], Connector DP-1 (pin 4) received hotplug event. [ 1340.097464] [drm:intel_hdmi_detect], [CONNECTOR:12:HDMI-A-1] [ 1340.104895] [drm:drm_do_probe_ddc_edid], drm: skipping non-existent adapter i915 gmbus dpb [ 1340.104913] [drm:intel_dp_detect], [CONNECTOR:16:DP-1] [ 1340.104925] [drm:intel_dp_detect_dpcd], Get dpcd [ 1340.105757] [drm:intel_dp_aux_native_read],
Re: [Intel-gfx] [PATCH] drm/i915: Wait for vblank after enabling the primary plane on BDW
I'm sure this might affect Wayne, so, cc'ing him here.
from my point of view this is right so:
Reviewed-by: Rodrigo Vivi
On Tue, Jun 24, 2014 at 3:59 AM, wrote:
> From: Ville Syrjälä
>
> BDW signals the flip done interrupt immediately after the DSPSURF write
> when the plane is disabled. This is true even if we've already armed
> DSPCNTR to enable the plane at the next vblank. This causes major
> problems for our page flip code which relies on the flip done interrupts
> happening at vblank time.
>
> So what happens is that we enable the plane, and immediately allow
> userspace to submit a page flip. If the plane is still in the process
> of being enabled when the page flip is issued, the flip done gets
> signalled immediately. Our DSPSURFLIVE check catches this to prevent
> premature flip completion, but it also means that we don't get a flip
> done interrupt when the plane actually gets enabled, and so the page
> flip is never completed.
>
> Work around this by re-introducing blocking vblank waits on BDW
> whenever we enable the primary plane.
>
> I removed some of the vblank waits here:
> commit 6304cd91e7f05f8802ea6f91287cac09741d9c46
> Author: Ville Syrjälä
> Date: Fri Apr 25 13:30:12 2014 +0300
>
> drm/i915: Drop the excessive vblank waits from modeset codepaths
>
> To avoid these blocking vblank waits we should start using the vblank
> interrupt instead of the flip done interrupt to complete page flips.
> But that's material for another patch.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79354
> Tested-by: Guo Jinxian
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 9 +
> drivers/gpu/drm/i915/intel_sprite.c | 8
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 9188fed..f92efc6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2087,6 +2087,7 @@ void intel_flush_primary_plane(struct
> drm_i915_private *dev_priv,
> static void intel_enable_primary_hw_plane(struct drm_i915_private
> *dev_priv,
> enum plane plane, enum pipe pipe)
> {
> + struct drm_device *dev = dev_priv->dev;
> struct intel_crtc *intel_crtc =
> to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> int reg;
> @@ -2106,6 +2107,14 @@ static void intel_enable_primary_hw_plane(struct
> drm_i915_private *dev_priv,
>
> I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE);
> intel_flush_primary_plane(dev_priv, plane);
> +
> + /*
> +* BDW signals flip done immediately if the plane
> +* is disabled, even if the plane enable is already
> +* armed to occur at the next vblank :(
> +*/
> + if (IS_BROADWELL(dev))
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 1b66ddc..9a17b4e 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -691,6 +691,14 @@ intel_post_enable_primary(struct drm_crtc *crtc)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> /*
> +* BDW signals flip done immediately if the plane
> +* is disabled, even if the plane enable is already
> +* armed to occur at the next vblank :(
> +*/
> + if (IS_BROADWELL(dev))
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> + /*
> * FIXME IPS should be fine as long as one plane is
> * enabled, but in practice it seems to have problems
> * when going from primary only to sprite only and vice
> --
> 1.8.5.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: fix sanitize_enable_ppgtt for full PPGTT
Apparently trinary logic is hard. We were falling through all the forced cases and simply enabling aliasing PPGTT or not based on hardware, rather than full PPGTT if available. References: https://bugs.freedesktop.org/show_bug.cgi?id=80083 Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_gem_gtt.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index a4153ee..86521a7 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -69,7 +69,13 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) return 0; } - return HAS_ALIASING_PPGTT(dev) ? 1 : 0; + /* Fall through to auto-detect */ + if (HAS_PPGTT(dev)) + return 2; + else if (HAS_ALIASING_PPGTT(dev)) + return 1; + + return 0; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] 3.15-rc: regression in suspend
On Mon 2014-06-09 13:03:31, Jiri Kosina wrote: > On Mon, 9 Jun 2014, Pavel Machek wrote: > > > > > Strange. It seems 3.15 with the patch reverted only boots in 30% or so > > > > cases... And I've seen resume failure, too, so maybe I was just lucky > > > > that it worked for a while. > > > > > > git bisect really likes 25f397a429dfa43f22c278d0119a60 - you're about > > > the 5th report or so that claims this is the culprit but it's > > > something else. The above code is definitely not used in i915 so bogus > > > bisect result. > > > > Note I did not do the bisect, I only attempted revert and test. > > > > And did three boots of successful s2ram.. only to find out that it > > does not really fix s2ram, I was just lucky :-(. > > > > Unfortunately, this means my s2ram problem will be tricky/impossible > > to bisect :-(. > > Welcome to the situation I have been in for past several months. Ok, so I have set up machines for ktest / autobisect, and found out that 3.16-rc1 no longer has that problem. Oh well, bisect would not be fun, anyway... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] mmu_notifier and i915_gem_userptr.c
On Fri, Jun 20, 2014 at 01:43:50PM +0200, Joerg Roedel wrote: > Change_pte is also called when the underlying page of an address > changes in the kernel which would matter for DMA. But that can only > happen in KSM and uprobes code which is probably not of interest for the > i915 driver. > > The other case where I think it matters is the do_wp_page() path for > COW. The code works by calling invalidate_range_start -> change_pte -> > invalidate_range_end. Your driver would react to this by unbinding the > vma from itself internally (after a fork for example). > > But I have to check whether this really matters here. Okay, I think it does not matter for the i915 driver. The code-paths which map pages read-only for COW invoke invalidate_range_start/end on the page-ranges which causes the driver to unbind the pages. When get_user_pages() is called again later it will do the COW by itself, so the driver doesn't need to care. So I tend to say that the i915 driver does not need a change_pte() call-back at all. But probably someone should double-check to make sure I didn't miss something. Joerg ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/11] drm/i915: Move VLV cmnlane workaround to intel_power_domains_init_hw()
On Wed, Jun 25, 2014 at 12:03:01PM -0700, Jesse Barnes wrote:
> On Fri, 13 Jun 2014 13:37:56 +0300
> ville.syrj...@linux.intel.com wrote:
>
> > From: Ville Syrjälä
> >
> > Now that the CMNRESET deassert is part of the cmnlane power well,
> > intel_reset_dpio() is called too late to make any difference. We've
> > deasserted CMNRESET by that time, and so the off+on toggle w/a will
> > never kick in.
> >
> > Move the workaround to intel_power_domains_init_hw() where it gets
> > called before we enable the init power domain.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 23 -
> > drivers/gpu/drm/i915/intel_drv.h | 3 +-
> > drivers/gpu/drm/i915/intel_pm.c | 67
> > ++--
> > 3 files changed, 58 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 33cc213..bcd32322 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1508,9 +1508,6 @@ static void intel_reset_dpio(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > - if (!IS_VALLEYVIEW(dev))
> > - return;
> > -
> > if (IS_CHERRYVIEW(dev)) {
> > enum dpio_phy phy;
> > u32 val;
> > @@ -1532,26 +1529,6 @@ static void intel_reset_dpio(struct drm_device *dev)
> > I915_WRITE(DISPLAY_PHY_CONTROL,
> > PHY_COM_LANE_RESET_DEASSERT(phy, val));
> > }
> > -
> > - } else {
> > - /*
> > -* If DPIO has already been reset, e.g. by BIOS, just skip all
> > -* this.
> > -*/
> > - if (I915_READ(DPIO_CTL) & DPIO_CMNRST)
> > - return;
> > -
> > - /*
> > -* From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
> > -* Need to assert and de-assert PHY SB reset by gating the
> > -* common lane power, then un-gating it.
> > -* Simply ungating isn't enough to reset the PHY enough to get
> > -* ports and lanes running.
> > -*/
> > - __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> > -false);
> > - __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> > -true);
> > }
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 5740be0..e565906 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -970,8 +970,7 @@ void intel_runtime_pm_put(struct drm_i915_private
> > *dev_priv);
> > void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
> > void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
> > void ilk_wm_get_hw_state(struct drm_device *dev);
> > -void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> > - enum punit_power_well power_well_id, bool enable);
> > +
> >
> > /* intel_sdvo.c */
> > bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool
> > is_sdvob);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index e9a8565..d8e20d3 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5962,9 +5962,10 @@ static bool i9xx_always_on_power_well_enabled(struct
> > drm_i915_private *dev_priv,
> > return true;
> > }
> >
> > -void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> > - enum punit_power_well power_well_id, bool enable)
> > +static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> > + struct i915_power_well *power_well, bool enable)
> > {
> > + enum punit_power_well power_well_id = power_well->data;
> > u32 mask;
> > u32 state;
> > u32 ctrl;
> > @@ -5997,14 +5998,6 @@ out:
> > mutex_unlock(&dev_priv->rps.hw_lock);
> > }
> >
> > -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> > - struct i915_power_well *power_well, bool enable)
> > -{
> > - enum punit_power_well power_well_id = power_well->data;
> > -
> > - __vlv_set_power_well(dev_priv, power_well_id, enable);
> > -}
> > -
> > static void vlv_power_well_sync_hw(struct drm_i915_private *dev_priv,
> >struct i915_power_well *power_well)
> > {
> > @@ -6435,6 +6428,21 @@ static struct i915_power_well vlv_power_wells[] = {
> > },
> > };
> >
> > +static struct i915_power_well *lookup_power_well(struct drm_i915_private
> > *dev_priv,
> > +enum punit_power_well
> > power_well_id)
> > +{
> > + struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > + struct i915_power_well *power_well;
> >
Re: [Intel-gfx] [PATCH 07/11] drm/i915: Warn if there's a cdclk change in progess
On Wed, Jun 25, 2014 at 11:55:58AM -0700, Jesse Barnes wrote: > On Fri, 13 Jun 2014 13:37:53 +0300 > ville.syrj...@linux.intel.com wrote: > > > From: Ville Syrjälä > > > > If someone is interested in the current cdclk frquency it should > > be stable and not in process of changing frquency. Warn if the current > > and requested cdclk don't match in .get_display_clock_spee() on vlv. > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_display.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 29dddec..601e97e 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -5234,6 +5234,10 @@ static int valleyview_get_display_clock_speed(struct > > drm_device *dev) > > > > divider = val & DISPLAY_FREQUENCY_VALUES; > > > > + WARN((val & DISPLAY_FREQUENCY_STATUS) != > > +(divider << DISPLAY_FREQUENCY_STATUS_SHIFT), > > +"cdclk change in progress\n"); > > + > > return DIV_ROUND_CLOSEST(vco << 1, divider + 1); > > } > > > > Hm, there's not much we can do in this case, so rather than warn maybe > we should try a wait instead, and only warn if it times out? Even then > there's not much we can do aside from poking the PUnit folks. This shouldn't happen unless we somehow messed up and triggered a cdclk change and didn't wait for it to complete, which would be a driver bug. So I think a simple WARN seems sufficient. -- 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 05/11] drm/i915: Use 200MHz cdclk on vlv when all pipes are off
On Wed, Jun 25, 2014 at 11:54:06AM -0700, Jesse Barnes wrote: > On Fri, 13 Jun 2014 13:37:51 +0300 > ville.syrj...@linux.intel.com wrote: > > > From: Ville Syrjälä > > > > Drop the cdclk frequency to 200MHz on vlv when all pipes are off. In > > theory we should be able to use 200MHz also when the pixel clock is at > > most 90% of 200MHz. However in practice all we seem to get is a solid > > color picture or an otherwise corrupted display. > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_display.c | 9 +++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 1f3985f..3a9b017 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -4520,14 +4520,19 @@ static int valleyview_calc_cdclk(struct > > drm_i915_private *dev_priv, > > * 400MHz > > * So we check to see whether we're above 90% of the lower bin and > > * adjust if needed. > > +* > > +* We seem to get an unstable or solid color picture at 200MHz. > > +* Not sure what's wrong. For now use 200MHz only when all pipes > > +* are off. > > */ > > if (max_pixclk > freq_320*9/10) > > return 40; > > else if (max_pixclk > 27*9/10) > > return freq_320; > > - else > > + else if (max_pixclk > 0) > > return 27; > > - /* Looks like the 200MHz CDclk freq doesn't work on some configs */ > > + else > > + return 20; > > } > > > > /* compute the max pixel clock for new configuration */ > > I guess this is safe, but optional (won't we be shutting off the clocks > anyway?). Ideally yes. But currently I'm not sure if that happens. > > Reviewed-by: Jesse Barnes > > -- > Jesse Barnes, Intel Open Source Technology Center -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 18/19] drm/i915: Only touch WRPLL hw state in enable/disable hooks
From: Daniel Vetter
To be able to do this we need to separately keep track of how many
crtcs need a given WRPLL and how many actually actively use it. The
common shared dpll framework already has all this, including massive
state readout and cross checking. Which allows us to do this switch in
a fairly small patch.
Signed-off-by: Daniel Vetter
Reviewed-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_ddi.c | 12 +---
drivers/gpu/drm/i915/intel_display.c | 15 +++
drivers/gpu/drm/i915/intel_drv.h | 2 --
3 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 4619ee2..64721e5 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -386,16 +386,6 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
return ret;
}
-void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
-{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- if (intel_crtc_to_shared_dpll(intel_crtc))
- intel_disable_shared_dpll(intel_crtc);
-
- intel_put_shared_dpll(intel_crtc);
-}
-
#define LC_FREQ 2700
#define LC_FREQ_2K (LC_FREQ * 2000)
@@ -716,7 +706,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
int type = intel_encoder->type;
int clock = intel_crtc->config.port_clock;
- intel_ddi_put_crtc_pll(crtc);
+ intel_put_shared_dpll(intel_crtc);
if (type == INTEL_OUTPUT_HDMI) {
struct intel_shared_dpll *pll;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1688913..62b6896 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4121,6 +4121,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc->active)
return;
+ if (intel_crtc_to_shared_dpll(intel_crtc))
+ intel_enable_shared_dpll(intel_crtc);
+
if (intel_crtc->config.has_dp_encoder)
intel_dp_set_m_n(intel_crtc);
@@ -4307,6 +4310,9 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
mutex_lock(&dev->struct_mutex);
intel_update_fbc(dev);
mutex_unlock(&dev->struct_mutex);
+
+ if (intel_crtc_to_shared_dpll(intel_crtc))
+ intel_disable_shared_dpll(intel_crtc);
}
static void ironlake_crtc_off(struct drm_crtc *crtc)
@@ -4315,10 +4321,6 @@ static void ironlake_crtc_off(struct drm_crtc *crtc)
intel_put_shared_dpll(intel_crtc);
}
-static void haswell_crtc_off(struct drm_crtc *crtc)
-{
- intel_ddi_put_crtc_pll(crtc);
-}
static void i9xx_pfit_enable(struct intel_crtc *crtc)
{
@@ -7565,9 +7567,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
if (!intel_ddi_pll_select(intel_crtc))
return -EINVAL;
- if (intel_crtc_to_shared_dpll(intel_crtc))
- intel_enable_shared_dpll(intel_crtc);
-
intel_crtc->lowfreq_avail = false;
return 0;
@@ -12170,7 +12169,7 @@ static void intel_init_display(struct drm_device *dev)
dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
dev_priv->display.crtc_enable = haswell_crtc_enable;
dev_priv->display.crtc_disable = haswell_crtc_disable;
- dev_priv->display.off = haswell_crtc_off;
+ dev_priv->display.off = ironlake_crtc_off;
dev_priv->display.update_primary_plane =
ironlake_update_primary_plane;
} else if (HAS_PCH_SPLIT(dev)) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8a31428..57cbecd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -708,7 +708,6 @@ void intel_ddi_disable_transcoder_func(struct
drm_i915_private *dev_priv,
void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc);
void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc);
bool intel_ddi_pll_select(struct intel_crtc *crtc);
-void intel_ddi_put_crtc_pll(struct drm_crtc *crtc);
void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
void intel_ddi_prepare_link_retrain(struct drm_encoder *encoder);
bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
@@ -800,7 +799,6 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
bool state);
#define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
#define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
-void intel_disable_shared_dpll(struct intel_crtc *crtc);
struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc);
void intel_put_shared_dpll(struct intel_crtc *crtc);
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 15/19] drm/i915: ->disable hook for WRPLLs
From: Daniel Vetter
Currently still with a redudant WARN_ON in there, the common shared
dpll code will take care of this in the future.
Also we need to flip the switch for the transitional hack now to make
sure that we disable the right pll.
Signed-off-by: Daniel Vetter
Reviewed-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_ddi.c | 26 +++---
drivers/gpu/drm/i915/intel_display.c | 8 +---
2 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9539405..61c2471 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -391,28 +391,20 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = crtc->dev->dev_private;
struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- uint32_t val;
+ struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(intel_crtc);
switch (intel_crtc->config.ddi_pll_sel) {
case PORT_CLK_SEL_WRPLL1:
plls->wrpll1_refcount--;
if (plls->wrpll1_refcount == 0) {
- DRM_DEBUG_KMS("Disabling WRPLL 1\n");
- val = I915_READ(WRPLL_CTL1);
- WARN_ON(!(val & WRPLL_PLL_ENABLE));
- I915_WRITE(WRPLL_CTL1, val & ~WRPLL_PLL_ENABLE);
- POSTING_READ(WRPLL_CTL1);
+ pll->disable(dev_priv, pll);
}
intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
break;
case PORT_CLK_SEL_WRPLL2:
plls->wrpll2_refcount--;
if (plls->wrpll2_refcount == 0) {
- DRM_DEBUG_KMS("Disabling WRPLL 2\n");
- val = I915_READ(WRPLL_CTL2);
- WARN_ON(!(val & WRPLL_PLL_ENABLE));
- I915_WRITE(WRPLL_CTL2, val & ~WRPLL_PLL_ENABLE);
- POSTING_READ(WRPLL_CTL2);
+ pll->disable(dev_priv, pll);
}
intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
break;
@@ -1317,6 +1309,17 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private
*dev_priv)
}
}
+static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
+ struct intel_shared_dpll *pll)
+{
+ uint32_t val;
+
+ val = I915_READ(WRPLL_CTL(pll->id));
+ WARN_ON(!(val & WRPLL_PLL_ENABLE));
+ I915_WRITE(WRPLL_CTL(pll->id), val & ~WRPLL_PLL_ENABLE);
+ POSTING_READ(WRPLL_CTL(pll->id));
+}
+
static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
struct intel_shared_dpll *pll,
struct intel_dpll_hw_state *hw_state)
@@ -1347,6 +1350,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
for (i = 0; i < 2; i++) {
dev_priv->shared_dplls[i].id = i;
dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
+ dev_priv->shared_dplls[i].disable = hsw_ddi_pll_disable;
dev_priv->shared_dplls[i].get_hw_state =
hsw_ddi_pll_get_hw_state;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d8277a1..1fab569 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5245,9 +5245,11 @@ static int intel_crtc_compute_config(struct intel_crtc
*crtc,
if (HAS_IPS(dev))
hsw_compute_ips_config(crtc, pipe_config);
- /* XXX: PCH clock sharing is done in ->mode_set, so make sure the old
-* clock survives for now. */
- if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
+ /*
+* XXX: PCH/WRPLL clock sharing is done in ->mode_set, so make sure the
+* old clock survives for now.
+*/
+ if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev) || HAS_DDI(dev))
pipe_config->shared_dpll = crtc->config.shared_dpll;
if (pipe_config->has_pch_encoder)
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/19] drm/i915: State readout and cross-checking for ddi_pll_sel
From: Daniel Vetter
To make things a bit more manageable extract a new function for
reading out common ddi port state. This means a bit of duplication
between encoders and the core since both look at the same registers,
but doesn't seem worth to make a fuzz about.
We can also remove the state readout code in intel_ddi_setup_hw_pll_state.
That code is only called from the hardware take over and not the cross
check code, and only after the crtc state is reconstructed. So we can
rely on an accurate value of crtc->config.ddi_pll_sel already.
Compared to the old code also trust the hw state more and don't
special-case port A - we want to cross-check the actual-state, not
bake in our own assumptions about how this is supposed to all be
linked up.
v2: Make use of the read-out ddi_pll_sel in intel_ddi_clock_get.
Signed-off-by: Daniel Vetter
Reviewed-by: Damien Lespiau
[imre: rebased on patchset version w/o pch/crt/fdi refactoring]
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_ddi.c | 40 +-
drivers/gpu/drm/i915/intel_display.c | 48
3 files changed, 34 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5db1959..3d61a53 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5772,6 +5772,7 @@ enum punit_power_well {
#define TRANS_DDI_FUNC_ENABLE (1<<31)
/* Those bits are ignored by pipe EDP since it can only connect to DDI A */
#define TRANS_DDI_PORT_MASK (7<<28)
+#define TRANS_DDI_PORT_SHIFT 28
#define TRANS_DDI_SELECT_PORT(x) ((x)<<28)
#define TRANS_DDI_PORT_NONE (0<<28)
#define TRANS_DDI_MODE_SELECT_MASK(7<<24)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c60d92a..5356e3e 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -612,11 +612,10 @@ static void intel_ddi_clock_get(struct intel_encoder
*encoder,
struct intel_crtc_config *pipe_config)
{
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
- enum port port = intel_ddi_get_encoder_port(encoder);
int link_clock = 0;
u32 val, pll;
- val = I915_READ(PORT_CLK_SEL(port));
+ val = pipe_config->ddi_pll_sel;
switch (val & PORT_CLK_SEL_MASK) {
case PORT_CLK_SEL_LCPLL_810:
link_clock = 81000;
@@ -,40 +1110,6 @@ bool intel_ddi_get_hw_state(struct intel_encoder
*encoder,
return false;
}
-static uint32_t intel_ddi_get_crtc_pll(struct drm_i915_private *dev_priv,
- enum pipe pipe)
-{
- uint32_t temp, ret;
- enum port port = I915_MAX_PORTS;
- enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
- pipe);
- int i;
-
- if (cpu_transcoder == TRANSCODER_EDP) {
- port = PORT_A;
- } else {
- temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
- temp &= TRANS_DDI_PORT_MASK;
-
- for (i = PORT_B; i <= PORT_E; i++)
- if (temp == TRANS_DDI_SELECT_PORT(i))
- port = i;
- }
-
- if (port == I915_MAX_PORTS) {
- WARN(1, "Pipe %c enabled on an unknown port\n",
-pipe_name(pipe));
- ret = PORT_CLK_SEL_NONE;
- } else {
- ret = I915_READ(PORT_CLK_SEL(port));
- DRM_DEBUG_KMS("Pipe %c connected to port %c using clock "
- "0x%08x\n", pipe_name(pipe), port_name(port),
- ret);
- }
-
- return ret;
-}
-
void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1163,9 +1128,6 @@ void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
continue;
}
- intel_crtc->config.ddi_pll_sel =
intel_ddi_get_crtc_pll(dev_priv,
-pipe);
-
switch (intel_crtc->config.ddi_pll_sel) {
case PORT_CLK_SEL_WRPLL1:
dev_priv->ddi_plls.wrpll1_refcount++;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 006b41b..ea8cc58 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7569,6 +7569,35 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
return 0;
}
+static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
+ struct intel_crtc_config *pipe_config)
+{
+ struct drm_device *dev = crtc->base.dev;
+ struct drm
[Intel-gfx] [PATCH 07/19] drm/i915: Move SPLL disabling into hsw_crt_post_disable
From: Daniel Vetter
Similar to how the ->crtc_mode_set hook should touch the hardware to
enable anything the ->crtc_off hook should disable anything in the
hardware. Otherwise runtime pm for dpms will not work.
Currently the only things left int the haswell_crtc_off hook is
disabling the ddi plls. We can't move the WRPLL enabling out yet
because the current ddi pll sharing code used by the haswell code
doesn't separately track active users and overall users. This must be
fixed by porting it to the generic shared display pll framework, which
is powerful enough.
But the SPLL source is only used by the crt encoder and so can be
moved already. We only need to make sure that the ddi port E is
already off, which hsw_fdi_disable does by calling
intel_ddi_post_disable.
With this the code reorg to shuffle hsw fdi/lpt specific code into a
new hsw-specific crt encoder type is now finally complete.
Signed-off-by: Daniel Vetter
[imre: rebased on patchset version w/o pch/crt/fdi refactoring]
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_crt.c | 15 +++
drivers/gpu/drm/i915/intel_ddi.c | 7 ---
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index a4bdf257..76ffa2c 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -206,6 +206,20 @@ static void intel_disable_crt(struct intel_encoder
*encoder)
intel_crt_set_dpms(encoder, DRM_MODE_DPMS_OFF);
}
+
+static void hsw_crt_post_disable(struct intel_encoder *encoder)
+{
+ struct drm_device *dev = encoder->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ uint32_t val;
+
+ DRM_DEBUG_KMS("Disabling SPLL\n");
+ val = I915_READ(SPLL_CTL);
+ WARN_ON(!(val & SPLL_PLL_ENABLE));
+ I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
+ POSTING_READ(SPLL_CTL);
+}
+
static void intel_enable_crt(struct intel_encoder *encoder)
{
struct intel_crt *crt = intel_encoder_to_crt(encoder);
@@ -873,6 +887,7 @@ void intel_crt_init(struct drm_device *dev)
crt->base.get_config = hsw_crt_get_config;
crt->base.get_hw_state = intel_ddi_get_hw_state;
crt->base.pre_enable = hsw_crt_pre_enable;
+ crt->base.post_disable = hsw_crt_post_disable;
} else {
crt->base.get_config = intel_crt_get_config;
crt->base.get_hw_state = intel_crt_get_hw_state;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index accacb9..3582270 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -394,13 +394,6 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
uint32_t val;
switch (intel_crtc->ddi_pll_sel) {
- case PORT_CLK_SEL_SPLL:
- DRM_DEBUG_KMS("Disabling SPLL\n");
- val = I915_READ(SPLL_CTL);
- WARN_ON(!(val & SPLL_PLL_ENABLE));
- I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
- POSTING_READ(SPLL_CTL);
- break;
case PORT_CLK_SEL_WRPLL1:
plls->wrpll1_refcount--;
if (plls->wrpll1_refcount == 0) {
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/19] drm/i915: Remove spll_refcount for hsw
From: Daniel Vetter
SPLL would be a reference clock we could potentially share,
especially if we want to use the SSC mode. But currently we
don't, so let's rip out this complexity for a simpler conversion
to the new display pll framework.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/intel_ddi.c | 41 +---
2 files changed, 13 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8cea596..bdc578b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -229,7 +229,6 @@ void intel_link_compute_m_n(int bpp, int nlanes,
struct intel_link_m_n *m_n);
struct intel_ddi_plls {
- int spll_refcount;
int wrpll1_refcount;
int wrpll2_refcount;
};
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ded6013..9f02281 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -394,14 +394,11 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
switch (intel_crtc->ddi_pll_sel) {
case PORT_CLK_SEL_SPLL:
- plls->spll_refcount--;
- if (plls->spll_refcount == 0) {
- DRM_DEBUG_KMS("Disabling SPLL\n");
- val = I915_READ(SPLL_CTL);
- WARN_ON(!(val & SPLL_PLL_ENABLE));
- I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
- POSTING_READ(SPLL_CTL);
- }
+ DRM_DEBUG_KMS("Disabling SPLL\n");
+ val = I915_READ(SPLL_CTL);
+ WARN_ON(!(val & SPLL_PLL_ENABLE));
+ I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
+ POSTING_READ(SPLL_CTL);
break;
case PORT_CLK_SEL_WRPLL1:
plls->wrpll1_refcount--;
@@ -425,7 +422,6 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
break;
}
- WARN(plls->spll_refcount < 0, "Invalid SPLL refcount\n");
WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
@@ -821,16 +817,9 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
}
} else if (type == INTEL_OUTPUT_ANALOG) {
- if (plls->spll_refcount == 0) {
- DRM_DEBUG_KMS("Using SPLL on pipe %c\n",
- pipe_name(pipe));
- plls->spll_refcount++;
- intel_crtc->ddi_pll_sel = PORT_CLK_SEL_SPLL;
- } else {
- DRM_ERROR("SPLL already in use\n");
- return false;
- }
-
+ DRM_DEBUG_KMS("Using SPLL on pipe %c\n",
+ pipe_name(pipe));
+ intel_crtc->ddi_pll_sel = PORT_CLK_SEL_SPLL;
} else {
WARN(1, "Invalid DDI encoder type %d\n", type);
return false;
@@ -869,13 +858,13 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
return;
case PORT_CLK_SEL_SPLL:
- pll_name = "SPLL";
- reg = SPLL_CTL;
- refcount = plls->spll_refcount;
new_val = SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz |
SPLL_PLL_SSC;
- break;
-
+ WARN(I915_READ(SPLL_CTL) & enable_bit, "SPLL already
enabled\n");
+ I915_WRITE(SPLL_CTL, new_val);
+ POSTING_READ(SPLL_CTL);
+ udelay(20);
+ return;
case PORT_CLK_SEL_WRPLL1:
case PORT_CLK_SEL_WRPLL2:
if (crtc->ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
@@ -1186,7 +1175,6 @@ void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
enum pipe pipe;
struct intel_crtc *intel_crtc;
- dev_priv->ddi_plls.spll_refcount = 0;
dev_priv->ddi_plls.wrpll1_refcount = 0;
dev_priv->ddi_plls.wrpll2_refcount = 0;
@@ -1203,9 +1191,6 @@ void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
pipe);
switch (intel_crtc->ddi_pll_sel) {
- case PORT_CLK_SEL_SPLL:
- dev_priv->ddi_plls.spll_refcount++;
- break;
case PORT_CLK_SEL_WRPLL1:
dev_priv->ddi_plls.wrpll1_refcount++;
break;
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/19] drm/i915: Basic shared dpll support for WRPLLs
From: Daniel Vetter
Just filing in names and ids, but not yet officially registering them
so that the hw state cross checker doesn't completely freak out about
them. Still since we do already read out and cross check
config->shared_dpll the basics are now there to flesh out the wrpll
shared dpll implementation.
The idea is now to roll out all the callbacks step-by-step and then at
the end switch to the shared dpll framework. This way hw and sw
changes are clearly separated.
Signed-off-by: Daniel Vetter
[imre: added const to hsw_ddi_pll_names (Damien)]
Reviewed-by: Damien Lespiau
---
drivers/gpu/drm/i915/i915_drv.h | 6 --
drivers/gpu/drm/i915/intel_ddi.c | 17 +
drivers/gpu/drm/i915/intel_display.c | 21 +
3 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bdc578b..7b0cbe4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -184,8 +184,10 @@ struct i915_mmu_object;
enum intel_dpll_id {
DPLL_ID_PRIVATE = -1, /* non-shared dpll in use */
/* real shared dpll ids must be >= 0 */
- DPLL_ID_PCH_PLL_A,
- DPLL_ID_PCH_PLL_B,
+ DPLL_ID_PCH_PLL_A = 0,
+ DPLL_ID_PCH_PLL_B = 1,
+ DPLL_ID_WRPLL1 = 0,
+ DPLL_ID_WRPLL2 = 1,
};
#define I915_NUM_PLLS 2
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 6e976ba..fae73d3 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -784,9 +784,11 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
if (reg == WRPLL_CTL1) {
plls->wrpll1_refcount++;
intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL1;
+ intel_crtc->config.shared_dpll = DPLL_ID_WRPLL1;
} else {
plls->wrpll2_refcount++;
intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
+ intel_crtc->config.shared_dpll = DPLL_ID_WRPLL2;
}
}
@@ -1313,10 +1315,25 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private
*dev_priv)
}
}
+static char *hsw_ddi_pll_names[] = {
+ "WRPLL 1",
+ "WRPLL 2",
+};
+
void intel_ddi_pll_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t val = I915_READ(LCPLL_CTL);
+ int i;
+
+ /* Dummy setup until everything is moved over to avoid upsetting the hw
+* state cross checker. */
+ dev_priv->num_shared_dpll = 0;
+
+ for (i = 0; i < 2; i++) {
+ dev_priv->shared_dplls[i].id = i;
+ dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
+ }
/* The LCPLL register should be turned on by the BIOS. For now let's
* just check its state and print errors in case something is wrong.
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index ea8cc58..2b83c07 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7582,6 +7582,16 @@ static void haswell_get_ddi_port_state(struct intel_crtc
*crtc,
port = (tmp & TRANS_DDI_PORT_MASK) >> TRANS_DDI_PORT_SHIFT;
pipe_config->ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
+
+ switch (pipe_config->ddi_pll_sel) {
+ case PORT_CLK_SEL_WRPLL1:
+ pipe_config->shared_dpll = DPLL_ID_WRPLL1;
+ break;
+ case PORT_CLK_SEL_WRPLL2:
+ pipe_config->shared_dpll = DPLL_ID_WRPLL2;
+ break;
+ }
+
/*
* Haswell has only FDI/PCH transcoder A. It is which is connected to
* DDI E. So just check whether this pipe is wired to DDI E and whether
@@ -11273,12 +11283,6 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
.page_flip = intel_crtc_page_flip,
};
-static void intel_cpu_pll_init(struct drm_device *dev)
-{
- if (HAS_DDI(dev))
- intel_ddi_pll_init(dev);
-}
-
static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv,
struct intel_shared_dpll *pll,
struct intel_dpll_hw_state *hw_state)
@@ -11366,7 +11370,9 @@ static void intel_shared_dpll_init(struct drm_device
*dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
+ if (HAS_DDI(dev))
+ intel_ddi_pll_init(dev);
+ else if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
ibx_pch_dpll_init(dev);
else
dev_priv->num_shared_dpll = 0;
@@ -12494,7 +12500,6 @@ void intel_modeset_init(struct drm_device *dev)
intel_init_dpio(dev);
intel_reset_dpio(dev);
- intel_cpu_pll_init(dev);
intel_shared_dpll_init(dev);
/*
[Intel-gfx] [PATCH 13/19] drm/i915: Document that the pll->mode_set hook is optional
From: Daniel Vetter
The WRPLLs won't use them.
Signed-off-by: Daniel Vetter
Reviewed-by: Damien Lespiau
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7b0cbe4..650a5ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -206,6 +206,8 @@ struct intel_shared_dpll {
/* should match the index in the dev_priv->shared_dplls array */
enum intel_dpll_id id;
struct intel_dpll_hw_state hw_state;
+ /* The mode_set hook is optional and should be used together with the
+* intel_prepare_shared_dpll function. */
void (*mode_set)(struct drm_i915_private *dev_priv,
struct intel_shared_dpll *pll);
void (*enable)(struct drm_i915_private *dev_priv,
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/19] drm/i915: ddi: move pch cleanup before encoder->post_disable
This is needed by an upcoming patch that moves the PCH/CRT PLL disabling
into the post_disable hook, after which we want to keep the modeset
sequence at its current state. At this point this won't have an effect
since the PCH/CRT post_disable hook is atm a NOP.
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_display.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b480d86..006b41b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4291,16 +4291,16 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
intel_ddi_disable_pipe_clock(intel_crtc);
- for_each_encoder_on_crtc(dev, crtc, encoder)
- if (encoder->post_disable)
- encoder->post_disable(encoder);
-
if (intel_crtc->config.has_pch_encoder) {
lpt_disable_pch_transcoder(dev_priv);
intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
intel_ddi_fdi_disable(crtc);
}
+ for_each_encoder_on_crtc(dev, crtc, encoder)
+ if (encoder->post_disable)
+ encoder->post_disable(encoder);
+
intel_crtc->active = false;
intel_update_watermarks(crtc);
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 17/19] drm/i915: Switch to common shared dpll framework for WRPLLs
From: Daniel Vetter
Mostly this patch is one big excersize in deleting code and asserts
which are no longer needed. Note that we still abuse the shared dpll
framework a bit since we call the enable/disable functions from the
crtc mode_set and off hooks. But changing the actual hardware sequence
will be done in the next step.
Note that besides the massive amount of changes in this patch the
places and order in which the low-level WRPLL code is called is
absolutely unchanged.
Signed-off-by: Daniel Vetter
Reviewed-by: Damien Lespiau
[imre: rebased on patchset version w/o pch/crt/fdi refactoring]
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_drv.h | 6 --
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_ddi.c | 143 ---
drivers/gpu/drm/i915/intel_display.c | 14 ++--
drivers/gpu/drm/i915/intel_drv.h | 9 ++-
5 files changed, 28 insertions(+), 145 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 855f055..bf88f2a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -233,11 +233,6 @@ void intel_link_compute_m_n(int bpp, int nlanes,
int pixel_clock, int link_clock,
struct intel_link_m_n *m_n);
-struct intel_ddi_plls {
- int wrpll1_refcount;
- int wrpll2_refcount;
-};
-
/* Interface history:
*
* 1.1: Original.
@@ -1485,7 +1480,6 @@ struct drm_i915_private {
int num_shared_dpll;
struct intel_shared_dpll shared_dplls[I915_NUM_PLLS];
- struct intel_ddi_plls ddi_plls;
int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
/* Reclocking support */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 654417e..a7e2ec8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5924,6 +5924,7 @@ enum punit_power_well {
#define PORT_CLK_SEL_LCPLL_1350 (1<<29)
#define PORT_CLK_SEL_LCPLL_810(2<<29)
#define PORT_CLK_SEL_SPLL (3<<29)
+#define PORT_CLK_SEL_WRPLL(pll) (((pll)+4)<<29)
#define PORT_CLK_SEL_WRPLL1 (4<<29)
#define PORT_CLK_SEL_WRPLL2 (5<<29)
#define PORT_CLK_SEL_NONE (7<<29)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index d1569d0..4619ee2 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -388,30 +388,12 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
{
- struct drm_i915_private *dev_priv = crtc->dev->dev_private;
- struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(intel_crtc);
- switch (intel_crtc->config.ddi_pll_sel) {
- case PORT_CLK_SEL_WRPLL1:
- plls->wrpll1_refcount--;
- if (plls->wrpll1_refcount == 0) {
- pll->disable(dev_priv, pll);
- }
- intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
- break;
- case PORT_CLK_SEL_WRPLL2:
- plls->wrpll2_refcount--;
- if (plls->wrpll2_refcount == 0) {
- pll->disable(dev_priv, pll);
- }
- intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
- break;
- }
+ if (intel_crtc_to_shared_dpll(intel_crtc))
+ intel_disable_shared_dpll(intel_crtc);
- WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
- WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
+ intel_put_shared_dpll(intel_crtc);
}
#define LC_FREQ 2700
@@ -731,17 +713,14 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
{
struct drm_crtc *crtc = &intel_crtc->base;
struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
- struct drm_i915_private *dev_priv = crtc->dev->dev_private;
- struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
int type = intel_encoder->type;
- enum pipe pipe = intel_crtc->pipe;
int clock = intel_crtc->config.port_clock;
intel_ddi_put_crtc_pll(crtc);
if (type == INTEL_OUTPUT_HDMI) {
struct intel_shared_dpll *pll;
- uint32_t reg, val;
+ uint32_t val;
unsigned p, n2, r2;
intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
@@ -750,79 +729,21 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
WRPLL_DIVIDER_POST(p);
- if (val == I915_READ(WRPLL_CTL1)) {
- DRM_DEBUG_KMS("Reusing WRPLL 1 on pipe %c\n",
-
[Intel-gfx] [PATCH 09/19] drm/i915: Move ddi_pll_sel into the pipe config
From: Daniel Vetter
Just boring sed job for preparation.
Signed-off-by: Daniel Vetter
Reviewed-by: Damien Lespiau
[imre: rebased on patchset version w/o pch/crt/fdi refactoring]
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_ddi.c | 34 +-
drivers/gpu/drm/i915/intel_drv.h | 5 +++--
2 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 3582270..c60d92a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -277,8 +277,8 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
/* Configure Port Clock Select */
- I915_WRITE(PORT_CLK_SEL(PORT_E), intel_crtc->ddi_pll_sel);
- WARN_ON(intel_crtc->ddi_pll_sel != PORT_CLK_SEL_SPLL);
+ I915_WRITE(PORT_CLK_SEL(PORT_E), intel_crtc->config.ddi_pll_sel);
+ WARN_ON(intel_crtc->config.ddi_pll_sel != PORT_CLK_SEL_SPLL);
/* Start the training iterating through available voltages and emphasis,
* testing each value twice. */
@@ -393,7 +393,7 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
uint32_t val;
- switch (intel_crtc->ddi_pll_sel) {
+ switch (intel_crtc->config.ddi_pll_sel) {
case PORT_CLK_SEL_WRPLL1:
plls->wrpll1_refcount--;
if (plls->wrpll1_refcount == 0) {
@@ -419,7 +419,7 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
- intel_crtc->ddi_pll_sel = PORT_CLK_SEL_NONE;
+ intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
}
#define LC_FREQ 2700
@@ -754,13 +754,13 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
switch (intel_dp->link_bw) {
case DP_LINK_BW_1_62:
- intel_crtc->ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
+ intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
break;
case DP_LINK_BW_2_7:
- intel_crtc->ddi_pll_sel = PORT_CLK_SEL_LCPLL_1350;
+ intel_crtc->config.ddi_pll_sel =
PORT_CLK_SEL_LCPLL_1350;
break;
case DP_LINK_BW_5_4:
- intel_crtc->ddi_pll_sel = PORT_CLK_SEL_LCPLL_2700;
+ intel_crtc->config.ddi_pll_sel =
PORT_CLK_SEL_LCPLL_2700;
break;
default:
DRM_ERROR("Link bandwidth %d unsupported\n",
@@ -804,16 +804,16 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
if (reg == WRPLL_CTL1) {
plls->wrpll1_refcount++;
- intel_crtc->ddi_pll_sel = PORT_CLK_SEL_WRPLL1;
+ intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL1;
} else {
plls->wrpll2_refcount++;
- intel_crtc->ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
+ intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
}
} else if (type == INTEL_OUTPUT_ANALOG) {
DRM_DEBUG_KMS("Using SPLL on pipe %c\n",
pipe_name(pipe));
- intel_crtc->ddi_pll_sel = PORT_CLK_SEL_SPLL;
+ intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_SPLL;
} else {
WARN(1, "Invalid DDI encoder type %d\n", type);
return false;
@@ -841,10 +841,10 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
BUILD_BUG_ON(enable_bit != SPLL_PLL_ENABLE);
BUILD_BUG_ON(enable_bit != WRPLL_PLL_ENABLE);
- switch (crtc->ddi_pll_sel) {
+ switch (crtc->config.ddi_pll_sel) {
case PORT_CLK_SEL_WRPLL1:
case PORT_CLK_SEL_WRPLL2:
- if (crtc->ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
+ if (crtc->config.ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
pll_name = "WRPLL1";
reg = WRPLL_CTL1;
refcount = plls->wrpll1_refcount;
@@ -1159,14 +1159,14 @@ void intel_ddi_setup_hw_pll_state(struct drm_device
*dev)
to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
if (!intel_crtc->active) {
- intel_crtc->ddi_pll_sel = PORT_CLK_SEL_NONE;
+ intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
continue;
}
- intel_crtc->ddi_pll_sel = intel_ddi_get_crtc_pll(dev_priv,
+ intel_crtc->config.ddi_pll_sel =
intel_ddi_get_crtc_pll(dev_priv,
pipe);
- switch (intel_crtc->ddi_pll
Re: [Intel-gfx] [WIP][PATCH 11/11] drm/i915: Turn off clocks when disp2d is powered down
On Fri, 13 Jun 2014 13:37:57 +0300
ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Set some bits in CCK/CCU to turn off display clocks when disp2d is power
> gated. Not sure this really helps with anything. Docs aren't all that clear.
>
> XXX: Doesn't actually work. CCK_DISPLAY_REF_CLOCK_CONTROL and CCU_ICLK_5
> writes don't have any effect on the registers for some reason. When clock
> gating disp2d via Punit CCK_DISPLAY_REF_CLOCK_CONTROL trunk off force bit
> gets set but again direct write has no effect.
> ---
> drivers/gpu/drm/i915/i915_reg.h | 5 +
> drivers/gpu/drm/i915/intel_pm.c | 35 +++
> 2 files changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2aa9a3c..be88b13 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -584,12 +584,17 @@ enum punit_power_well {
> #define DSI_PLL_M1_DIV_SHIFT0
> #define DSI_PLL_M1_DIV_MASK (0x1ff << 0)
> #define CCK_DISPLAY_CLOCK_CONTROL0x6b
> +#define CCK_DISPLAY_REF_CLOCK_CONTROL0x6c
> #define DISPLAY_TRUNK_FORCE_ON (1 << 17)
> #define DISPLAY_TRUNK_FORCE_OFF (1 << 16)
> #define DISPLAY_FREQUENCY_STATUS(0x1f << 8)
> #define DISPLAY_FREQUENCY_STATUS_SHIFT 8
> #define DISPLAY_FREQUENCY_VALUES(0x1f << 0)
>
> +#define CCU_ICLK_5 0x114
> +#define DISPSS_CLKREQ (1 << 1)
> +#define DISPBEND_CLKREQ (1 << 0)
> +
> /**
> * DOC: DPIO
> *
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d8e20d3..96614c3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6057,6 +6057,20 @@ static void vlv_display_power_well_enable(struct
> drm_i915_private *dev_priv,
> {
> WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DISP2D);
>
> + mutex_lock(&dev_priv->dpio_lock);
> + /*
> + * (re)enable ref clocks at CCU
> + * FIXME maybe move to cmnlane?
> + */
> + vlv_ccu_write(dev_priv, CCU_ICLK_5,
> + vlv_ccu_read(dev_priv, CCU_ICLK_5) |
> + DISPBEND_CLKREQ | DISPSS_CLKREQ);
> + /*
> + * Punit clears CCK trunk force off bits
> + * automagically while ungating disp2d
> + */
> + mutex_unlock(&dev_priv->dpio_lock);
> +
> vlv_set_power_well(dev_priv, power_well, true);
>
> spin_lock_irq(&dev_priv->irq_lock);
> @@ -6085,6 +6099,27 @@ static void vlv_display_power_well_disable(struct
> drm_i915_private *dev_priv,
> spin_unlock_irq(&dev_priv->irq_lock);
>
> vlv_set_power_well(dev_priv, power_well, false);
> +
> + mutex_lock(&dev_priv->dpio_lock);
> + /*
> + * Punit doesn't set the CCK trunk force off bits when power gating
> + * disp2d. It does set them when clock gating disp2d, but we ask it
> + * to power gate instead of clock gate.
> + */
> + vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL,
> + vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL) |
> + DISPLAY_TRUNK_FORCE_OFF);
> + vlv_cck_write(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL,
> + vlv_cck_read(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL) |
> + DISPLAY_TRUNK_FORCE_OFF);
> + /*
> + * disable ref clocks at CCU
> + * FIXME maybe move to cmnlane?
> + */
> + vlv_ccu_write(dev_priv, CCU_ICLK_5,
> + vlv_ccu_read(dev_priv, CCU_ICLK_5) &
> + ~(DISPBEND_CLKREQ | DISPSS_CLKREQ));
> + mutex_unlock(&dev_priv->dpio_lock);
> }
>
> static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
I guess you could ping the hw folks about this, maybe starting with
Cesar, to see if we can drop the power any further by doing this or
poking some other reg...
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 16/19] drm/i915: ->enable hook for WRPLLs
From: Daniel Vetter
This time around another cute hack to pre-fill the pll->hw_state with
the right values. And also remove a bunch of checks which will be
replaced by lots more checks in the common framework.
Signed-off-by: Daniel Vetter
Reviewed-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_ddi.c | 51 +++-
1 file changed, 19 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 61c2471..d1569d0 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -740,6 +740,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
intel_ddi_put_crtc_pll(crtc);
if (type == INTEL_OUTPUT_HDMI) {
+ struct intel_shared_dpll *pll;
uint32_t reg, val;
unsigned p, n2, r2;
@@ -784,6 +785,9 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
}
intel_crtc->config.dpll_hw_state.wrpll = val;
+
+ pll = &dev_priv->shared_dplls[intel_crtc->config.shared_dpll];
+ pll->hw_state.wrpll = val;
}
return true;
@@ -798,54 +802,24 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
- int clock = crtc->config.port_clock;
- uint32_t reg, cur_val, new_val;
int refcount;
- const char *pll_name;
- uint32_t enable_bit = (1 << 31);
- unsigned int p, n2, r2;
-
- BUILD_BUG_ON(enable_bit != SPLL_PLL_ENABLE);
- BUILD_BUG_ON(enable_bit != WRPLL_PLL_ENABLE);
+ struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
switch (crtc->config.ddi_pll_sel) {
case PORT_CLK_SEL_WRPLL1:
case PORT_CLK_SEL_WRPLL2:
if (crtc->config.ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
- pll_name = "WRPLL1";
- reg = WRPLL_CTL1;
refcount = plls->wrpll1_refcount;
} else {
- pll_name = "WRPLL2";
- reg = WRPLL_CTL2;
refcount = plls->wrpll2_refcount;
}
-
- intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
-
- new_val = WRPLL_PLL_ENABLE | WRPLL_PLL_LCPLL |
- WRPLL_DIVIDER_REFERENCE(r2) |
- WRPLL_DIVIDER_FEEDBACK(n2) | WRPLL_DIVIDER_POST(p);
-
break;
-
- case PORT_CLK_SEL_NONE:
- WARN(1, "Bad selected pll: PORT_CLK_SEL_NONE\n");
- return;
default:
return;
}
- cur_val = I915_READ(reg);
-
- WARN(refcount < 1, "Bad %s refcount: %d\n", pll_name, refcount);
if (refcount == 1) {
- WARN(cur_val & enable_bit, "%s already enabled\n", pll_name);
- I915_WRITE(reg, new_val);
- POSTING_READ(reg);
- udelay(20);
- } else {
- WARN((cur_val & enable_bit) == 0, "%s disabled\n", pll_name);
+ pll->enable(dev_priv, pll);
}
}
@@ -1309,6 +1283,18 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private
*dev_priv)
}
}
+static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
+ struct intel_shared_dpll *pll)
+{
+ uint32_t cur_val;
+
+ cur_val = I915_READ(WRPLL_CTL(pll->id));
+ WARN(cur_val & WRPLL_PLL_ENABLE, "%s already enabled\n", pll->name);
+ I915_WRITE(WRPLL_CTL(pll->id), pll->hw_state.wrpll);
+ POSTING_READ(WRPLL_CTL(pll->id));
+ udelay(20);
+}
+
static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
struct intel_shared_dpll *pll)
{
@@ -1351,6 +1337,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
dev_priv->shared_dplls[i].id = i;
dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
dev_priv->shared_dplls[i].disable = hsw_ddi_pll_disable;
+ dev_priv->shared_dplls[i].enable = hsw_ddi_pll_enable;
dev_priv->shared_dplls[i].get_hw_state =
hsw_ddi_pll_get_hw_state;
}
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/19] drm/i915: Add a debugfs file for the shared dpll state
From: Daniel Vetter
Signed-off-by: Daniel Vetter
Reviewed-by: Damien Lespiau
---
drivers/gpu/drm/i915/i915_debugfs.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index a93b3bf..a0d8733 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2388,6 +2388,31 @@ static int i915_display_info(struct seq_file *m, void
*unused)
return 0;
}
+static int i915_shared_dplls_info(struct seq_file *m, void *unused)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_device *dev = node->minor->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ int i;
+
+ drm_modeset_lock_all(dev);
+ for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+ struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
+
+ seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->name, pll->id);
+ seq_printf(m, " refcount: %i, active: %i, on: %s\n",
pll->refcount,
+ pll->active, yesno(pll->on));
+ seq_printf(m, " tracked hardware state:\n");
+ seq_printf(m, " dpll:0x%08x\n", pll->hw_state.dpll);
+ seq_printf(m, " dpll_md: 0x%08x\n", pll->hw_state.dpll_md);
+ seq_printf(m, " fp0: 0x%08x\n", pll->hw_state.fp0);
+ seq_printf(m, " fp1: 0x%08x\n", pll->hw_state.fp1);
+ }
+ drm_modeset_unlock_all(dev);
+
+ return 0;
+}
+
struct pipe_crc_info {
const char *name;
struct drm_device *dev;
@@ -3839,6 +3864,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_pc8_status", i915_pc8_status, 0},
{"i915_power_domain_info", i915_power_domain_info, 0},
{"i915_display_info", i915_display_info, 0},
+ {"i915_shared_dplls_info", i915_shared_dplls_info, 0},
};
#define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/19] drm/i915: Move the SPLL enabling into hsw_crt_pre_enable
From: Daniel Vetter
The call to intel_ddi_pll_enable in haswell_crtc_mode_set is the only
function that still touches the hardware state from the crtc mode_set
callback on hsw. Since the SPLL isn't ever shared we can easily take
it out into the hsw crt encoder functions.
Temporarily we'll loose a bit of WARN_ON coverage with this, but once
the WRPLLs are switched over that will be restored. For the SPLL
selection add a WARN in the hsw fdi link training code.
Signed-off-by: Daniel Vetter
[imre: rebased on patchset version w/o pch/crt/fdi refactoring]
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_crt.c | 13 +
drivers/gpu/drm/i915/intel_ddi.c | 19 +--
2 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 8da5ef9..a4bdf257 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -137,6 +137,18 @@ static void hsw_crt_get_config(struct intel_encoder
*encoder,
pipe_config->adjusted_mode.flags |= intel_crt_get_flags(encoder);
}
+static void hsw_crt_pre_enable(struct intel_encoder *encoder)
+{
+ struct drm_device *dev = encoder->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ WARN(I915_READ(SPLL_CTL) & SPLL_PLL_ENABLE, "SPLL already enabled\n");
+ I915_WRITE(SPLL_CTL,
+ SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC);
+ POSTING_READ(SPLL_CTL);
+ udelay(20);
+}
+
/* Note: The caller is required to filter out dpms modes not supported by the
* platform. */
static void intel_crt_set_dpms(struct intel_encoder *encoder, int mode)
@@ -860,6 +872,7 @@ void intel_crt_init(struct drm_device *dev)
if (HAS_DDI(dev)) {
crt->base.get_config = hsw_crt_get_config;
crt->base.get_hw_state = intel_ddi_get_hw_state;
+ crt->base.pre_enable = hsw_crt_pre_enable;
} else {
crt->base.get_config = intel_crt_get_config;
crt->base.get_hw_state = intel_crt_get_hw_state;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 16c9163..accacb9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -278,6 +278,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
/* Configure Port Clock Select */
I915_WRITE(PORT_CLK_SEL(PORT_E), intel_crtc->ddi_pll_sel);
+ WARN_ON(intel_crtc->ddi_pll_sel != PORT_CLK_SEL_SPLL);
/* Start the training iterating through available voltages and emphasis,
* testing each value twice. */
@@ -848,23 +849,6 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
BUILD_BUG_ON(enable_bit != WRPLL_PLL_ENABLE);
switch (crtc->ddi_pll_sel) {
- case PORT_CLK_SEL_LCPLL_2700:
- case PORT_CLK_SEL_LCPLL_1350:
- case PORT_CLK_SEL_LCPLL_810:
- /*
-* LCPLL should always be enabled at this point of the mode set
-* sequence, so nothing to do.
-*/
- return;
-
- case PORT_CLK_SEL_SPLL:
- new_val = SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz |
- SPLL_PLL_SSC;
- WARN(I915_READ(SPLL_CTL) & enable_bit, "SPLL already
enabled\n");
- I915_WRITE(SPLL_CTL, new_val);
- POSTING_READ(SPLL_CTL);
- udelay(20);
- return;
case PORT_CLK_SEL_WRPLL1:
case PORT_CLK_SEL_WRPLL2:
if (crtc->ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
@@ -889,7 +873,6 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
WARN(1, "Bad selected pll: PORT_CLK_SEL_NONE\n");
return;
default:
- WARN(1, "Bad selected pll: 0x%08x\n", crtc->ddi_pll_sel);
return;
}
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/19] drm/i915: ddi: enable runtime pm during dpms
From: Daniel Vetter
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_display.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 62b6896..35e7c89 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4920,12 +4920,10 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
* yet, so do runtime PM for DPMS only for all other
* platforms for now.
*/
- if (!HAS_DDI(dev)) {
- domains = get_crtc_power_domains(crtc);
- for_each_power_domain(domain, domains)
- intel_display_power_get(dev_priv,
domain);
- intel_crtc->enabled_power_domains = domains;
- }
+ domains = get_crtc_power_domains(crtc);
+ for_each_power_domain(domain, domains)
+ intel_display_power_get(dev_priv, domain);
+ intel_crtc->enabled_power_domains = domains;
dev_priv->display.crtc_enable(crtc);
}
@@ -4933,12 +4931,10 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
if (intel_crtc->active) {
dev_priv->display.crtc_disable(crtc);
- if (!HAS_DDI(dev)) {
- domains = intel_crtc->enabled_power_domains;
- for_each_power_domain(domain, domains)
- intel_display_power_put(dev_priv,
domain);
- intel_crtc->enabled_power_domains = 0;
- }
+ domains = intel_crtc->enabled_power_domains;
+ for_each_power_domain(domain, domains)
+ intel_display_power_put(dev_priv, domain);
+ intel_crtc->enabled_power_domains = 0;
}
}
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders
From: Daniel Vetter
This way only the dynamic WRPLL selection for hdmi ddi mode is
done in intel_ddi_pll_select.
v2: Don't clobber the precomputed values when selecting clocks fro
hdmi encoders.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_crt.c | 4 +++-
drivers/gpu/drm/i915/intel_ddi.c | 34 +++---
drivers/gpu/drm/i915/intel_dp.c | 23 ---
3 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 76ffa2c..88db4b6 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -315,8 +315,10 @@ static bool intel_crt_compute_config(struct intel_encoder
*encoder,
pipe_config->pipe_bpp = 24;
/* FDI must always be 2.7 GHz */
- if (HAS_DDI(dev))
+ if (HAS_DDI(dev)) {
+ pipe_config->ddi_pll_sel = PORT_CLK_SEL_SPLL;
pipe_config->port_clock = 135000 * 2;
+ }
return true;
}
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 5356e3e..6e976ba 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -403,6 +403,7 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
I915_WRITE(WRPLL_CTL1, val & ~WRPLL_PLL_ENABLE);
POSTING_READ(WRPLL_CTL1);
}
+ intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
break;
case PORT_CLK_SEL_WRPLL2:
plls->wrpll2_refcount--;
@@ -413,13 +414,12 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
I915_WRITE(WRPLL_CTL2, val & ~WRPLL_PLL_ENABLE);
POSTING_READ(WRPLL_CTL2);
}
+ intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
break;
}
WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
-
- intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
}
#define LC_FREQ 2700
@@ -739,7 +739,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
{
struct drm_crtc *crtc = &intel_crtc->base;
struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
- struct drm_encoder *encoder = &intel_encoder->base;
struct drm_i915_private *dev_priv = crtc->dev->dev_private;
struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
int type = intel_encoder->type;
@@ -748,26 +747,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
intel_ddi_put_crtc_pll(crtc);
- if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
- struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-
- switch (intel_dp->link_bw) {
- case DP_LINK_BW_1_62:
- intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
- break;
- case DP_LINK_BW_2_7:
- intel_crtc->config.ddi_pll_sel =
PORT_CLK_SEL_LCPLL_1350;
- break;
- case DP_LINK_BW_5_4:
- intel_crtc->config.ddi_pll_sel =
PORT_CLK_SEL_LCPLL_2700;
- break;
- default:
- DRM_ERROR("Link bandwidth %d unsupported\n",
- intel_dp->link_bw);
- return false;
- }
-
- } else if (type == INTEL_OUTPUT_HDMI) {
+ if (type == INTEL_OUTPUT_HDMI) {
uint32_t reg, val;
unsigned p, n2, r2;
@@ -808,14 +788,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
plls->wrpll2_refcount++;
intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
}
-
- } else if (type == INTEL_OUTPUT_ANALOG) {
- DRM_DEBUG_KMS("Using SPLL on pipe %c\n",
- pipe_name(pipe));
- intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_SPLL;
- } else {
- WARN(1, "Invalid DDI encoder type %d\n", type);
- return false;
}
return true;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec489..9a2ede0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -746,6 +746,22 @@ intel_dp_connector_unregister(struct intel_connector
*intel_connector)
}
static void
+hsw_dp_set_ddi_pll_sel(struct intel_crtc_config *pipe_config, int link_bw)
+{
+ switch (link_bw) {
+ case DP_LINK_BW_1_62:
+ pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
+ break;
+ case DP_LINK_BW_2_7:
+ pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_1350;
+ break;
+ case DP_LINK_BW_5_4:
+
[Intel-gfx] [PATCH 14/19] drm/i915: State readout support for WRPLLs
From: Daniel Vetter
Still tacked onto the side, but slowly getting there.
v2: Don't forget the debugfs file.
Signed-off-by: Daniel Vetter
Reviewed-by: Damien Lespiau
---
drivers/gpu/drm/i915/i915_debugfs.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_ddi.c | 16
drivers/gpu/drm/i915/intel_display.c | 9 +
5 files changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index a0d8733..2dee463 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2407,6 +2407,7 @@ static int i915_shared_dplls_info(struct seq_file *m,
void *unused)
seq_printf(m, " dpll_md: 0x%08x\n", pll->hw_state.dpll_md);
seq_printf(m, " fp0: 0x%08x\n", pll->hw_state.fp0);
seq_printf(m, " fp1: 0x%08x\n", pll->hw_state.fp1);
+ seq_printf(m, " wrpll: 0x%08x\n", pll->hw_state.wrpll);
}
drm_modeset_unlock_all(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 650a5ad..855f055 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -196,6 +196,7 @@ struct intel_dpll_hw_state {
uint32_t dpll_md;
uint32_t fp0;
uint32_t fp1;
+ uint32_t wrpll;
};
struct intel_shared_dpll {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3d61a53..654417e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5900,6 +5900,7 @@ enum punit_power_well {
/* WRPLL */
#define WRPLL_CTL1 0x46040
#define WRPLL_CTL2 0x46060
+#define WRPLL_CTL(pll) (pll == 0 ? WRPLL_CTL1 : WRPLL_CTL2)
#define WRPLL_PLL_ENABLE (1<<31)
#define WRPLL_PLL_SSC (1<<28)
#define WRPLL_PLL_NON_SSC (2<<28)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index fae73d3..9539405 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -790,6 +790,8 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
intel_crtc->config.shared_dpll = DPLL_ID_WRPLL2;
}
+
+ intel_crtc->config.dpll_hw_state.wrpll = val;
}
return true;
@@ -1315,6 +1317,18 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private
*dev_priv)
}
}
+static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
+struct intel_shared_dpll *pll,
+struct intel_dpll_hw_state *hw_state)
+{
+ uint32_t val;
+
+ val = I915_READ(WRPLL_CTL(pll->id));
+ hw_state->wrpll = val;
+
+ return val & WRPLL_PLL_ENABLE;
+}
+
static char *hsw_ddi_pll_names[] = {
"WRPLL 1",
"WRPLL 2",
@@ -1333,6 +1347,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
for (i = 0; i < 2; i++) {
dev_priv->shared_dplls[i].id = i;
dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
+ dev_priv->shared_dplls[i].get_hw_state =
+ hsw_ddi_pll_get_hw_state;
}
/* The LCPLL register should be turned on by the BIOS. For now let's
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2b83c07..d8277a1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7574,6 +7574,7 @@ static void haswell_get_ddi_port_state(struct intel_crtc
*crtc,
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_shared_dpll *pll;
enum port port;
uint32_t tmp;
@@ -7592,6 +7593,13 @@ static void haswell_get_ddi_port_state(struct intel_crtc
*crtc,
break;
}
+ if (pipe_config->shared_dpll >= 0) {
+ pll = &dev_priv->shared_dplls[pipe_config->shared_dpll];
+
+ WARN_ON(!pll->get_hw_state(dev_priv, pll,
+ &pipe_config->dpll_hw_state));
+ }
+
/*
* Haswell has only FDI/PCH transcoder A. It is which is connected to
* DDI E. So just check whether this pipe is wired to DDI E and whether
@@ -10423,6 +10431,7 @@ intel_pipe_config_compare(struct drm_device *dev,
PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
+ PIPE_CONF_CHECK_X(dpll_hw_state.wrpll);
if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
PIPE_CONF_CHECK_I(pipe_bpp);
--
1.8.4
___
Intel-gfx
Re: [Intel-gfx] [PATCH 10/11] drm/i915: Move VLV cmnlane workaround to intel_power_domains_init_hw()
On Fri, 13 Jun 2014 13:37:56 +0300
ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Now that the CMNRESET deassert is part of the cmnlane power well,
> intel_reset_dpio() is called too late to make any difference. We've
> deasserted CMNRESET by that time, and so the off+on toggle w/a will
> never kick in.
>
> Move the workaround to intel_power_domains_init_hw() where it gets
> called before we enable the init power domain.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 23 -
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_pm.c | 67
> ++--
> 3 files changed, 58 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 33cc213..bcd32322 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1508,9 +1508,6 @@ static void intel_reset_dpio(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - if (!IS_VALLEYVIEW(dev))
> - return;
> -
> if (IS_CHERRYVIEW(dev)) {
> enum dpio_phy phy;
> u32 val;
> @@ -1532,26 +1529,6 @@ static void intel_reset_dpio(struct drm_device *dev)
> I915_WRITE(DISPLAY_PHY_CONTROL,
> PHY_COM_LANE_RESET_DEASSERT(phy, val));
> }
> -
> - } else {
> - /*
> - * If DPIO has already been reset, e.g. by BIOS, just skip all
> - * this.
> - */
> - if (I915_READ(DPIO_CTL) & DPIO_CMNRST)
> - return;
> -
> - /*
> - * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
> - * Need to assert and de-assert PHY SB reset by gating the
> - * common lane power, then un-gating it.
> - * Simply ungating isn't enough to reset the PHY enough to get
> - * ports and lanes running.
> - */
> - __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> - false);
> - __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> - true);
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 5740be0..e565906 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -970,8 +970,7 @@ void intel_runtime_pm_put(struct drm_i915_private
> *dev_priv);
> void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
> void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
> void ilk_wm_get_hw_state(struct drm_device *dev);
> -void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> - enum punit_power_well power_well_id, bool enable);
> +
>
> /* intel_sdvo.c */
> bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool
> is_sdvob);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e9a8565..d8e20d3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5962,9 +5962,10 @@ static bool i9xx_always_on_power_well_enabled(struct
> drm_i915_private *dev_priv,
> return true;
> }
>
> -void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> - enum punit_power_well power_well_id, bool enable)
> +static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> +struct i915_power_well *power_well, bool enable)
> {
> + enum punit_power_well power_well_id = power_well->data;
> u32 mask;
> u32 state;
> u32 ctrl;
> @@ -5997,14 +5998,6 @@ out:
> mutex_unlock(&dev_priv->rps.hw_lock);
> }
>
> -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> -struct i915_power_well *power_well, bool enable)
> -{
> - enum punit_power_well power_well_id = power_well->data;
> -
> - __vlv_set_power_well(dev_priv, power_well_id, enable);
> -}
> -
> static void vlv_power_well_sync_hw(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> @@ -6435,6 +6428,21 @@ static struct i915_power_well vlv_power_wells[] = {
> },
> };
>
> +static struct i915_power_well *lookup_power_well(struct drm_i915_private
> *dev_priv,
> + enum punit_power_well
> power_well_id)
> +{
> + struct i915_power_domains *power_domains = &dev_priv->power_domains;
> + struct i915_power_well *power_well;
> + int i;
> +
> + for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
> + if (power_well->data == power_well_id)
> + return power_well;
> + }
> +
> + return NUL
[Intel-gfx] [PATCH 01/19] drm/i915: Check hw state in assert_can_disable_lcpll
From: Daniel Vetter
All the other checks also check hw state, so checking our software
refcounts for the plls looks a bit odd. Also this will simplify the
conversion over to the shared dpll framework, which itself has massive
amounts of checks to make sure that we never leave a display pll
enabled when we shouldn't.
So after that conversion we should stil have a good enough coverage of
asserts for entering pc8/runtime pm on hsw/bdw.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 065984d..2442a88 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7322,7 +7322,6 @@ static bool ironlake_get_pipe_config(struct intel_crtc
*crtc,
static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv->dev;
- struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
struct intel_crtc *crtc;
for_each_intel_crtc(dev, crtc)
@@ -7330,9 +7329,9 @@ static void assert_can_disable_lcpll(struct
drm_i915_private *dev_priv)
pipe_name(crtc->pipe));
WARN(I915_READ(HSW_PWR_WELL_DRIVER), "Power well on\n");
- WARN(plls->spll_refcount, "SPLL enabled\n");
- WARN(plls->wrpll1_refcount, "WRPLL1 enabled\n");
- WARN(plls->wrpll2_refcount, "WRPLL2 enabled\n");
+ WARN(I915_READ(SPLL_CTL) & SPLL_PLL_ENABLE, "SPLL enabled\n");
+ WARN(I915_READ(WRPLL_CTL1) & WRPLL_PLL_ENABLE, "WRPLL1 enabled\n");
+ WARN(I915_READ(WRPLL_CTL2) & WRPLL_PLL_ENABLE, "WRPLL2 enabled\n");
WARN(I915_READ(PCH_PP_STATUS) & PP_ON, "Panel power on\n");
WARN(I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE,
"CPU PWM1 enabled\n");
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/19] drm/i915: Clean up WRPLL/SPLL #defines
From: Daniel Vetter
Luckily the bit definitions match, but it's still confusing
to use one when handling the other. So sprinkle some OCD over
the #defines to make them match and use the right version in
each place.
Maybe we should unify these definitions completely, but that
can always be done sometime in the future.
Signed-off-by: Daniel Vetter
Reviewed-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_reg.h | 7 ---
drivers/gpu/drm/i915/intel_ddi.c | 12 ++--
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3488567..5db1959 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5900,9 +5900,10 @@ enum punit_power_well {
#define WRPLL_CTL1 0x46040
#define WRPLL_CTL2 0x46060
#define WRPLL_PLL_ENABLE (1<<31)
-#define WRPLL_PLL_SELECT_SSC (0x01<<28)
-#define WRPLL_PLL_SELECT_NON_SSC (0x02<<28)
-#define WRPLL_PLL_SELECT_LCPLL_2700 (0x03<<28)
+#define WRPLL_PLL_SSC (1<<28)
+#define WRPLL_PLL_NON_SSC (2<<28)
+#define WRPLL_PLL_LCPLL (3<<28)
+#define WRPLL_PLL_REF_MASK(3<<28)
/* WRPLL divider programming */
#define WRPLL_DIVIDER_REFERENCE(x)((x)<<0)
#define WRPLL_DIVIDER_REF_MASK(0xff)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9f02281..16c9163 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -588,9 +588,9 @@ static int intel_ddi_calc_wrpll_link(struct
drm_i915_private *dev_priv,
u32 wrpll;
wrpll = I915_READ(reg);
- switch (wrpll & SPLL_PLL_REF_MASK) {
- case SPLL_PLL_SSC:
- case SPLL_PLL_NON_SSC:
+ switch (wrpll & WRPLL_PLL_REF_MASK) {
+ case WRPLL_PLL_SSC:
+ case WRPLL_PLL_NON_SSC:
/*
* We could calculate spread here, but our checking
* code only cares about 5% accuracy, and spread is a max of
@@ -598,7 +598,7 @@ static int intel_ddi_calc_wrpll_link(struct
drm_i915_private *dev_priv,
*/
refclk = 135;
break;
- case SPLL_PLL_LCPLL:
+ case WRPLL_PLL_LCPLL:
refclk = LC_FREQ;
break;
default:
@@ -780,7 +780,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
- val = WRPLL_PLL_ENABLE | WRPLL_PLL_SELECT_LCPLL_2700 |
+ val = WRPLL_PLL_ENABLE | WRPLL_PLL_LCPLL |
WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
WRPLL_DIVIDER_POST(p);
@@ -879,7 +879,7 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
- new_val = WRPLL_PLL_ENABLE | WRPLL_PLL_SELECT_LCPLL_2700 |
+ new_val = WRPLL_PLL_ENABLE | WRPLL_PLL_LCPLL |
WRPLL_DIVIDER_REFERENCE(r2) |
WRPLL_DIVIDER_FEEDBACK(n2) | WRPLL_DIVIDER_POST(p);
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/19] ddi: respin of runtime PM for DPMS
This is a respin of the unmerged part of Daniel's runtime PM for DPMS patchset [1]. The original one also included a refactoring of the DDI PCH/CRT encoder modesetting path, I left the corresponding patches out from this series. This is because there hasn't been yet an agreement on those parts, but people would like to see the RPM DPMS support already applied. Some patches needed to be updated/rebased because of the above omission, but these weren't anywhere significant so I just marked the fact with my s-o-b line. I also added two minor change to keep the modeset sequence at its current order and collected all the reviewed-by lines. Tested on HSW DP/VGA, with basic DPMS on/off and igt/pm_rpm. [1] http://lists.freedesktop.org/archives/intel-gfx/2014-April/044179.html Daniel Vetter (17): drm/i915: Check hw state in assert_can_disable_lcpll drm/i915: Remove spll_refcount for hsw drm/i915: Clean up WRPLL/SPLL #defines drm/i915: Move the SPLL enabling into hsw_crt_pre_enable drm/i915: Move SPLL disabling into hsw_crt_post_disable drm/i915: Add a debugfs file for the shared dpll state drm/i915: Move ddi_pll_sel into the pipe config drm/i915: State readout and cross-checking for ddi_pll_sel drm/i915: Precompute static ddi_pll_sel values in encoders drm/i915: Basic shared dpll support for WRPLLs drm/i915: Document that the pll->mode_set hook is optional drm/i915: State readout support for WRPLLs drm/i915: ->disable hook for WRPLLs drm/i915: ->enable hook for WRPLLs drm/i915: Switch to common shared dpll framework for WRPLLs drm/i915: Only touch WRPLL hw state in enable/disable hooks drm/i915: ddi: enable runtime pm during dpms Imre Deak (2): drm/i915: ddi: move pch setup after encoder->pre_enable drm/i915: ddi: move pch cleanup before encoder->post_disable drivers/gpu/drm/i915/i915_debugfs.c | 27 +++ drivers/gpu/drm/i915/i915_drv.h | 16 +- drivers/gpu/drm/i915/i915_reg.h | 10 +- drivers/gpu/drm/i915/intel_crt.c | 32 +++- drivers/gpu/drm/i915/intel_ddi.c | 340 +++ drivers/gpu/drm/i915/intel_display.c | 157 +--- drivers/gpu/drm/i915/intel_dp.c | 23 ++- drivers/gpu/drm/i915/intel_drv.h | 14 +- 8 files changed, 258 insertions(+), 361 deletions(-) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/19] drm/i915: ddi: move pch setup after encoder->pre_enable
This is needed by an upcoming patch that moves the PCH/CRT PLL enabling
into the pre_enable hook, after which we want to keep the modeset
sequence at its current state. At this point this won't have an effect
since the PCH/CRT pre_enable hook is atm a NOP.
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_display.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2442a88..b480d86 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4145,15 +4145,14 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
intel_crtc->active = true;
intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
- if (intel_crtc->config.has_pch_encoder)
- intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
+ for_each_encoder_on_crtc(dev, crtc, encoder)
+ if (encoder->pre_enable)
+ encoder->pre_enable(encoder);
- if (intel_crtc->config.has_pch_encoder)
+ if (intel_crtc->config.has_pch_encoder) {
+ intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
dev_priv->display.fdi_link_train(crtc);
-
- for_each_encoder_on_crtc(dev, crtc, encoder)
- if (encoder->pre_enable)
- encoder->pre_enable(encoder);
+ }
intel_ddi_enable_pipe_clock(intel_crtc);
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/11] drm/i915: Pull the cmnlane tricks into its own power well ops
On Fri, 13 Jun 2014 13:37:55 +0300
ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Remove the clutter in __vlv_set_power_well() by moving the cmnlane
> handling into custom enable/disable hooks for the cmnlane.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_pm.c | 92
> -
> 1 file changed, 55 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 9d7b082..e9a8565 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5965,31 +5965,9 @@ static bool i9xx_always_on_power_well_enabled(struct
> drm_i915_private *dev_priv,
> void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> enum punit_power_well power_well_id, bool enable)
> {
> - struct drm_device *dev = dev_priv->dev;
> u32 mask;
> u32 state;
> u32 ctrl;
> - enum pipe pipe;
> -
> - if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC) {
> - if (enable) {
> - /*
> - * Enable the CRI clock source so we can get at the
> - * display and the reference clock for VGA
> - * hotplug / manual detection.
> - */
> - I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> -DPLL_REFA_CLK_ENABLE_VLV |
> -DPLL_INTEGRATED_CRI_CLK_VLV);
> - udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
> - } else {
> - for_each_pipe(pipe)
> - assert_pll_disabled(dev_priv, pipe);
> - /* Assert common reset */
> - I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) &
> -~DPIO_CMNRST);
> - }
> - }
>
> mask = PUNIT_PWRGT_MASK(power_well_id);
> state = enable ? PUNIT_PWRGT_PWR_ON(power_well_id) :
> @@ -6017,20 +5995,6 @@ void __vlv_set_power_well(struct drm_i915_private
> *dev_priv,
>
> out:
> mutex_unlock(&dev_priv->rps.hw_lock);
> -
> - /*
> - * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
> - * 6. De-assert cmn_reset/side_reset. Same as VLV X0.
> - * a. GUnit 0x2110 bit[0] set to 1 (def 0)
> - * b. The other bits such as sfr settings / modesel may all
> - * be set to 0.
> - *
> - * This should only be done on init and resume from S3 with
> - * both PLLs disabled, or we risk losing DPIO and PLL
> - * synchronization.
> - */
> - if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC && enable)
> - I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST);
> }
>
> static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> @@ -6130,6 +6094,53 @@ static void vlv_display_power_well_disable(struct
> drm_i915_private *dev_priv,
> vlv_set_power_well(dev_priv, power_well, false);
> }
>
> +static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
> +struct i915_power_well *power_well)
> +{
> + WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC);
> +
> + /*
> + * Enable the CRI clock source so we can get at the
> + * display and the reference clock for VGA
> + * hotplug / manual detection.
> + */
> + I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> +DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> + udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
> +
> + vlv_set_power_well(dev_priv, power_well, true);
> +
> + /*
> + * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
> + * 6. De-assert cmn_reset/side_reset. Same as VLV X0.
> + * a. GUnit 0x2110 bit[0] set to 1 (def 0)
> + * b. The other bits such as sfr settings / modesel may all
> + * be set to 0.
> + *
> + * This should only be done on init and resume from S3 with
> + * both PLLs disabled, or we risk losing DPIO and PLL
> + * synchronization.
> + */
> + I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST);
> +}
> +
> +static void vlv_dpio_cmn_power_well_disable(struct drm_i915_private
> *dev_priv,
> + struct i915_power_well *power_well)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + enum pipe pipe;
> +
> + WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC);
> +
> + for_each_pipe(pipe)
> + assert_pll_disabled(dev_priv, pipe);
> +
> + /* Assert common reset */
> + I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) & ~DPIO_CMNRST);
> +
> + vlv_set_power_well(dev_priv, power_well, false);
> +}
> +
> static void check_power_well_state(struct drm_i915_private *dev_priv,
> struct i915_pow
Re: [Intel-gfx] [PATCH 08/11] drm/i915: Kill duplicated cdclk readout code from i2c
On Fri, 13 Jun 2014 13:37:54 +0300
ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> We have a slightly different way of readoing out the cdclk in
> gmbus_set_freq(). Kill that and just call .get_display_clock_speed().
>
> Also need to remove the GMBUSFREQ update from intel_i2c_reset() since
> that gets called way too early. Let's do it in intel_modeset_init_hw()
> instead, and also pull the initial vlv_cdclk_freq update there from
> init_clock gating.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 25 ++---
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> drivers/gpu/drm/i915/intel_i2c.c | 54
>
> drivers/gpu/drm/i915/intel_pm.c | 4 ---
> 4 files changed, 21 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 601e97e..33cc213 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4430,7 +4430,7 @@ static void modeset_update_crtc_power_domains(struct
> drm_device *dev)
> }
>
> /* returns HPLL frequency in kHz */
> -int valleyview_get_vco(struct drm_i915_private *dev_priv)
> +static int valleyview_get_vco(struct drm_i915_private *dev_priv)
> {
> int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
>
> @@ -4443,6 +4443,22 @@ int valleyview_get_vco(struct drm_i915_private
> *dev_priv)
> return vco_freq[hpll_freq] * 1000;
> }
>
> +static void vlv_update_cdclk(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + dev_priv->vlv_cdclk_freq =
> dev_priv->display.get_display_clock_speed(dev);
> + DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
> + dev_priv->vlv_cdclk_freq);
> +
> + /*
> + * Program the gmbus_freq based on the cdclk frequency.
> + * BSpec erroneously claims we should aim for 4MHz, but
> + * in fact 1MHz is the correct frequency.
> + */
> + I915_WRITE(GMBUSFREQ_VLV, dev_priv->vlv_cdclk_freq);
> +}
> +
> /* Adjust CDclk dividers to allow high res or save power if possible */
> static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
> {
> @@ -4450,7 +4466,6 @@ static void valleyview_set_cdclk(struct drm_device
> *dev, int cdclk)
> u32 val, cmd;
>
> WARN_ON(dev_priv->display.get_display_clock_speed(dev) !=
> dev_priv->vlv_cdclk_freq);
> - dev_priv->vlv_cdclk_freq = cdclk;
>
> if (cdclk >= 32) /* jump to highest voltage for 400MHz too */
> cmd = 2;
> @@ -4507,8 +4522,7 @@ static void valleyview_set_cdclk(struct drm_device
> *dev, int cdclk)
> vlv_bunit_write(dev_priv, BUNIT_REG_BISOC, val);
> mutex_unlock(&dev_priv->dpio_lock);
>
> - /* Since we changed the CDclk, we need to update the GMBUSFREQ too */
> - intel_i2c_reset(dev);
> + vlv_update_cdclk(dev);
> }
>
> static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> @@ -11974,6 +11988,9 @@ void intel_modeset_init_hw(struct drm_device *dev)
> {
> intel_prepare_ddi(dev);
>
> + if (IS_VALLEYVIEW(dev))
> + vlv_update_cdclk(dev);
> +
> intel_init_clock_gating(dev);
>
> intel_reset_dpio(dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 65ce0bb..5740be0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -800,7 +800,6 @@ void hsw_disable_ips(struct intel_crtc *crtc);
> void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
> enum intel_display_power_domain
> intel_display_port_power_domain(struct intel_encoder *intel_encoder);
> -int valleyview_get_vco(struct drm_i915_private *dev_priv);
> void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>struct intel_crtc_config *pipe_config);
> int intel_format_to_fourcc(int format);
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c
> b/drivers/gpu/drm/i915/intel_i2c.c
> index 9ce4f09..b31088a 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -34,11 +34,6 @@
> #include
> #include "i915_drv.h"
>
> -enum disp_clk {
> - CDCLK,
> - CZCLK
> -};
> -
> struct gmbus_port {
> const char *name;
> int reg;
> @@ -63,60 +58,11 @@ to_intel_gmbus(struct i2c_adapter *i2c)
> return container_of(i2c, struct intel_gmbus, adapter);
> }
>
> -static int get_disp_clk_div(struct drm_i915_private *dev_priv,
> - enum disp_clk clk)
> -{
> - u32 reg_val;
> - int clk_ratio;
> -
> - reg_val = I915_READ(CZCLK_CDCLK_FREQ_RATIO);
> -
> - if (clk == CDCLK)
> - clk_ratio =
> - ((reg_val & CDCLK_FREQ_MASK) >> CDCLK_FREQ_SHIFT) + 1;
> - else
> - clk_ratio = (reg_val & CZCLK_FREQ_MASK) + 1;
> -
> - return clk_ratio;
> -
Re: [Intel-gfx] [PATCH 07/11] drm/i915: Warn if there's a cdclk change in progess
On Fri, 13 Jun 2014 13:37:53 +0300 ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > If someone is interested in the current cdclk frquency it should > be stable and not in process of changing frquency. Warn if the current > and requested cdclk don't match in .get_display_clock_spee() on vlv. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 29dddec..601e97e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5234,6 +5234,10 @@ static int valleyview_get_display_clock_speed(struct > drm_device *dev) > > divider = val & DISPLAY_FREQUENCY_VALUES; > > + WARN((val & DISPLAY_FREQUENCY_STATUS) != > + (divider << DISPLAY_FREQUENCY_STATUS_SHIFT), > + "cdclk change in progress\n"); > + > return DIV_ROUND_CLOSEST(vco << 1, divider + 1); > } > Hm, there's not much we can do in this case, so rather than warn maybe we should try a wait instead, and only warn if it times out? Even then there's not much we can do aside from poking the PUnit folks. -- Jesse Barnes, 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 06/11] drm/i915: Wait for cdclk change to occure when going for 400MHz
On Fri, 13 Jun 2014 13:37:52 +0300
ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> VLV Punit doesn't support the 400MHz cdclk option, so we bypass the
> Punit and poke at CCK directly. However we forgot to wait for the
> frequeency change to complete. Poll the CCK clock status to make sure
> the clock has changed before we fire up any pipes.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 3a9b017..29dddec 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4483,6 +4483,11 @@ static void valleyview_set_cdclk(struct drm_device
> *dev, int cdclk)
> val &= ~DISPLAY_FREQUENCY_VALUES;
> val |= divider;
> vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, val);
> +
> + if (wait_for((vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL)
> &
> + DISPLAY_FREQUENCY_STATUS) == (divider <<
> DISPLAY_FREQUENCY_STATUS_SHIFT),
> + 50))
> + DRM_ERROR("timed out waiting for CDclk change\n");
> mutex_unlock(&dev_priv->dpio_lock);
> }
>
Seems reasonable, assuming this actually works in testing.
Reviewed-by: Jesse Barnes
--
Jesse Barnes, 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 05/11] drm/i915: Use 200MHz cdclk on vlv when all pipes are off
On Fri, 13 Jun 2014 13:37:51 +0300 ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Drop the cdclk frequency to 200MHz on vlv when all pipes are off. In > theory we should be able to use 200MHz also when the pixel clock is at > most 90% of 200MHz. However in practice all we seem to get is a solid > color picture or an otherwise corrupted display. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 1f3985f..3a9b017 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4520,14 +4520,19 @@ static int valleyview_calc_cdclk(struct > drm_i915_private *dev_priv, >* 400MHz >* So we check to see whether we're above 90% of the lower bin and >* adjust if needed. > + * > + * We seem to get an unstable or solid color picture at 200MHz. > + * Not sure what's wrong. For now use 200MHz only when all pipes > + * are off. >*/ > if (max_pixclk > freq_320*9/10) > return 40; > else if (max_pixclk > 27*9/10) > return freq_320; > - else > + else if (max_pixclk > 0) > return 27; > - /* Looks like the 200MHz CDclk freq doesn't work on some configs */ > + else > + return 20; > } > > /* compute the max pixel clock for new configuration */ I guess this is safe, but optional (won't we be shutting off the clocks anyway?). Reviewed-by: Jesse Barnes -- Jesse Barnes, 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 04/11] drm/i915: Handle 320 vs. 333 MHz cdclk on vlv
On Fri, 13 Jun 2014 13:37:50 +0300
ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Depending on the HPLL frequency one of the supported cdclk frquencies is
> either 320MHz or 333MHz. Figure out which one it is to accurately pick
> the minimal required cdclk. This would also avoid a warning from the
> cdclk code where it compares the actual cdclk read out from the hardware
> with a value that was calculated using valleyview_calc_cdclk().
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 61d7ea2..1f3985f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4509,19 +4509,22 @@ static void valleyview_set_cdclk(struct drm_device
> *dev, int cdclk)
> static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
>int max_pixclk)
> {
> + int vco = valleyview_get_vco(dev_priv);
> + int freq_320 = (vco << 1) % 32 != 0 ? 33 : 32;
> +
> /*
>* Really only a few cases to deal with, as only 4 CDclks are supported:
>* 200MHz
>* 267MHz
> - * 320MHz
> + * 320/333MHz (depends on HPLL freq)
>* 400MHz
>* So we check to see whether we're above 90% of the lower bin and
>* adjust if needed.
>*/
> - if (max_pixclk > 32*9/10)
> + if (max_pixclk > freq_320*9/10)
> return 40;
> else if (max_pixclk > 27*9/10)
> - return 32;
> + return freq_320;
> else
> return 27;
> /* Looks like the 200MHz CDclk freq doesn't work on some configs */
Looks good. This doc searching has been fun.
Reviewed-by: Jesse Barnes
--
Jesse Barnes, 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] Regression in i915 driver in 3.16-rc2
On Wed, 25 Jun 2014, Ville [iso-8859-1] Syrj�l� wrote: > On Wed, Jun 25, 2014 at 02:06:55PM -0400, Alan Stern wrote: > > Daniel: > > > > I encountered a new problem in the i915 driver the first time I booted > > a 3.16-rc kernel on this computer. When it switched over to the frame > > buffer driver, the screen went blank and stayed that way. > > > > 3.15 works okay. > > > > Attached are log extracts from 3.15 and 3.16 (both with drm.debug=0xe > > in the boot command line). The timestamps have been stripped out for > > easy diff comparison. These are the result of > > > > dmesg | egrep 'drm|i915|frame' > > > > with a couple of irrelevant lines removed. I can send the complete > > logs if you need them. > > > > Alan Stern > > > Kernel command line: BOOT_IMAGE=/boot/test-3.x root=/dev/hda8 ro > > console=ttyS0,115200 console=tty0 earlyprintk=serial,ttyS0,115200 > > no_console_suspend drm.debug=0xe vconsole.keymap=us > > vconsole.font=latarcyrheb-sun16 LANG=en_US.UTF-8 usbcore.dyndbg=+p > > ehci_hcd.dyndbg=+p > > [drm] Initialized drm 1.1.0 20060810 > > [drm:i915_dump_device_info] i915 device info: gen=2, pciid=0x2572 > > flags=has_overlay,overlay_needs_physical, > > [drm:intel_detect_pch] No PCH found. > > [drm] Memory usable by graphics device = 128M > > [drm:i915_gem_gtt_init] GMADR size = 128M > > [drm:i915_gem_gtt_init] GTT stolen size = 8M > > [drm:i915_gem_gtt_init] ppgtt mode: 0 > > [drm:intel_opregion_setup] graphic opregion physical addr: 0x0 > > [drm:intel_opregion_setup] ACPI OpRegion not supported! > > [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > > [drm] Driver supports precise vblank timestamp query. > > [drm:init_vbt_defaults] Set default to SSC at 7 kHz > > [drm:parse_general_features] BDB_GENERAL_FEATURES int_tv_support 1 > > int_crt_support 0 lvds_use_ssc 0 lvds_ssc_freq 48000 display_clock_mode 0 > > fdi_rx_polarity_inverted 0 > > ^ > > This should get your display back: > http://patchwork.freedesktop.org/patch/28508/ It did indeed. Thank you very much. Alan Stern ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/11] drm/i915: Move vlv cdclk code to .get_display_clock_speed()
On Fri, 13 Jun 2014 13:37:49 +0300
ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> We have a standard hook for reading out the current cdclk. Move the VLV
> code from valleyview_cur_cdclk() to .get_display_clock_speed().
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 33 +
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> drivers/gpu/drm/i915/intel_pm.c | 2 +-
> 3 files changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 36562b5..61d7ea2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4449,7 +4449,7 @@ static void valleyview_set_cdclk(struct drm_device
> *dev, int cdclk)
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 val, cmd;
>
> - WARN_ON(valleyview_cur_cdclk(dev_priv) != dev_priv->vlv_cdclk_freq);
> + WARN_ON(dev_priv->display.get_display_clock_speed(dev) !=
> dev_priv->vlv_cdclk_freq);
> dev_priv->vlv_cdclk_freq = cdclk;
>
> if (cdclk >= 32) /* jump to highest voltage for 400MHz too */
> @@ -4506,24 +4506,6 @@ static void valleyview_set_cdclk(struct drm_device
> *dev, int cdclk)
> intel_i2c_reset(dev);
> }
>
> -int valleyview_cur_cdclk(struct drm_i915_private *dev_priv)
> -{
> - int cur_cdclk, vco;
> - int divider;
> -
> - vco = valleyview_get_vco(dev_priv);
> -
> - mutex_lock(&dev_priv->dpio_lock);
> - divider = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> - mutex_unlock(&dev_priv->dpio_lock);
> -
> - divider &= DISPLAY_FREQUENCY_VALUES;
> -
> - cur_cdclk = DIV_ROUND_CLOSEST(vco << 1, divider + 1);
> -
> - return cur_cdclk;
> -}
> -
> static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
>int max_pixclk)
> {
> @@ -5228,7 +5210,18 @@ static int intel_crtc_compute_config(struct intel_crtc
> *crtc,
>
> static int valleyview_get_display_clock_speed(struct drm_device *dev)
> {
> - return 40; /* FIXME */
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + int vco = valleyview_get_vco(dev_priv);
> + u32 val;
> + int divider;
> +
> + mutex_lock(&dev_priv->dpio_lock);
> + val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> + mutex_unlock(&dev_priv->dpio_lock);
> +
> + divider = val & DISPLAY_FREQUENCY_VALUES;
> +
> + return DIV_ROUND_CLOSEST(vco << 1, divider + 1);
> }
>
> static int i945_get_display_clock_speed(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 78d4124..65ce0bb 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -717,7 +717,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> const char *intel_output_name(int output);
> bool intel_has_pending_fb_unpin(struct drm_device *dev);
> int intel_pch_rawclk(struct drm_device *dev);
> -int valleyview_cur_cdclk(struct drm_i915_private *dev_priv);
> void intel_mark_busy(struct drm_device *dev);
> void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
> struct intel_engine_cs *ring);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a9ddf74..67f019c1 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5535,7 +5535,7 @@ static void valleyview_init_clock_gating(struct
> drm_device *dev)
> }
> DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
>
> - dev_priv->vlv_cdclk_freq = valleyview_cur_cdclk(dev_priv);
> + dev_priv->vlv_cdclk_freq =
> dev_priv->display.get_display_clock_speed(dev);
> DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
>dev_priv->vlv_cdclk_freq);
>
Looks like we only really use that on < gen4 but so it seems harmless.
Reviewed-by: Jesse Barnes
--
Jesse Barnes, 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 02/11] drm/i915: Give names to the CCK_DISPLAY_CLOCK_CONTROL bits
On Fri, 13 Jun 2014 13:37:48 +0300
ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Avoid using magic values for CCK frequency bits. Also the mask we were
> using for the requested frequency was one bit too short. Fix it up.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_reg.h | 5 +
> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0f4a0ed..2aa9a3c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -584,6 +584,11 @@ enum punit_power_well {
> #define DSI_PLL_M1_DIV_SHIFT0
> #define DSI_PLL_M1_DIV_MASK (0x1ff << 0)
> #define CCK_DISPLAY_CLOCK_CONTROL0x6b
> +#define DISPLAY_TRUNK_FORCE_ON (1 << 17)
> +#define DISPLAY_TRUNK_FORCE_OFF (1 << 16)
> +#define DISPLAY_FREQUENCY_STATUS(0x1f << 8)
> +#define DISPLAY_FREQUENCY_STATUS_SHIFT 8
> +#define DISPLAY_FREQUENCY_VALUES(0x1f << 0)
>
> /**
> * DOC: DPIO
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 5f66dc8..36562b5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4480,7 +4480,7 @@ static void valleyview_set_cdclk(struct drm_device
> *dev, int cdclk)
> mutex_lock(&dev_priv->dpio_lock);
> /* adjust cdclk divider */
> val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> - val &= ~0xf;
> + val &= ~DISPLAY_FREQUENCY_VALUES;
> val |= divider;
> vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, val);
> mutex_unlock(&dev_priv->dpio_lock);
> @@ -4517,7 +4517,7 @@ int valleyview_cur_cdclk(struct drm_i915_private
> *dev_priv)
> divider = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> mutex_unlock(&dev_priv->dpio_lock);
>
> - divider &= 0xf;
> + divider &= DISPLAY_FREQUENCY_VALUES;
>
> cur_cdclk = DIV_ROUND_CLOSEST(vco << 1, divider + 1);
>
You snuck in a fix for the mask here, but it looks correct.
Reviewed-by: Jesse Barnes http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/11] drm/i915: Change vlv cdclk to use kHz units
On Fri, 13 Jun 2014 13:37:47 +0300
ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Use kHz units in vlv cdclk code since that's more customary.
>
> Also replace the precomputed 90% values with *9/10 computation
> for extra clarity.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 27 ++-
> drivers/gpu/drm/i915/intel_i2c.c | 2 +-
> drivers/gpu/drm/i915/intel_pm.c | 2 +-
> 3 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 5762726..5f66dc8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4429,6 +4429,7 @@ static void modeset_update_crtc_power_domains(struct
> drm_device *dev)
> intel_display_set_init_power(dev_priv, false);
> }
>
> +/* returns HPLL frequency in kHz */
> int valleyview_get_vco(struct drm_i915_private *dev_priv)
> {
> int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
> @@ -4439,7 +4440,7 @@ int valleyview_get_vco(struct drm_i915_private
> *dev_priv)
> CCK_FUSE_HPLL_FREQ_MASK;
> mutex_unlock(&dev_priv->dpio_lock);
>
> - return vco_freq[hpll_freq];
> + return vco_freq[hpll_freq] * 1000;
> }
>
> /* Adjust CDclk dividers to allow high res or save power if possible */
> @@ -4451,9 +4452,9 @@ static void valleyview_set_cdclk(struct drm_device
> *dev, int cdclk)
> WARN_ON(valleyview_cur_cdclk(dev_priv) != dev_priv->vlv_cdclk_freq);
> dev_priv->vlv_cdclk_freq = cdclk;
>
> - if (cdclk >= 320) /* jump to highest voltage for 400MHz too */
> + if (cdclk >= 32) /* jump to highest voltage for 400MHz too */
> cmd = 2;
> - else if (cdclk == 266)
> + else if (cdclk == 27)
> cmd = 1;
> else
> cmd = 0;
> @@ -4470,11 +4471,11 @@ static void valleyview_set_cdclk(struct drm_device
> *dev, int cdclk)
> }
> mutex_unlock(&dev_priv->rps.hw_lock);
>
> - if (cdclk == 400) {
> + if (cdclk == 40) {
> u32 divider, vco;
>
> vco = valleyview_get_vco(dev_priv);
> - divider = ((vco << 1) / cdclk) - 1;
> + divider = DIV_ROUND_CLOSEST(vco << 1, cdclk) - 1;
>
> mutex_lock(&dev_priv->dpio_lock);
> /* adjust cdclk divider */
> @@ -4494,7 +4495,7 @@ static void valleyview_set_cdclk(struct drm_device
> *dev, int cdclk)
>* For high bandwidth configs, we set a higher latency in the bunit
>* so that the core display fetch happens in time to avoid underruns.
>*/
> - if (cdclk == 400)
> + if (cdclk == 40)
> val |= 4500 / 250; /* 4.5 usec */
> else
> val |= 3000 / 250; /* 3.0 usec */
> @@ -4518,7 +4519,7 @@ int valleyview_cur_cdclk(struct drm_i915_private
> *dev_priv)
>
> divider &= 0xf;
>
> - cur_cdclk = (vco << 1) / (divider + 1);
> + cur_cdclk = DIV_ROUND_CLOSEST(vco << 1, divider + 1);
>
> return cur_cdclk;
> }
> @@ -4535,12 +4536,12 @@ static int valleyview_calc_cdclk(struct
> drm_i915_private *dev_priv,
>* So we check to see whether we're above 90% of the lower bin and
>* adjust if needed.
>*/
> - if (max_pixclk > 288000) {
> - return 400;
> - } else if (max_pixclk > 24) {
> - return 320;
> - } else
> - return 266;
> + if (max_pixclk > 32*9/10)
> + return 40;
> + else if (max_pixclk > 27*9/10)
> + return 32;
> + else
> + return 27;
> /* Looks like the 200MHz CDclk freq doesn't work on some configs */
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c
> b/drivers/gpu/drm/i915/intel_i2c.c
> index d33b61d..9ce4f09 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -86,7 +86,7 @@ static void gmbus_set_freq(struct drm_i915_private
> *dev_priv)
>
> BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
>
> - vco = valleyview_get_vco(dev_priv);
> + vco = valleyview_get_vco(dev_priv) / 1000;
>
> /* Get the CDCLK divide ratio */
> cdclk_div = get_disp_clk_div(dev_priv, CDCLK);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5024bc8..a9ddf74 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5536,7 +5536,7 @@ static void valleyview_init_clock_gating(struct
> drm_device *dev)
> DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
>
> dev_priv->vlv_cdclk_freq = valleyview_cur_cdclk(dev_priv);
> - DRM_DEBUG_DRIVER("Current CD clock rate: %d MHz",
> + DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
>dev_priv->vlv_cdclk_freq);
>
> I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
Reviewed-by: J
Re: [Intel-gfx] [PATCH] drm/i915: only apply crt_present check on VLV
On Wed, Jun 25, 2014 at 08:24:29AM -0700, Jesse Barnes wrote:
> Apparently we can't trust this field on other platforms and need to find
> some other way.
>
> Signed-off-by: Jesse Barnes
Reviewed-by: Ville Syrjälä
Since it's a regression it might be a good idea to note the
offender:
commit 27da3bdfcf7f5233cdfe4563f53edf1ecab7cea0
Author: Jesse Barnes
Date: Fri Apr 4 16:12:07 2014 -0700
drm/i915: use VBT to determine whether to enumerate the VGA port
> ---
> drivers/gpu/drm/i915/intel_display.c | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 065984d..9f80de5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11807,6 +11807,22 @@ const char *intel_output_name(int output)
> return names[output];
> }
>
> +static bool intel_crt_present(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (IS_ULT(dev))
> + return false;
> +
> + if (IS_CHERRYVIEW(dev))
> + return false;
> +
> + if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
> + return false;
> +
> + return true;
> +}
> +
> static void intel_setup_outputs(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -11815,7 +11831,7 @@ static void intel_setup_outputs(struct drm_device
> *dev)
>
> intel_lvds_init(dev);
>
> - if (!IS_ULT(dev) && !IS_CHERRYVIEW(dev) &&
> dev_priv->vbt.int_crt_support)
> + if (intel_crt_present(dev))
> intel_crt_init(dev);
>
> if (HAS_DDI(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] Regression in i915 driver in 3.16-rc2
On Wed, Jun 25, 2014 at 02:06:55PM -0400, Alan Stern wrote: > Daniel: > > I encountered a new problem in the i915 driver the first time I booted > a 3.16-rc kernel on this computer. When it switched over to the frame > buffer driver, the screen went blank and stayed that way. > > 3.15 works okay. > > Attached are log extracts from 3.15 and 3.16 (both with drm.debug=0xe > in the boot command line). The timestamps have been stripped out for > easy diff comparison. These are the result of > > dmesg | egrep 'drm|i915|frame' > > with a couple of irrelevant lines removed. I can send the complete > logs if you need them. > > Alan Stern > Kernel command line: BOOT_IMAGE=/boot/test-3.x root=/dev/hda8 ro > console=ttyS0,115200 console=tty0 earlyprintk=serial,ttyS0,115200 > no_console_suspend drm.debug=0xe vconsole.keymap=us > vconsole.font=latarcyrheb-sun16 LANG=en_US.UTF-8 usbcore.dyndbg=+p > ehci_hcd.dyndbg=+p > [drm] Initialized drm 1.1.0 20060810 > [drm:i915_dump_device_info] i915 device info: gen=2, pciid=0x2572 > flags=has_overlay,overlay_needs_physical, > [drm:intel_detect_pch] No PCH found. > [drm] Memory usable by graphics device = 128M > [drm:i915_gem_gtt_init] GMADR size = 128M > [drm:i915_gem_gtt_init] GTT stolen size = 8M > [drm:i915_gem_gtt_init] ppgtt mode: 0 > [drm:intel_opregion_setup] graphic opregion physical addr: 0x0 > [drm:intel_opregion_setup] ACPI OpRegion not supported! > [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > [drm] Driver supports precise vblank timestamp query. > [drm:init_vbt_defaults] Set default to SSC at 7 kHz > [drm:parse_general_features] BDB_GENERAL_FEATURES int_tv_support 1 > int_crt_support 0 lvds_use_ssc 0 lvds_ssc_freq 48000 display_clock_mode 0 > fdi_rx_polarity_inverted 0 ^ This should get your display back: http://patchwork.freedesktop.org/patch/28508/ -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Regression in i915 driver in 3.16-rc2
Daniel: I encountered a new problem in the i915 driver the first time I booted a 3.16-rc kernel on this computer. When it switched over to the frame buffer driver, the screen went blank and stayed that way. 3.15 works okay. Attached are log extracts from 3.15 and 3.16 (both with drm.debug=0xe in the boot command line). The timestamps have been stripped out for easy diff comparison. These are the result of dmesg | egrep 'drm|i915|frame' with a couple of irrelevant lines removed. I can send the complete logs if you need them. Alan Stern Kernel command line: BOOT_IMAGE=/boot/test-3.x root=/dev/hda8 ro console=ttyS0,115200 console=tty0 earlyprintk=serial,ttyS0,115200 no_console_suspend drm.debug=0xe vconsole.keymap=us vconsole.font=latarcyrheb-sun16 LANG=en_US.UTF-8 usbcore.dyndbg=+p ehci_hcd.dyndbg=+p [drm] Initialized drm 1.1.0 20060810 [drm:i915_dump_device_info] i915 device info: gen=2, pciid=0x2572 flags=has_overlay,overlay_needs_physical, [drm:intel_detect_pch] No PCH found. [drm] Memory usable by graphics device = 128M [drm:i915_gem_gtt_init] GMADR size = 128M [drm:i915_gem_gtt_init] GTT stolen size = 8M [drm:i915_gem_gtt_init] ppgtt mode: 0 [drm:intel_opregion_setup] graphic opregion physical addr: 0x0 [drm:intel_opregion_setup] ACPI OpRegion not supported! [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [drm] Driver supports precise vblank timestamp query. [drm:init_vbt_defaults] Set default to SSC at 7 kHz [drm:parse_general_features] BDB_GENERAL_FEATURES int_tv_support 1 int_crt_support 0 lvds_use_ssc 0 lvds_ssc_freq 48000 display_clock_mode 0 fdi_rx_polarity_inverted 0 [drm:parse_general_definitions] crt_ddc_bus_pin: 2 [drm:parse_sdvo_device_mapping] different child size is found. Invalid. [drm:parse_device_mapping] different child size is found. Invalid. [drm:parse_mipi] No MIPI BDB found [drm:intel_display_power_get] enabling always-on [drm:intel_modeset_init] 1 display pipe available. [drm:intel_modeset_init] pipe A sprite A init failed: -19 [drm:intel_gmbus_force_bit] enabling bit-banging on i915 gmbus dpb. force bit now 1 [drm:intel_gmbus_force_bit] disabling bit-banging on i915 gmbus dpb. force bit now 0 [drm:intel_gmbus_force_bit] enabling bit-banging on i915 gmbus dpb. force bit now 1 [drm:intel_gmbus_force_bit] disabling bit-banging on i915 gmbus dpb. force bit now 0 [drm:intel_gmbus_force_bit] enabling bit-banging on i915 gmbus dpb. force bit now 1 [drm:intel_gmbus_force_bit] disabling bit-banging on i915 gmbus dpb. force bit now 0 [drm:intel_gmbus_force_bit] enabling bit-banging on i915 gmbus ssc. force bit now 1 [drm:intel_gmbus_force_bit] disabling bit-banging on i915 gmbus ssc. force bit now 0 [drm:intel_gmbus_force_bit] enabling bit-banging on i915 gmbus dpb. force bit now 1 [drm:tfp410_init] tfp410 not detected got VID : from i915 gmbus dpb Slave 56. [drm:intel_gmbus_force_bit] disabling bit-banging on i915 gmbus dpb. force bit now 0 [drm:intel_gmbus_force_bit] enabling bit-banging on i915 gmbus dpb. force bit now 1 [drm:intel_gmbus_force_bit] disabling bit-banging on i915 gmbus dpb. force bit now 0 [drm:intel_gmbus_force_bit] enabling bit-banging on i915 gmbus dpb. force bit now 1 [drm:intel_gmbus_force_bit] disabling bit-banging on i915 gmbus dpb. force bit now 0 [drm:intel_modeset_readout_hw_state] [CRTC:5] hw state readout: enabled [drm:intel_modeset_readout_hw_state] [ENCODER:7:DAC-7] hw state readout: enabled, pipe A [drm:intel_modeset_readout_hw_state] [CONNECTOR:6:VGA-1] hw state readout: enabled [drm:intel_dump_pipe_config] [CRTC:5][setup_hw_state] config for pipe A [drm:intel_dump_pipe_config] cpu_transcoder: A [drm:intel_dump_pipe_config] pipe bpp: 0, dithering: 0 [drm:intel_dump_pipe_config] fdi/pch: 0, lanes: 0, gmch_m: 0, gmch_n: 0, link_m: 0, link_n: 0, tu: 0 [drm:intel_dump_pipe_config] dp: 0, gmch_m: 0, gmch_n: 0, link_m: 0, link_n: 0, tu: 0 [drm:intel_dump_pipe_config] requested mode: [drm:drm_mode_debug_printmodeline] Modeline 0:"" 0 0 640 0 0 0 480 0 0 0 0x0 0x0 [drm:intel_dump_pipe_config] adjusted mode: [drm:drm_mode_debug_printmodeline] Modeline 0:"" 0 0 0 0 0 0 0 0 0 0 0x0 0xa [drm:intel_dump_crtc_timings] crtc timings: 25154 640 656 752 800 480 490 492 525, type: 0x0 flags: 0xa [drm:intel_dump_pipe_config] port clock: 25154 [drm:intel_dump_pipe_config] pipe src size: 640x480 [drm:intel_dump_pipe_config] gmch pfit: control: 0x, ratios: 0x, lvds border: 0x [drm:intel_dump_pipe_config] pch pfit: pos: 0x, size: 0x, disabled [drm:intel_dump_pipe_config] ips: 0 [drm:intel_dump_pipe_config] double wide: 0 [drm:intel_connector_check_state] [CONNECTOR:6:VGA-1] [drm:check_encoder_state] [ENCODER:7:DAC-7] [drm:check_crtc_state] [CRTC:5] [drm:i9xx_get_plane_config] pipe/plane 0/0 with fb: size=640x480@8, offset=0, pitch 640, size 0x4b000 [drm:i915_gem_setup_global_gtt] clearing unused GTT space: [0, 7fff000] [drm:i915_gem_context_init] fake context support
[Intel-gfx] [PATCH] drm/i915: only apply crt_present check on VLV
Apparently we can't trust this field on other platforms and need to find
some other way.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/intel_display.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 065984d..9f80de5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11807,6 +11807,22 @@ const char *intel_output_name(int output)
return names[output];
}
+static bool intel_crt_present(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (IS_ULT(dev))
+ return false;
+
+ if (IS_CHERRYVIEW(dev))
+ return false;
+
+ if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support)
+ return false;
+
+ return true;
+}
+
static void intel_setup_outputs(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -11815,7 +11831,7 @@ static void intel_setup_outputs(struct drm_device *dev)
intel_lvds_init(dev);
- if (!IS_ULT(dev) && !IS_CHERRYVIEW(dev) &&
dev_priv->vbt.int_crt_support)
+ if (intel_crt_present(dev))
intel_crt_init(dev);
if (HAS_DDI(dev)) {
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: correct BLC vs PWM enable/disable ordering
On Wed, 25 Jun 2014 15:21:12 +0300
Jani Nikula wrote:
> On Mon, 31 Mar 2014, Jesse Barnes wrote:
> > With the new checks in place, we can see we're doing things backwards,
> > so fix them up per the spec.
> >
> > Signed-off-by: Jesse Barnes
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 13 +++--
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index b6f7087..d540fbe 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1273,8 +1273,6 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
> >
> > DRM_DEBUG_KMS("Turn eDP power off\n");
> >
> > - edp_wait_backlight_off(intel_dp);
> > -
>
> Please justify moving this wait to intel_edp_backlight_off. I thought
> the point of these wait calls is such that we'll only end up waiting
> when we really have to. If this is left as-is, we can do useful stuff
> *while* waiting (case in point, intel_dp_sink_dpms in intel_disable_dp).
The wait needs to happen between the BLC disable and the backlight
disable per the eDP timing spec. We could put the disable into a
delayed work queue if you want to reclaim the time, but it should be
pretty small compared to a full panel power sequence.
The wait here looks like it was to prevent us from re-enabling the
backlight too quickly, but I don't have timing info for that; not sure
if there are specific requirements there or not.
Jesse
> Otherwise this looks good to me, although I didn't find proper
> explanations for everything. Do I understand correctly that the
> EDP_BLC_ENABLE bit in PP_CONTROL is basically the final switch for
> enabling/disabling the PWM signal to the eDP? So before this patch, we
> let the disabled PWM signal through to the panel?
Right, something like that. Enabling PWM starts driving power to some
components, but the PP_CONTROL bit controls whether it actually gets
out to the panel meaningfully, and at least according to the scope
readouts we have, doing it in this order corrects the backward ordering
we saw.
--
Jesse Barnes, 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] [RFC] drm/i915: Add variable gem object size support to i915
On Wed, Jun 25, 2014 at 02:26:52PM +0100, Tvrtko Ursulin wrote: > >That's a good question to ask a GL team. In the light of sparse > >textures I think the region idea would be better. > > > >We would need to define what the coordinates mean, for instance: > > - 2D view of the buffer, and the kernel takes care of translating what > > it means for the underlying pages? > > - See the buffer object as an array of pages, and those numbers define > > a region of pages. > > This would mean kernel has to know about all possible tiling > formats? Would that be asking a bit too much (of the kernel)? Not if we see the buffer as an (2D) array of pages. > How (im)possible would it be to allocate backing store on demand, on > page by page basis, on write rather than on binding into address > space? I think Chris would be very upset to lose the performance benefit of pre-faulting the correct pages? -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i915: Add variable gem object size support to i915
On 06/25/2014 01:57 PM, Damien Lespiau wrote: On Wed, Jun 25, 2014 at 12:46:57PM +0100, Siluvery, Arun wrote: On 25/06/2014 12:14, Damien Lespiau wrote: On Wed, Jun 25, 2014 at 11:51:33AM +0100, Damien Lespiau wrote: (This is not necessarily things one would need to take into account for this work, just a few thoughts). One thing I'm wondering is how fitting the "size" parameter really is when talking about inherently 2D buffers. For instance, let's take a Y-tiled texture with MIPLAYOUT_RIGHT, if we want to allocate mip map levels 0 and 1, and use the ioctl "naively" to reserve the LOD1 region in one go, we'll end up over allocating the space below LOD1 (if I'm not mistaken that is). This can be mitigated by several calls to this fallocate ioctl, to reserve columns of pages (in the case above, columns for the LOD1 region). So, how about trying to reduce this ioctl overhead by providing a list of (start, length) in the ioctl structure? One more thing to factor in is (let's assume one future hardware will support that): https://www.opengl.org/registry/specs/ARB/sparse_texture.txt So maybe what we really want is to be able to specify region of pages that could be specified in (x, y, width, height, stride) ? (idea popped when talking to Neil Roberts (I now have someone working on Mesa in the office). Hi Damien, Thank you for your comments and the idea to improve this ioctl. At the moment start, end of a region are expected to be page-aligned; ioctl can be modified to accept a multiple ranges and modify them in one go to reduce the overhead of the ioctl. We can define how we want to specify multiple ranges, if userspace can provide the list as (start, end) pairs kernel can directly use them but what would be the preferred way from the user point of view? That's a good question to ask a GL team. In the light of sparse textures I think the region idea would be better. We would need to define what the coordinates mean, for instance: - 2D view of the buffer, and the kernel takes care of translating what it means for the underlying pages? - See the buffer object as an array of pages, and those numbers define a region of pages. This would mean kernel has to know about all possible tiling formats? Would that be asking a bit too much (of the kernel)? How (im)possible would it be to allocate backing store on demand, on page by page basis, on write rather than on binding into address space? Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm/i915 KMS regression in Linux 3.16-rc2 (with git bisect result)
On Wed, Jun 25, 2014 at 03:03:44PM +0200, Tom Van Braeckel wrote: > Hi, > > There seems to be a regression in the upcoming Linux 3.16-rc2 release > candidate that I bisected down to this first bad commit: > [dbb42748ac4929987c1449ecb296b39ef8956b62] drm/i915: Move the C3 LP > write bit setup to gen3_init_clock_gating() for KMS. Can you attach the dmesg with rc2 and dbb4274 reverted? -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] [RFC] drm/i915: Add variable gem object size support to i915
On Wed, Jun 25, 2014 at 12:46:57PM +0100, Siluvery, Arun wrote: > On 25/06/2014 12:14, Damien Lespiau wrote: > >On Wed, Jun 25, 2014 at 11:51:33AM +0100, Damien Lespiau wrote: > >>(This is not necessarily things one would need to take into account for > >>this work, just a few thoughts). > >> > >>One thing I'm wondering is how fitting the "size" parameter really is > >>when talking about inherently 2D buffers. > >> > >>For instance, let's take a Y-tiled texture with MIPLAYOUT_RIGHT, if we > >>want to allocate mip map levels 0 and 1, and use the ioctl "naively" to > >>reserve the LOD1 region in one go, we'll end up over allocating the > >>space below LOD1 (if I'm not mistaken that is). > >> > >>This can be mitigated by several calls to this fallocate ioctl, to > >>reserve columns of pages (in the case above, columns for the LOD1 > >>region). > >> > >>So, how about trying to reduce this ioctl overhead by providing a list > >>of (start, length) in the ioctl structure? > > > >One more thing to factor in is (let's assume one future hardware will > >support that): > >https://www.opengl.org/registry/specs/ARB/sparse_texture.txt > > > >So maybe what we really want is to be able to specify region of pages > >that could be specified in (x, y, width, height, stride) ? (idea popped > >when talking to Neil Roberts (I now have someone working on Mesa in the > >office). > > > > Hi Damien, > > Thank you for your comments and the idea to improve this ioctl. > At the moment start, end of a region are expected to be > page-aligned; ioctl can be modified to accept a multiple ranges and > modify them in one go to reduce the overhead of the ioctl. > > We can define how we want to specify multiple ranges, if userspace > can provide the list as (start, end) pairs kernel can directly use > them but what would be the preferred way from the user point of > view? That's a good question to ask a GL team. In the light of sparse textures I think the region idea would be better. We would need to define what the coordinates mean, for instance: - 2D view of the buffer, and the kernel takes care of translating what it means for the underlying pages? - See the buffer object as an array of pages, and those numbers define a region of pages. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: add PWM and BLC assertion checks
On Tue, 01 Apr 2014, Jesse Barnes wrote:
> On Tue, 01 Apr 2014 10:19:29 +0300
> Jani Nikula wrote:
>
>> On Mon, 31 Mar 2014, Jesse Barnes wrote:
>> > To make sure we properly follow the enable/disable sequences.
>> >
>> > Signed-off-by: Jesse Barnes
>> > ---
>> > drivers/gpu/drm/i915/intel_dp.c| 62
>> > --
>> > drivers/gpu/drm/i915/intel_drv.h | 1 +
>> > drivers/gpu/drm/i915/intel_panel.c | 5 ++-
>> > 3 files changed, 65 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> > b/drivers/gpu/drm/i915/intel_dp.c
>> > index bf73771..b6f7087 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -301,6 +301,20 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>> >return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
>> > }
>> >
>> > +static void assert_pwm(struct intel_connector *connector,
>> > + bool expected_state)
>> > +{
>> > + bool state;
>> > +
>> > + state = intel_panel_get_backlight(connector);
>>
>> If the duty cycle is regarded as a binary on/off, I'd rather add an
>> additional "is enabled" call to intel_panel.c. Especially so because the
>> duty cycle value returned by intel_panel_get_backlight is meaningless
>> without the max value.
>
> Hm I guess that would be cleaner; for my purposes I thought any
> non-zero PWM duty cycle would be sufficient, but of course other checks
> are needed as well, like whether the PWM enable bit is on, and checks
> against the BLC_EN bit in the PP regs, but those are logically
> separate. is_enabled might better map back to the PWM_EN bit rather
> than a non-zero duty cycle though.
We could add intel_panel_backlight_enabled() call that returns
connector->panel.backlight.enabled.
BR,
Jani.
>
>> >
>> > + if (I915_READ(VLV_BLC_PWM_CTL2(pipe) & BLM_PWM_ENABLE))
>> > + return 0;
>> > +
>>
>> If our internal state is consistent, I don't think this should be
>> necessary. And if our internal state isn't consistent, we should fix
>> that and maybe add internal asserts within intel_panel.c.
>
> Yeah this could be covered with other asserts as long as we have them
> in all the right places.
>
> --
> Jesse Barnes, Intel Open Source Technology Center
--
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 2/3] drm/i915: correct BLC vs PWM enable/disable ordering
On Mon, 31 Mar 2014, Jesse Barnes wrote:
> With the new checks in place, we can see we're doing things backwards,
> so fix them up per the spec.
>
> Signed-off-by: Jesse Barnes
> ---
> drivers/gpu/drm/i915/intel_dp.c | 13 +++--
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b6f7087..d540fbe 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1273,8 +1273,6 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
>
> DRM_DEBUG_KMS("Turn eDP power off\n");
>
> - edp_wait_backlight_off(intel_dp);
> -
Please justify moving this wait to intel_edp_backlight_off. I thought
the point of these wait calls is such that we'll only end up waiting
when we really have to. If this is left as-is, we can do useful stuff
*while* waiting (case in point, intel_dp_sink_dpms in intel_disable_dp).
Otherwise this looks good to me, although I didn't find proper
explanations for everything. Do I understand correctly that the
EDP_BLC_ENABLE bit in PP_CONTROL is basically the final switch for
enabling/disabling the PWM signal to the eDP? So before this patch, we
let the disabled PWM signal through to the panel?
BR,
Jani.
> WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n");
>
> /* By this time the PWM and BLC bits should be off already */
> @@ -1316,6 +1314,9 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp)
> return;
>
> DRM_DEBUG_KMS("\n");
> +
> + intel_panel_enable_backlight(intel_dp->attached_connector);
> +
> /*
>* If we enable the backlight right away following a panel power
>* on, we may see slight flicker as the panel syncs with the eDP
> @@ -1333,8 +1334,6 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp)
>
> I915_WRITE(pp_ctrl_reg, pp);
> POSTING_READ(pp_ctrl_reg);
> -
> - intel_panel_enable_backlight(intel_dp->attached_connector);
> }
>
> void intel_edp_backlight_off(struct intel_dp *intel_dp)
> @@ -1350,8 +1349,6 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp)
> /* PWM must still be enabled here */
> assert_pwm_enabled(intel_dp->attached_connector);
>
> - intel_panel_disable_backlight(intel_dp->attached_connector);
> -
> DRM_DEBUG_KMS("\n");
> pp = ironlake_get_pp_control(intel_dp);
> pp &= ~EDP_BLC_ENABLE;
> @@ -1361,6 +1358,10 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp)
> I915_WRITE(pp_ctrl_reg, pp);
> POSTING_READ(pp_ctrl_reg);
> intel_dp->last_backlight_off = jiffies;
> +
> + edp_wait_backlight_off(intel_dp);
> +
> + intel_panel_disable_backlight(intel_dp->attached_connector);
> }
>
> static void ironlake_edp_pll_on(struct intel_dp *intel_dp)
> --
> 1.8.4.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] [RFC] drm/i915: Add variable gem object size support to i915
On 25/06/2014 12:14, Damien Lespiau wrote: On Wed, Jun 25, 2014 at 11:51:33AM +0100, Damien Lespiau wrote: (This is not necessarily things one would need to take into account for this work, just a few thoughts). One thing I'm wondering is how fitting the "size" parameter really is when talking about inherently 2D buffers. For instance, let's take a Y-tiled texture with MIPLAYOUT_RIGHT, if we want to allocate mip map levels 0 and 1, and use the ioctl "naively" to reserve the LOD1 region in one go, we'll end up over allocating the space below LOD1 (if I'm not mistaken that is). This can be mitigated by several calls to this fallocate ioctl, to reserve columns of pages (in the case above, columns for the LOD1 region). So, how about trying to reduce this ioctl overhead by providing a list of (start, length) in the ioctl structure? One more thing to factor in is (let's assume one future hardware will support that): https://www.opengl.org/registry/specs/ARB/sparse_texture.txt So maybe what we really want is to be able to specify region of pages that could be specified in (x, y, width, height, stride) ? (idea popped when talking to Neil Roberts (I now have someone working on Mesa in the office). Hi Damien, Thank you for your comments and the idea to improve this ioctl. At the moment start, end of a region are expected to be page-aligned; ioctl can be modified to accept a multiple ranges and modify them in one go to reduce the overhead of the ioctl. We can define how we want to specify multiple ranges, if userspace can provide the list as (start, end) pairs kernel can directly use them but what would be the preferred way from the user point of view? regards Arun ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i915: Add variable gem object size support to i915
On Wed, Jun 25, 2014 at 11:51:33AM +0100, Damien Lespiau wrote: > (This is not necessarily things one would need to take into account for > this work, just a few thoughts). > > One thing I'm wondering is how fitting the "size" parameter really is > when talking about inherently 2D buffers. > > For instance, let's take a Y-tiled texture with MIPLAYOUT_RIGHT, if we > want to allocate mip map levels 0 and 1, and use the ioctl "naively" to > reserve the LOD1 region in one go, we'll end up over allocating the > space below LOD1 (if I'm not mistaken that is). > > This can be mitigated by several calls to this fallocate ioctl, to > reserve columns of pages (in the case above, columns for the LOD1 > region). > > So, how about trying to reduce this ioctl overhead by providing a list > of (start, length) in the ioctl structure? One more thing to factor in is (let's assume one future hardware will support that): https://www.opengl.org/registry/specs/ARB/sparse_texture.txt So maybe what we really want is to be able to specify region of pages that could be specified in (x, y, width, height, stride) ? (idea popped when talking to Neil Roberts (I now have someone working on Mesa in the office). -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/opregion: ignore firmware requests for backlight change
On Tue, 24 Jun 2014, Aaron Lu wrote:
> Some Thinkpad laptops' firmware will initiate a backlight level change
> request through operation region on the events of AC plug/unplug, but
> since we are not using firmware's interface to do the backlight setting
> on these affected laptops, we do not want the firmware to use some
> arbitrary value from its ASL variable to set the backlight level on
> AC plug/unplug either.
I'm curious whether this happens with EFI boot, or only with legacy.
One comment inline, otherwise
Acked-by: Jani Nikula
for merging through the ACPI tree, as the change is more likely to
conflict there.
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=76491
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=77091
> Reported-and-tested-by: Igor Gnatenko
> Reported-and-tested-by: Anton Gubarkov
> Signed-off-by: Aaron Lu
> ---
> drivers/acpi/video.c | 3 ++-
> drivers/gpu/drm/i915/intel_opregion.c | 7 +++
> include/acpi/video.h | 2 ++
> 3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index fb9ffe9adc64..cf99d6d2d491 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -241,13 +241,14 @@ static bool acpi_video_use_native_backlight(void)
> return use_native_backlight_dmi;
> }
>
> -static bool acpi_video_verify_backlight_support(void)
> +bool acpi_video_verify_backlight_support(void)
> {
> if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
> backlight_device_registered(BACKLIGHT_RAW))
> return false;
> return acpi_video_backlight_support();
> }
> +EXPORT_SYMBOL(acpi_video_verify_backlight_support);
>
> /* backlight device sysfs support */
> static int acpi_video_get_brightness(struct backlight_device *bd)
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
> b/drivers/gpu/drm/i915/intel_opregion.c
> index 2e2c71fcc9ed..02943d93e88e 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -403,6 +403,13 @@ static u32 asle_set_backlight(struct drm_device *dev,
> u32 bclp)
>
> DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
>
> + /*
> + * If the acpi_video interface is not supposed to be used, don't
> + * bother processing backlight level change requests from firmware.
> + */
> + if (!acpi_video_verify_backlight_support())
> + return 0;
I'd appreciate a DRM_DEBUG_KMS here about what happened. We're bound to
wonder about that staring at some dmesg later on!
> +
> if (!(bclp & ASLE_BCLP_VALID))
> return ASLC_BACKLIGHT_FAILED;
>
> diff --git a/include/acpi/video.h b/include/acpi/video.h
> index ea4c7bbded4d..92f8c4bffefb 100644
> --- a/include/acpi/video.h
> +++ b/include/acpi/video.h
> @@ -22,6 +22,7 @@ extern void acpi_video_unregister(void);
> extern void acpi_video_unregister_backlight(void);
> extern int acpi_video_get_edid(struct acpi_device *device, int type,
> int device_id, void **edid);
> +extern bool acpi_video_verify_backlight_support(void);
> #else
> static inline int acpi_video_register(void) { return 0; }
> static inline void acpi_video_unregister(void) { return; }
> @@ -31,6 +32,7 @@ static inline int acpi_video_get_edid(struct acpi_device
> *device, int type,
> {
> return -ENODEV;
> }
> +static bool acpi_video_verify_backlight_support() { return false; }
> #endif
>
> #endif
> --
> 1.9.3
>
--
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] [RFC] drm/i915: Add variable gem object size support to i915
On Mon, Apr 28, 2014 at 04:01:29PM +0100, arun.siluv...@linux.intel.com wrote: > From: "Siluvery, Arun" > > This patch adds support to have gem objects of variable size. > The size of the gem object obj->size is always constant and this fact > is tightly coupled in the driver; this implementation allows to vary > its effective size using an interface similar to fallocate(). > > A new ioctl() is introduced to mark a range as scratch/usable. > Once marked as scratch, associated backing store is released and the > region is filled with scratch pages. The region can also be unmarked > at a later point in which case new backing pages are created. > The range can be anywhere within the object space, it can have multiple > ranges possibly overlapping forming a large contiguous range. > > There is only one single scratch page and Kernel allows to write to this > page; userspace need to keep track of scratch page range otherwise any > subsequent writes to these pages will overwrite previous content. > > This feature is useful where the exact size of the object is not clear > at the time of its creation, in such case we usually create an object > with more than the required size but end up using it partially. > In devices where there are tight memory constraints it would be useful > to release that additional space which is currently unused. Using this > interface the region can be simply marked as scratch which releases > its backing store thus reducing the memory pressure on the kernel. > > Many thanks to Daniel, ChrisW, Tvrtko, Bob for the idea and feedback > on this implementation. > > v2: fix holes in error handling and use consistent data types (Tvrtko) > - If page allocation fails simply return error; do not try to invoke >shrinker to free backing store. > - Release new pages created by us in case of error during page allocation >or sg_table update. > - Use 64-bit data types for start and length values to avoid truncation. > (This is not necessarily things one would need to take into account for this work, just a few thoughts). One thing I'm wondering is how fitting the "size" parameter really is when talking about inherently 2D buffers. For instance, let's take a Y-tiled texture with MIPLAYOUT_RIGHT, if we want to allocate mip map levels 0 and 1, and use the ioctl "naively" to reserve the LOD1 region in one go, we'll end up over allocating the space below LOD1 (if I'm not mistaken that is). This can be mitigated by several calls to this fallocate ioctl, to reserve columns of pages (in the case above, columns for the LOD1 region). So, how about trying to reduce this ioctl overhead by providing a list of (start, length) in the ioctl structure? -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't try to look up object for non-existent fb
On Wed, 25 Jun 2014, Chris Wilson wrote: > On Tue, Jun 24, 2014 at 05:05:02PM -0700, Matt Roper wrote: >> crtc->primary->fb may be NULL upon entry to intel_pipe_set_base() if the >> primary plane has previously been disabled via the universal plane >> interface. We need to check for NULL before trying to reference >> old_fb's obj. >> >> This fixes a regression introduced in >> >> commit a071fa00647bc9a3c53f917b236fff9aea175e3a >> Author: Daniel Vetter >> Date: Wed Jun 18 23:28:09 2014 +0200 >> >> drm/i915: Introduce accurate frontbuffer tracking >> >> Testcase: igt/kms_universal_plane >> Cc: Daniel Vetter >> Signed-off-by: Matt Roper > > Oh for a safe version of to_fb_obj(). > Reviewed-by: Chris Wilson Pushed to dinq, thanks for the patch and review. BR, Jani. > -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 -- 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] drm/i915: Don't try to look up object for non-existent fb
On Tue, Jun 24, 2014 at 05:05:02PM -0700, Matt Roper wrote: > crtc->primary->fb may be NULL upon entry to intel_pipe_set_base() if the > primary plane has previously been disabled via the universal plane > interface. We need to check for NULL before trying to reference > old_fb's obj. > > This fixes a regression introduced in > > commit a071fa00647bc9a3c53f917b236fff9aea175e3a > Author: Daniel Vetter > Date: Wed Jun 18 23:28:09 2014 +0200 > > drm/i915: Introduce accurate frontbuffer tracking > > Testcase: igt/kms_universal_plane > Cc: Daniel Vetter > Signed-off-by: Matt Roper Oh for a safe version of to_fb_obj(). 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 3/6] drm/i915: Implement basic Displayport automated testing function for EDID operations
On Wed, 25 Jun 2014, Todd Previte wrote:
> Implements some of the basic EDID tests for Displayport compliance. These
> tests
> include reading the EDID, verifying the checksum and writing the test
> responses
> back to the sink device.
>
> Signed-off-by: Todd Previte
> ---
> drivers/gpu/drm/i915/intel_dp.c | 36 +++-
> 1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 43fcabe..d060853 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3352,8 +3352,42 @@ intel_dp_autotest_video_pattern(struct intel_dp
> *intel_dp)
> static uint8_t
> intel_dp_autotest_edid(struct intel_dp *intel_dp)
> {
> + struct drm_connector *connector = &intel_dp->attached_connector->base;
> + struct i2c_adapter *adapter = &intel_dp->aux.ddc;
> + struct edid *edid_read = NULL;
> + uint8_t *edid_data = NULL;
> uint8_t test_result = DP_TEST_NAK;
> - return test_result;
> + uint32_t i = 0, ret = 0, checksum = 0;
> + struct drm_display_mode *use_mode = NULL;
> + dp_compliance_mode comp_mode_type = DP_COMPLIANCE_MODE_PREFERRED;
> + int mode_count = 0;
> +
> + edid_read = drm_get_edid(connector, adapter);
> +
> + DRM_DEBUG_KMS("Displayport: EDID automated test\n");
> +
> + if (edid_read) {
It is customary to have if (!edid_read) and bail out early. Then the
rest will be a happy day scenario with minimal indentation.
BR,
Jani.
> + test_result = true;
> + edid_data = (uint8_t*) edid_read;
> + // Compute checksum
> + for (i = 0; i < 128; i++)
> + checksum += edid_data[i];
> +
> + DRM_DEBUG_KMS("Displayport: EDID test - computed byte sum =
> %02x\n", checksum);
> + // Verify EDID checksum
> + if (checksum % 256 == 0) {
> + /* Write the checksum to EDID checksum register */
> + ret = drm_dp_dpcd_write(&intel_dp->aux,
> DP_TEST_EDID_CHECKSUM, &edid_read->checksum, 1);
> + // Reponse is ACK and and checksum written
> + test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
> + DRM_DEBUG_KMS("Displayport: EDID test - checksum =
> %02x\n", edid_read->checksum);
> + }
> + else {
> + // Invalid checksum, set for failsafe mode
> + comp_mode_type = DP_COMPLIANCE_MODE_FAILSAFE;
> + }
> + }
> +return test_result;
> }
>
> /* Displayport compliance testing - PHY pattern testing */
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type
Il 25/06/2014 09:34, Chen, Tiejun ha scritto: On 2014/6/25 14:48, Paolo Bonzini wrote: Second problem. Your IGD passthrough code currently works with QEMU's PIIX4-based machine. But what happens if you try to extend it, so that Yes, current xen machine, xenpv, is based on pii4, and also I don't known if we will plan to migrate to q35 or others. So its hard to further say more now. it works with QEMU's ICH9-based machine? That's a more modern machine that has a PCIe chipset and hence has its ISA bridge at 00:1f.0. Now But even in this case, could we set the real vendor/device ids for that ISA bridge at 00:1f.0? If not, what's broken? The config space layout changes for different vendor/device ids, so the guest firmware only works if you have the right vendor/device id. It is only slightly better, but the right solution is to fix the driver. There is absolutely zero reason why a graphics driver should know about the vendor/device ids of the PCH. This means we have to fix this both on Linux and Windows but I'm not sure if this is feasible to us. You have to do it if you want this feature in QEMU in a future-proof way. You _can_ provide the ugly PIIX4-specific hack as a compatibility fallback (and this patch is okay to make the compatibility fallback less hacky). However, I don't think QEMU should accept the patch for IGD passthrough unless Intel is willing to make drivers virtualization-friendly. Once you assign the IGD, it is not that integrated anymore and the drivers must take that into account. It is worthwhile pointing out that neither AMD nor nVidia need any of this. The right way could be to make QEMU add a vendor-specific capability to the video device. The driver can probe for that capability before Do you mean we can pick two unused offsets in the configuration space of the video device as a vendor-specific capability to hold the vendor/device ids of the PCH? Yes, either that or add a new capability (which lets you choose the offsets more freely). If the IGD driver needs config space fields of the MCH, those fields could also be mirrored in the new capability. QEMU would forward them automatically. It could even be a new BAR, which gives even more freedom to allocate the fields. looking at the PCI bus. QEMU can add the capability to the list, it is easy because all accesses to the video device's configuration space trap to QEMU. Then you do not need to add fake devices to the machine. In fact, it would be nice if Intel added such a capability on the next generation of integrated graphics, too. On real hardware, ACPI or some Maybe, but even this would be implemented, shouldn't we need to be compatible with those old generations? Yes. - old generation / old driver: use 00:1f.0 hack, only guaranteed to work on PIIX4-based virtual guest - old generation / new driver: use 00:1f.0 hack on real hardware, use capability on 00:02.0 on virtual guest, can work on PCIe virtual guest - new generation / old driver: doesn't exist - new generation / new driver: always use capability on 00:02.0, can work on PCIe virtual guest. Paolo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type
On 2014/6/25 14:48, Paolo Bonzini wrote: Il 22/06/2014 10:25, Chen, Tiejun ha scritto: In qemu-upstream, as you commented we can't create this as a ISA class type explicitly. Note I didn't say that QEMU doesn't like having two ISA bridges. I commented that the firmware will see two ISA bridges and will try to initialize both of them. It currently doesn't just because it only knows of two southbridge PCI IDs, and both of them are old or relatively old (PIIX3/4 and ICH9). But what would happen if you did graphics passthrough on a machine with an ICH9 southbridge? Your code will create the PIIX4 ISA bridge, and will create an ICH9 southbridge just to please the i915 driver. The BIOS will recognize the ICH9's vendor/device ids and try to initialize it. It will write to the configuration space to set up PCI PIRQ[A-H] routing, for example, which makes no sense since your ICH9 is just a dummy device. Thanks for your detailed explanation. Second problem. Your IGD passthrough code currently works with QEMU's PIIX4-based machine. But what happens if you try to extend it, so that Yes, current xen machine, xenpv, is based on pii4, and also I don't known if we will plan to migrate to q35 or others. So its hard to further say more now. it works with QEMU's ICH9-based machine? That's a more modern machine that has a PCIe chipset and hence has its ISA bridge at 00:1f.0. Now But even in this case, could we set the real vendor/device ids for that ISA bridge at 00:1f.0? If not, what's broken? you have a conflict. In other words, if you want IGD passthrough in QEMU, you need a solution that is future-proof. So we compromise by faking this ISA bridge without ISA class type setting (as I recall you already said this way is slightly better). It is only slightly better, but the right solution is to fix the driver. There is absolutely zero reason why a graphics driver should know about the vendor/device ids of the PCH. This means we have to fix this both on Linux and Windows but I'm not sure if this is feasible to us. The right way could be to make QEMU add a vendor-specific capability to the video device. The driver can probe for that capability before Do you mean we can pick two unused offsets in the configuration space of the video device as a vendor-specific capability to hold the vendor/device ids of the PCH? looking at the PCI bus. QEMU can add the capability to the list, it is easy because all accesses to the video device's configuration space trap to QEMU. Then you do not need to add fake devices to the machine. In fact, it would be nice if Intel added such a capability on the next generation of integrated graphics, too. On real hardware, ACPI or some Maybe, but even this would be implemented, shouldn't we need to be compatible with those old generations? Thanks Tiejun other kind of firmware can probe the PCH at 00:1f.0 and initialize that vendor-specific capability. QEMU would just forward the value from the host, so that virtualized guests never depend on the PCH at 00:1f.0. Paolo Maybe we will figure better way in the future. But anyway, this is always registered as 00:15.0, right? So I think the i915 driver can go back to probe the devfn like the native environment. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
