[RFC 5/8] drm/fence: add fence to drm_pending_event
On Fri, Apr 15, 2016 at 11:59:00AM -0700, Gustavo Padovan wrote:
> 2016-04-15 Daniel Vetter :
> > On Thu, Apr 14, 2016 at 06:29:38PM -0700, Gustavo Padovan wrote:
> > > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> > > index aeef58e..38def49 100644
> > > --- a/drivers/gpu/drm/drm_fops.c
> > > +++ b/drivers/gpu/drm/drm_fops.c
> > > @@ -801,8 +801,9 @@ void drm_send_event_locked(struct drm_device *dev,
> > > struct drm_pending_event *e)
> > > {
> > > assert_spin_locked(&dev->event_lock);
> > >
> > > - if (!e->file_priv) {
> > > - e->destroy(e);
> > > + if (!e->file_priv || !e->event) {
> >
> > This would be a bug: e->file_priv != NULL iff e->event != NULL. How did
> > this happen?
>
> Not sure now. But I needed this to prevent a crash, I don't have logs of
> it anymore, I'll check this again.
There was a massive irc discussion with Daniel Stone, so I'll try to
summarize it here. There are 3 possible cases:
e->file_priv == NULL && e->event == NULL:
This is a drm_event without a drm_event. Probably e->fence is set, if not
then it's a completeley useless thing (but not forbidden).
e->file_priv != NULL && e->event != NULL:
drm_event with an event for the given file_priv attached.
e->file_priv == NULL && e->event != NULL:
Above case, but with the file_priv closed and unlinked from the event.
The 4th case, which is the only case things will change with the above
hunk, is not allowed. If you hit it, there's a bug somewhere.
This is all completely idependent of e->fence, which this patch adds.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[RFC 1/8] dma-buf/fence: add fence_collection fences
On Fri, Apr 15, 2016 at 11:27:50AM -0700, Gustavo Padovan wrote: > 2016-04-15 Christian König : > > Amdgpu also has an implementation for a fence collection which uses a a > > hashtable to keep the fences grouped by context (e.g. only the latest fence > > is keept for each context). See amdgpu_sync.c for reference. > > > > We should either make the collection similar in a way that you can add as > > many fences as you want (like the amdgpu implementation) or make it static > > and only add a fixed number of fences right from the beginning. > > > > I can certainly see use cases for both, but if you want to stick with a > > static approach you should probably call the new object fence_array instead > > of fence_collection and do as Daniel suggested. > > Maybe we can go for something in between. Have fence_collection_init() > need at least two fences to create the fence_collection. Then > fence_collection_add() would add more dinamically. The problem with adding fences later on is that it makes it trivial to add deadlocks and loops. Just add the fence collection to itself, boom. From that pov it's an unsafe api, and hence something to avoid. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[RFC 1/8] dma-buf/fence: add fence_collection fences
On Fri, Apr 15, 2016 at 11:29:34AM -0700, Gustavo Padovan wrote: > 2016-04-15 Daniel Vetter : > > > On Fri, Apr 15, 2016 at 11:03 AM, Christian König > > wrote: > > > Might be that how amdgpu uses the fence context and sequence number is a > > > bit > > > questionable, but this will completely break it. > > > > You mean it tries to qualesce fences in the same context down to just > > the last one? That's how it's supposed to be done, and > > fence_collections do break this somewhat. Without fixing up > > fence_is_later and friends. Sounds like amdgpu is a good use case to > > make sure the changes in semantics in these functions result in > > sensible code. In a way a fence_collection is a fence where the > > timeline never matches with any other timeline (since it's a > > combiation). > > > > And yeah I think fence_collection should probably compress down the > > fences to 1 per timeline. But then that's just an implementation > > detail we can fix later on. > > You mean asking for a new context for every collection? That would be one solution, but I fear it's a bit expensive. Having a special-case context for collections might be the better approach. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[Bug 103271] [regression bisected] AMD R9 270X Flickering with DPM Enabled on Linux 4.1 with RadeonSI
https://bugzilla.kernel.org/show_bug.cgi?id=103271 Amarildo changed: What|Removed |Added CC||amarildosjr at riseup.net --- Comment #22 from Amarildo --- Could you guys revert the patch? Maybe there is something that can be changed in the quirk that might let the quirk stay in without causing problems on systems like mine and Kevin's. Regards, Amarildo -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 94921] [drm] rcu slot is busy Logging noise with recent agd5f drm-next-4.7-wip
https://bugs.freedesktop.org/show_bug.cgi?id=94921 Andy Furniss changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #1 from Andy Furniss --- Not seeing these anymore after updating. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/79c6e5d0/attachment.html>
[PATCH 3/7] drm/amdgpu: introduce graphics object id helpers.
Hi Dave,
On 14 April 2016 at 03:56, Dave Airlie wrote:
> +static enum connector_id connector_id_from_bios_object_id(uint32_t
> bios_object_id)
> +{
> + uint32_t bios_connector_id =
> gpu_id_from_bios_object_id(bios_object_id);
> +
> + enum connector_id id;
> +
> + switch (bios_connector_id) {
> + case CONNECTOR_OBJECT_ID_SINGLE_LINK_DVI_I:
> + id = CONNECTOR_ID_SINGLE_LINK_DVII;
> + break;
> + case CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_I:
> + id = CONNECTOR_ID_DUAL_LINK_DVII;
> + break;
> + case CONNECTOR_OBJECT_ID_SINGLE_LINK_DVI_D:
> + id = CONNECTOR_ID_SINGLE_LINK_DVID;
> + break;
> + case CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_D:
> + id = CONNECTOR_ID_DUAL_LINK_DVID;
> + break;
> + case CONNECTOR_OBJECT_ID_VGA:
> + id = CONNECTOR_ID_VGA;
> + break;
> + case CONNECTOR_OBJECT_ID_HDMI_TYPE_A:
> + id = CONNECTOR_ID_HDMI_TYPE_A;
> + break;
> + case CONNECTOR_OBJECT_ID_LVDS:
> + id = CONNECTOR_ID_LVDS;
> + break;
> + case CONNECTOR_OBJECT_ID_PCIE_CONNECTOR:
> + id = CONNECTOR_ID_PCIE;
> + break;
> + case CONNECTOR_OBJECT_ID_HARDCODE_DVI:
> + id = CONNECTOR_ID_HARDCODE_DVI;
> + break;
> + case CONNECTOR_OBJECT_ID_DISPLAYPORT:
> + id = CONNECTOR_ID_DISPLAY_PORT;
> + break;
> + case CONNECTOR_OBJECT_ID_eDP:
> + id = CONNECTOR_ID_EDP;
> + break;
> + case CONNECTOR_OBJECT_ID_MXM:
> + id = CONNECTOR_ID_MXM;
> + break;
> + default:
> + id = CONNECTOR_ID_UNKNOWN;
> + break;
One could move all the new mappings (meaning - here and follow up
patches) to static const table(s), saving a wee bit of space ;-)
-Emil
[PATCH] drm/i915/vlv: Enable/disable VGA hotplugging properly
On Fri, Apr 15, 2016 at 09:47:51AM -0400, Lyude Paul wrote:
> Looks like we might not need to worry about this patch anymore actually, looks
> like this problem got fixed by accident by one of the other vlv fixes you
> pushed.
Not sure what exactly changed for you, but we definitely need to
reinitialize ADPA when re-enabling the power well.
> Now it's not always modesetting on hotplug when it was before though :(,
> so I'll get to work on bisecting that.
>
> On Thu, 2016-04-14 at 20:59 +0300, Ville Syrjälä wrote:
> > On Tue, Mar 29, 2016 at 04:46:30PM -0400, Lyude wrote:
> > >
> > > On Valleyview, VGA hotplugging is controlled through a seperate register
> > > than everything else, VLV_ADPA, which must be explicitly set.
> > >
> > > While VGA hotplugging worked(ish) before, it looks like that was mainly
> > > because we'd unintentionally enable it in
> > > valleyview_crt_detect_hotplug() when we did a force trigger. This
> > > doesn't work reliably enough because whenever the display powerwell on
> > > vlv gets disabled, the values set in VLV_ADPA get cleared and
> > > consequently VGA hotplugging gets disabled. This causes bugs such as one
> > > we found on an Intel NUC, where doing the following sequence of
> > > hotplugs:
> > >
> > > - Disconnect all monitors
> > > - Connect VGA
> > > - Disconnect VGA
> > > - Connect HDMI
> > >
> > > Would result in hotplugging getting disabled, due to the display
> > > powerwells getting toggled in the process of connecting HDMI.
> > >
> > > CC: stable at vger.kernel.org
> > > Signed-off-by: Lyude
> > > ---
> > > Â drivers/gpu/drm/i915/i915_irq.c | 14 ++
> > > Â 1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > b/drivers/gpu/drm/i915/i915_irq.c
> > > index 5aa4239..60592a4 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -3611,6 +3611,7 @@ static void valleyview_display_irqs_install(struct
> > > drm_i915_private *dev_priv)
> > > Â {
> > > Â u32 pipestat_mask;
> > > Â u32 iir_mask;
> > > + u32 adpa_reg;
> > > Â enum pipe pipe;
> > > Â
> > > Â pipestat_mask = PIPESTAT_INT_STATUS_MASK |
> > > @@ -3627,6 +3628,12 @@ static void valleyview_display_irqs_install(struct
> > > drm_i915_private *dev_priv)
> > > Â for_each_pipe(dev_priv, pipe)
> > > Â Â Â Â Â Â Â i915_enable_pipestat(dev_priv, pipe,
> > > pipestat_mask);
> > > Â
> > > + if (IS_VALLEYVIEW(dev_priv)) {
> > > + adpa_reg = I915_READ(VLV_ADPA);
> > > + adpa_reg |= ADPA_CRT_HOTPLUG_ENABLE;
> > > + I915_WRITE(VLV_ADPA, adpa_reg);
> > > + }
> > We might not want to enable that when there's no VGA connector.
> >
> > Seems like we should just be calling intel_crt_reset() here. We
> > definitely don't want to call the reset for hooks for all the other
> > connectors so drm_mode_config_reset() is out. Also the connector
> > locking might be problematic here, so I might suggest adjusting
> > intel_crt_reset() to take an encoder instead of connector, and then
> > we should be able to walk the encoder list without any troubles.
> >
> > >
> > > +
> > > Â iir_mask = I915_DISPLAY_PORT_INTERRUPT |
> > > Â Â Â Â I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> > > Â Â Â Â I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
> > > @@ -3645,8 +3652,15 @@ static void
> > > valleyview_display_irqs_uninstall(struct
> > > drm_i915_private *dev_priv)
> > > Â {
> > > Â u32 pipestat_mask;
> > > Â u32 iir_mask;
> > > + u32 adpa_reg;
> > > Â enum pipe pipe;
> > > Â
> > > + if (IS_VALLEYVIEW(dev_priv)) {
> > > + adpa_reg = I915_READ(VLV_ADPA);
> > > + adpa_reg &= ~ADPA_CRT_HOTPLUG_ENABLE;
> > > + I915_WRITE(VLV_ADPA, adpa_reg);
> > > + }
> > > +
> > > Â iir_mask = I915_DISPLAY_PORT_INTERRUPT |
> > > Â Â Â Â I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> > > Â Â Â Â I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
> > > --Â
> > > 2.5.5
> > >
> > > ___
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
Allocation of frame buffer at a specific memory range or address
On Fri, Apr 15, 2016 at 5:48 PM, Alexey Brodkin wrote: > Hello, > > I'm wondering if there's a way to force kernel to allocate backing > memory for frame buffer in a special location? > > A little bit of background below. > I continue to work on DRM driver for ARC PGU, latest > is v5 and available here - https://lkml.org/lkml/2016/3/28/170 > > In current state everything more or less works but I'd like to > implement one improvement - I'd like to have an ability to specify > where in memory will be allocated frame-buffer's backing storage area. > I.e. buffer which will be read by PGU hardware. > > Currently we use whatever DRM susbsystem put in "gem->paddr". > That's a snippet of the code which tells PGU hardware > which memory location to scan for data: > ->8 > arcpgu = crtc_to_arcpgu_priv(plane->state->crtc); > gem = drm_fb_cma_get_gem_obj(plane->state->fb, 0); > > /* Telling PGU hardware where is data to read */ > arc_pgu_write(arcpgu, ARCPGU_REG_BUF0_ADDR, gem->paddr); > ->8 > > But we may use a special memory area which works better > for that case, i.e. for example could be accessed faster by both > CPU and PGU hardware (like on-chip SRAM as opposed to external DDR). > > And now the question is how to force DRM subsystem or just that driver > to use whatever predefined (say via device tree) location in memory > for data buffer allocation. > > All thoughts on this are more than welcome. Instead of using cma, you need to create your own gem buffer objects and allocate the memory for those from wherever you want to. Or you can do a mix of cma gem allocations and other objects (e.g. omapdrm does that). Using cma (which simply uses dma_alloc_coherent underneath) is just a convenient way, and generally the right choice on SoC display drivers. There's no need to use that at all, KMS is fully agnostic to how/what your underlying buffer objects even are. You could have allocations in some sepcial secure memory that only the gpu could even read/write, as another example. Maybe we need to make this part in the overview docs better? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[RESEND PATCH] drm/panel: simple-panel: set appropriate mode type
From: Boris Brezillon All modes exposed by simple panels should be tagged as driver defined modes. Moreover, if a panel supports only one mode, this mode is obviously the preferred one. Doing this also fix a problem occurring when a 'video=' parameter is passed on the kernel cmdline. In some cases the user provided mode is preferred over the simple panel ones, which might result in unpredictable behavior. Signed-off-by: Boris Brezillon Reviewed-by: Nicolas Ferre Tested-by: Nicolas Ferre --- drivers/gpu/drm/panel/panel-simple.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index ceb20486dacf..45b924ebed57 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -115,6 +115,10 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel) continue; } + mode->type |= DRM_MODE_TYPE_DRIVER; + if (panel->desc->num_modes == 1) + mode->type |= DRM_MODE_TYPE_PREFERRED; + drm_display_mode_from_videomode(&vm, mode); drm_mode_set_name(mode); @@ -132,6 +136,10 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel) continue; } + mode->type |= DRM_MODE_TYPE_DRIVER; + if (panel->desc->num_modes == 1) + mode->type |= DRM_MODE_TYPE_PREFERRED; + drm_mode_set_name(mode); drm_mode_probed_add(connector, mode); -- 2.1.3
[Bug 94726] [Tonga] ARK: Survival Evolved crashes on savegame load. Out of Memory
https://bugs.freedesktop.org/show_bug.cgi?id=94726 --- Comment #9 from thomas.rinsch at arcor.de --- (In reply to Nicolai H�hnle from comment #8) > Thanks for testing, and sorry to hear that. > > Is there anything in the dmesg? Would you mind uploading another apitrace? Of course, here is a new API trace: https://drive.google.com/file/d/0BwKS4-SC1bfyQlM4dDNkNTNIOUE/view?usp=sharing Nothing in dmesg or /var/log/messages However obviously I can't get the dmesg when the system is frozen. Any way to automatically log it on change? I just noticed that the system gets extremely unresponsive shortly before freezing completely if that is of any help. System Monitor shows normal CPU and Memory usage until the end, so I would imagine it's gpu related. (max 5.2GB used). Btw. this might be very good to improve the driver. However if you find this to be an error on the application side, notice that the developer has announced a major rework of the OpenGL renderer. Best regards and many thanks again. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/6da695f5/attachment.html>
[PATCH] drm: panel: simple-panel: set appropriate mode type
Le 28/05/2015 16:25, Boris Brezillon a écrit : > Hi Thierry, > > Could you have a look at this patch (a.k.a. ping) ? It's been a long time and... It seems that this patch doesn't apply anymore but: Reviewed-by: Nicolas Ferre So, I'm updating it and resending right now. Bye, > On Thu, 30 Apr 2015 16:39:30 +0200 > Boris Brezillon wrote: > >> All modes exposed by simple panels should be tagged as driver defined >> modes. >> Moreover, if a panel supports only one mode, this mode is obviously the >> preferred one. >> >> Doing this also fix a problem occurring when a 'video=' parameter is passed >> on the kernel cmdline. In some cases the user provided mode is preferred >> over the simple panel ones, which might result in unpredictable behavior. >> >> Signed-off-by: Boris Brezillon >> --- >> drivers/gpu/drm/panel/panel-simple.c | 8 >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/gpu/drm/panel/panel-simple.c >> b/drivers/gpu/drm/panel/panel-simple.c >> index d14b904..95ae390 100644 >> --- a/drivers/gpu/drm/panel/panel-simple.c >> +++ b/drivers/gpu/drm/panel/panel-simple.c >> @@ -111,6 +111,10 @@ static int panel_simple_get_fixed_modes(struct >> panel_simple *panel) >> continue; >> } >> >> +mode->type |= DRM_MODE_TYPE_DRIVER; >> +if (panel->desc->num_modes == 1) >> +mode->type |= DRM_MODE_TYPE_PREFERRED; >> + >> drm_display_mode_from_videomode(&vm, mode); >> >> drm_mode_probed_add(connector, mode); >> @@ -127,6 +131,10 @@ static int panel_simple_get_fixed_modes(struct >> panel_simple *panel) >> continue; >> } >> >> +mode->type |= DRM_MODE_TYPE_DRIVER; >> +if (panel->desc->num_modes == 1) >> +mode->type |= DRM_MODE_TYPE_PREFERRED; >> + >> drm_mode_set_name(mode); >> >> drm_mode_probed_add(connector, mode); > > > -- Nicolas Ferre
[Bug 93478] [amdgpu] [tonga] VM Faults and card lock up
https://bugs.freedesktop.org/show_bug.cgi?id=93478 Mike Lothian changed: What|Removed |Added CC||mike at fireburn.co.uk --- Comment #2 from Mike Lothian --- It's been awhile since I've played, I'll give it another go at the weekend and see how I get on -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/47d58a37/attachment-0001.html>
[Bug 67713] Freezes on Trinity 7500G
https://bugs.freedesktop.org/show_bug.cgi?id=67713 Mike Lothian changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |INVALID --- Comment #11 from Mike Lothian --- No longer have access to hardware -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/907f1bf3/attachment.html>
[Bug 93217] [tonga] [powerplay] Radon M395X isn't initialised with the powerplay branch
https://bugs.freedesktop.org/show_bug.cgi?id=93217 Mike Lothian changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #32 from Mike Lothian --- I've just tried booting without using the PCIe DPM patch and it now seems to work Not sure if it was a commit or the new firmware - either way this works for me now -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/c10246ce/attachment.html>
[Bug 93460] [amdgpu] Ooops during shutdown - amdgpu_vm_grab_id
https://bugs.freedesktop.org/show_bug.cgi?id=93460 Mike Lothian changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #9 from Mike Lothian --- Not seen this in a while now -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/eb29ee2c/attachment.html>
[PATCH v3 1/9] ARM: imx: clk-vf610: fix DCU clock tree
On 04/04, Stefan Agner wrote: > Similar to an earlier fix for the SAI clocks, the DCU clock hierarchy > mixes the bus clock with the display controllers pixel clock. Tests > have shown that the gates in CCM_CCGR3/9 registers do not control > the DCU pixel clock, but only the register access clock (bus clock). > > Fix this by defining the parent clock of VF610_CLK_DCUx to be the bus > clock (ipg_bus). > > Since the clock has not been used far, there are no further changes > needed. > > Signed-off-by: Stefan Agner > --- Acked-by: Stephen Boyd -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v6 3/4] drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read()
On Fri, Apr 15, 2016 at 10:25:35AM -0400, Lyude wrote:
> This is part of a patch series to migrate all of the workarounds for
> commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's
> DP helper.
>
> Some sinks will just return garbage for the first aux tranaction they
> receive when coming out of sleep mode, so we need to perform an additional
> read before the actual read to workaround this.
>
> Changes since v5
> - If the throwaway read in drm_dp_dpcd_read() fails, return the error
> from that instead of continuing. This follows the same logic we do in
> drm_dp_dpcd_access() (e.g. the error from the first transaction may
> differ from the errors that proceeding attempts might return).
>
> Signed-off-by: Lyude
Reviewed-by: Ville Syrjälä
Tested-by: Ville Syrjälä
> ---
> drivers/gpu/drm/drm_dp_helper.c | 19 +++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 540c3e4..eeaf5a7 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -248,6 +248,25 @@ unlock:
> ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
>void *buffer, size_t size)
> {
> + int ret;
> +
> + /*
> + * HP ZR24w corrupts the first DPCD access after entering power save
> + * mode. Eg. on a read, the entire buffer will be filled with the same
> + * byte. Do a throw away read to avoid corrupting anything we care
> + * about. Afterwards things will work correctly until the monitor
> + * gets woken up and subsequently re-enters power save mode.
> + *
> + * The user pressing any button on the monitor is enough to wake it
> + * up, so there is no particularly good place to do the workaround.
> + * We just have to do it before any DPCD access and hope that the
> + * monitor doesn't power down exactly after the throw away read.
> + */
> + ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, DP_DPCD_REV, buffer,
> + 1);
> + if (ret != 1)
> + return ret;
> +
> return drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, buffer,
> size);
> }
> --
> 2.5.5
--
Ville Syrjälä
Intel OTC
[Nouveau] [PATCH 1/2] nouveau/bl: Assign different names to interfaces
er would be so much simpler... --" I was planning to use a dynamic size to allow numbers bigger than 99, and lesser than 10, but stopped midway through the process. Iâll revert to a plain stack-allocated. > > > + const int nb = atomic_inc_return(&bl_interfaces_nb) - 1; > > This kinda sucks if you reload nouveau a bunch. How about using an > "ida". Have a look in drivers/gpu/drm/drm_crtc.c for how I use that > one. Yeah, Iâm not really fond of that part. Iâll have a look at drm_crtc.c! Pierre > > > + if (nb > 0 && nb < 100) > > + sprintf(backlight_name, "nv_backlight%d", nb); > > + else > > + sprintf(backlight_name, "nv_backlight"); > > + return backlight_name; > > +} > > -- > > 2.8.0 > > > > ___ > > Nouveau mailing list > > Nouveau at lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/nouveau -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/37ae3e4d/attachment.sig>
[PATCH 2/2] drm/amdgpu: remove sorting of CS BOs
From: Christian König
Not needed any more.
Signed-off-by: Christian König
Reviewed-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 23 ---
1 file changed, 23 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 9392e50..00cf74a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -24,7 +24,6 @@
* Authors:
*Jerome Glisse
*/
-#include
#include
#include
#include
@@ -527,16 +526,6 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p)
return 0;
}
-static int cmp_size_smaller_first(void *priv, struct list_head *a,
- struct list_head *b)
-{
- struct amdgpu_bo_list_entry *la = list_entry(a, struct
amdgpu_bo_list_entry, tv.head);
- struct amdgpu_bo_list_entry *lb = list_entry(b, struct
amdgpu_bo_list_entry, tv.head);
-
- /* Sort A before B if A is smaller. */
- return (int)la->robj->tbo.num_pages - (int)lb->robj->tbo.num_pages;
-}
-
/**
* cs_parser_fini() - clean parser states
* @parser:parser structure holding parsing context.
@@ -553,18 +542,6 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser
*parser, int error, bo
if (!error) {
amdgpu_vm_move_pt_bos_in_lru(parser->adev, &fpriv->vm);
- /* Sort the buffer list from the smallest to largest buffer,
-* which affects the order of buffers in the LRU list.
-* This assures that the smallest buffers are added first
-* to the LRU list, so they are likely to be later evicted
-* first, instead of large buffers whose eviction is more
-* expensive.
-*
-* This slightly lowers the number of bytes moved by TTM
-* per frame under memory pressure.
-*/
- list_sort(NULL, &parser->validated, cmp_size_smaller_first);
-
ttm_eu_fence_buffer_objects(&parser->ticket,
&parser->validated,
parser->fence);
--
2.5.0
[PATCH 1/2] drm/amdgpu: group BOs by log2 of the size on the LRU v2
From: Christian König
This allows us to have small BOs on the LRU before big ones.
v2: fix of by one and list corruption bug
Signed-off-by: Christian König
Reviewed-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 11 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61 +++--
2 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index c4a21c6..7b90323 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -391,6 +391,14 @@ unsigned amdgpu_fence_count_emitted(struct amdgpu_ring
*ring);
/*
* TTM.
*/
+
+#define AMDGPU_TTM_LRU_SIZE20
+
+struct amdgpu_mman_lru {
+ struct list_head*lru[TTM_NUM_MEM_TYPES];
+ struct list_head*swap_lru;
+};
+
struct amdgpu_mman {
struct ttm_bo_global_refbo_global_ref;
struct drm_global_reference mem_global_ref;
@@ -408,6 +416,9 @@ struct amdgpu_mman {
struct amdgpu_ring *buffer_funcs_ring;
/* Scheduler entity for buffer moves */
struct amd_sched_entity entity;
+
+ /* custom LRU management */
+ struct amdgpu_mman_lru log2_size[AMDGPU_TTM_LRU_SIZE];
};
int amdgpu_copy_buffer(struct amdgpu_ring *ring,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index fefaa9b..27f3f47 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -910,6 +910,52 @@ uint32_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device
*adev, struct ttm_tt *ttm,
return flags;
}
+static void amdgpu_ttm_lru_removal(struct ttm_buffer_object *tbo)
+{
+ struct amdgpu_device *adev = amdgpu_get_adev(tbo->bdev);
+ unsigned i, j;
+
+ for (i = 0; i < AMDGPU_TTM_LRU_SIZE; ++i) {
+ struct amdgpu_mman_lru *lru = &adev->mman.log2_size[i];
+
+ for (j = 0; j < TTM_NUM_MEM_TYPES; ++j)
+ if (&tbo->lru == lru->lru[j])
+ lru->lru[j] = tbo->lru.prev;
+
+ if (&tbo->swap == lru->swap_lru)
+ lru->swap_lru = tbo->swap.prev;
+ }
+}
+
+static struct amdgpu_mman_lru *amdgpu_ttm_lru(struct ttm_buffer_object *tbo)
+{
+ struct amdgpu_device *adev = amdgpu_get_adev(tbo->bdev);
+ unsigned log2_size = min(ilog2(tbo->num_pages),
+AMDGPU_TTM_LRU_SIZE - 1);
+
+ return &adev->mman.log2_size[log2_size];
+}
+
+static struct list_head *amdgpu_ttm_lru_tail(struct ttm_buffer_object *tbo)
+{
+ struct amdgpu_mman_lru *lru = amdgpu_ttm_lru(tbo);
+ struct list_head *res = lru->lru[tbo->mem.mem_type];
+
+ lru->lru[tbo->mem.mem_type] = &tbo->lru;
+
+ return res;
+}
+
+static struct list_head *amdgpu_ttm_swap_lru_tail(struct ttm_buffer_object
*tbo)
+{
+ struct amdgpu_mman_lru *lru = amdgpu_ttm_lru(tbo);
+ struct list_head *res = lru->swap_lru;
+
+ lru->swap_lru = &tbo->swap;
+
+ return res;
+}
+
static struct ttm_bo_driver amdgpu_bo_driver = {
.ttm_tt_create = &amdgpu_ttm_tt_create,
.ttm_tt_populate = &amdgpu_ttm_tt_populate,
@@ -923,12 +969,14 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
.fault_reserve_notify = &amdgpu_bo_fault_reserve_notify,
.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
.io_mem_free = &amdgpu_ttm_io_mem_free,
- .lru_tail = &ttm_bo_default_lru_tail,
- .swap_lru_tail = &ttm_bo_default_swap_lru_tail,
+ .lru_removal = &amdgpu_ttm_lru_removal,
+ .lru_tail = &amdgpu_ttm_lru_tail,
+ .swap_lru_tail = &amdgpu_ttm_swap_lru_tail,
};
int amdgpu_ttm_init(struct amdgpu_device *adev)
{
+ unsigned i, j;
int r;
r = amdgpu_ttm_global_init(adev);
@@ -946,6 +994,15 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
DRM_ERROR("failed initializing buffer object driver(%d).\n", r);
return r;
}
+
+ for (i = 0; i < AMDGPU_TTM_LRU_SIZE; ++i) {
+ struct amdgpu_mman_lru *lru = &adev->mman.log2_size[i];
+
+ for (j = 0; j < TTM_NUM_MEM_TYPES; ++j)
+ lru->lru[j] = &adev->mman.bdev.man[j].lru;
+ lru->swap_lru = &adev->mman.bdev.glob->swap_lru;
+ }
+
adev->mman.initialized = true;
r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_VRAM,
adev->mc.real_vram_size >> PAGE_SHIFT);
--
2.5.0
[PATCH v3 11/19] drm/panel: simple: Add timings for the Olimex LCD-OLinuXino-4.3TS
On Wed, Mar 23, 2016 at 05:38:34PM +0100, Maxime Ripard wrote: > Add support for the Olimex LCD-OLinuXino-4.3TS panel to the DRM simple > panel driver. > > It is a 480x272 panel connected through a 24-bits RGB interface. > > Signed-off-by: Maxime Ripard > Acked-by: Rob Herring > --- > .../display/panel/olimex,lcd-olinuxino-43-ts.txt | 7 ++ > drivers/gpu/drm/panel/panel-simple.c | 26 > ++ > 2 files changed, 33 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/olimex,lcd-olinuxino-43-ts.txt Applied, thanks. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/85336cbb/attachment.sig>
[PATCH 2/2] nouveau/bl: Do not register interface if Apple GMUX detected
The Apple GMUX is the one managing the backlight, so there is no need for
Nouveau to register its own backlight interface.
Signed-off-by: Pierre Moreau
---
drm/nouveau/nouveau_backlight.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
index 914e2cb..37a1577 100644
--- a/drm/nouveau/nouveau_backlight.c
+++ b/drm/nouveau/nouveau_backlight.c
@@ -30,6 +30,7 @@
* Register locations derived from NVClock by Roderick Colenbrander
*/
+#include
#include
#include "nouveau_drm.h"
@@ -239,6 +240,12 @@ nouveau_backlight_init(struct drm_device *dev)
struct nvif_device *device = &drm->device;
struct drm_connector *connector;
+ if (apple_gmux_present()) {
+ NV_INFO(drm, "Apple GMUX detected: not registering Nouveau"
+ " backlight interface");
+ return 0;
+ }
+
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS &&
connector->connector_type != DRM_MODE_CONNECTOR_eDP)
--
2.8.0
[PATCH 1/2] nouveau/bl: Assign different names to interfaces
Currently, every backlight interface created by Nouveau uses the same name,
nv_backlight. This leads to a sysfs warning as it tries to create an already
existing folder. This patch adds a incremented number to the name, but keeps
the initial name as nv_backlight, to avoid possibly breaking userspace; the
second interface will be named nv_backlight1, and so on.
Fixes: fdo#86539
Signed-off-by: Pierre Moreau
---
drm/nouveau/nouveau_backlight.c | 35 +--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
index 89eb460..914e2cb 100644
--- a/drm/nouveau/nouveau_backlight.c
+++ b/drm/nouveau/nouveau_backlight.c
@@ -36,6 +36,10 @@
#include "nouveau_reg.h"
#include "nouveau_encoder.h"
+static atomic_t bl_interfaces_nb = { 0 };
+
+static char* nouveau_get_backlight_name(void);
+
static int
nv40_get_intensity(struct backlight_device *bd)
{
@@ -74,6 +78,7 @@ nv40_backlight_init(struct drm_connector *connector)
struct nvif_object *device = &drm->device.object;
struct backlight_properties props;
struct backlight_device *bd;
+ char* backlight_name = NULL;
if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) & NV40_PMC_BACKLIGHT_MASK))
return 0;
@@ -81,8 +86,14 @@ nv40_backlight_init(struct drm_connector *connector)
memset(&props, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 31;
- bd = backlight_device_register("nv_backlight", connector->kdev, drm,
+ backlight_name = nouveau_get_backlight_name();
+ bd = backlight_device_register(backlight_name , connector->kdev, drm,
&nv40_bl_ops, &props);
+
+ // backlight_device_register() makes a copy
+ kfree(backlight_name);
+ backlight_name = NULL;
+
if (IS_ERR(bd))
return PTR_ERR(bd);
drm->backlight = bd;
@@ -182,6 +193,7 @@ nv50_backlight_init(struct drm_connector *connector)
struct backlight_properties props;
struct backlight_device *bd;
const struct backlight_ops *ops;
+ char* backlight_name = NULL;
nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS);
if (!nv_encoder) {
@@ -203,8 +215,14 @@ nv50_backlight_init(struct drm_connector *connector)
memset(&props, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 100;
- bd = backlight_device_register("nv_backlight", connector->kdev,
+ backlight_name = nouveau_get_backlight_name();
+ bd = backlight_device_register(backlight_name, connector->kdev,
nv_encoder, ops, &props);
+
+ // backlight_device_register() makes a copy
+ kfree(backlight_name);
+ backlight_name = NULL;
+
if (IS_ERR(bd))
return PTR_ERR(bd);
@@ -252,3 +270,16 @@ nouveau_backlight_exit(struct drm_device *dev)
drm->backlight = NULL;
}
}
+
+static char*
+nouveau_get_backlight_name(void)
+{
+ // 12 chars for "nv_backlight" + 2 for two digits number + 1 for '\0'
+ char* backlight_name = (char*)kmalloc(sizeof(char[15]), GFP_KERNEL);
+ const int nb = atomic_inc_return(&bl_interfaces_nb) - 1;
+ if (nb > 0 && nb < 100)
+ sprintf(backlight_name, "nv_backlight%d", nb);
+ else
+ sprintf(backlight_name, "nv_backlight");
+ return backlight_name;
+}
--
2.8.0
[PATCH 2/5] panel-simple: Add the 7" DPI panel from Adafruit.
On Thu, Mar 24, 2016 at 05:23:48PM -0700, Eric Anholt wrote: > This is a basic TFT panel with a 40-pin FPC connector on it. The > specification doesn't define timings, but the Adafruit instructions > were setting up 800x480 CVT. > > v2: Add .bus_format and vsync/hsync flags. > > Signed-off-by: Eric Anholt > Acked-by: Rob Herring > --- > .../bindings/display/panel/ontat,yx700wv03.txt | 7 + > drivers/gpu/drm/panel/panel-simple.c | 35 > ++ > 2 files changed, 42 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/ontat,yx700wv03.txt Applied, thanks. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/e570ea0d/attachment.sig>
[PATCH 1/5] of: Add vendor prefix for On Tat Industrial Company.
On Thu, Mar 24, 2016 at 05:23:47PM -0700, Eric Anholt wrote: > This is the vendor for a 7" DPI panel sold by Adafruit which I'd like > to describe in DT. > > Signed-off-by: Eric Anholt > Acked-by: Rob Herring > --- > Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + > 1 file changed, 1 insertion(+) Applied, thanks. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/144654cb/attachment.sig>
[Bug 94874] radeon: Failed to allocate virtual address for buffer
: 65536 bytes radeon:alignment : 4096 bytes radeon:domains : 4 radeon:va: 0x0080 radeon: Failed to deallocate virtual address for buffer: radeon:size : 65536 bytes radeon:va: 0x80 radeonsi: Failed to create a context. radeon: Failed to allocate virtual address for buffer: radeon:size : 65536 bytes radeon:alignment : 4096 bytes radeon:domains : 4 radeon:va: 0x0080 radeon: Failed to deallocate virtual address for buffer: radeon:size : 65536 bytes radeon:va: 0x80 radeon: Failed to allocate virtual address for buffer: radeon:size : 65536 bytes radeon:alignment : 4096 bytes radeon:domains : 4 radeon:va: 0x0080 radeon: Failed to deallocate virtual address for buffer: radeon:size : 65536 bytes radeon:va: 0x80 radeonsi: Failed to create a context. radeon: Failed to allocate virtual address for buffer: radeon:size : 65536 bytes radeon:alignment : 4096 bytes radeon:domains : 4 radeon:va: 0x0080 radeon: Failed to deallocate virtual address for buffer: radeon:size : 65536 bytes radeon:va: 0x80 radeon: Failed to allocate virtual address for buffer: radeon:size : 65536 bytes radeon:alignment : 4096 bytes radeon:domains : 4 radeon:va: 0x0080 radeon: Failed to deallocate virtual address for buffer: radeon:size : 65536 bytes radeon:va: 0x80 radeonsi: Failed to create a context. radeon: Failed to allocate virtual address for buffer: radeon:size : 65536 bytes radeon:alignment : 4096 bytes radeon:domains : 4 radeon:va: 0x0080 radeon: Failed to deallocate virtual address for buffer: radeon:size : 65536 bytes radeon:va: 0x80 radeon: Failed to allocate virtual address for buffer: radeon:size : 65536 bytes radeon:alignment : 4096 bytes radeon:domains : 4 radeon:va: 0x0080 radeon: Failed to deallocate virtual address for buffer: radeon:size : 65536 bytes radeon:va: 0x80 radeonsi: Failed to create a context. X Error of failed request: GLXBadContext Major opcode of failed request: 154 (GLX) Minor opcode of failed request: 6 (X_GLXIsDirect) Serial number of failed request: 43 Current serial number in output stream: 42 and in dmesg: Abr 15 13:37:57 hydra kernel: [drm] probing gen 2 caps for device 8086:9c18 = 5323c42/0 Abr 15 13:37:57 hydra kernel: [drm] PCIE gen 2 link speeds already enabled Abr 15 13:37:57 hydra kernel: [drm] PCIE GART of 2048M enabled (table at 0x002E8000). Abr 15 13:37:57 hydra kernel: radeon :03:00.0: WB enabled Abr 15 13:37:57 hydra kernel: radeon :03:00.0: fence driver on ring 0 use gpu addr 0x8c00 and cpu addr 0x8801a3202c00 Abr 15 13:37:57 hydra kernel: radeon :03:00.0: fence driver on ring 1 use gpu addr 0x8c04 and cpu addr 0x8801a3202c04 Abr 15 13:37:57 hydra kernel: radeon :03:00.0: fence driver on ring 2 use gpu addr 0x8c08 and cpu addr 0x8801a3202c08 Abr 15 13:37:57 hydra kernel: radeon :03:00.0: fence driver on ring 3 use gpu addr 0x8c0c and cpu addr 0x8801a3202c0c Abr 15 13:37:57 hydra kernel: radeon :03:00.0: fence driver on ring 4 use gpu addr 0x8c10 and cpu addr 0x8801a3202c10 Abr 15 13:37:57 hydra kernel: radeon :03:00.0: fence driver on ring 5 use gpu addr 0x00075a18 and cpu addr 0xc90001435a18 Abr 15 13:37:57 hydra kernel: radeon :03:00.0: VCE init error (-22). Abr 15 13:37:58 hydra kernel: [drm:r600_ring_test [radeon]] *ERROR* radeon: ring 0 test failed (scratch(0x850C)=0xCAFEDEAD) Abr 15 13:37:58 hydra kernel: [drm:si_resume [radeon]] *ERROR* si startup failed on resume same behaviour with opencl call, like clinfo mesa git eeff13315858fcb09eefba9a94e6bae5820572e0 llvm svn266408 xf86-video-ati 1ca677309720e2f6c953c9e76f5b34c22a4416c6 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/4959018f/attachment-0001.html>
[PATCH] drm/amdgpu: fix compare_const_fl.cocci warnings
On Fri, 15 Apr 2016, Christian König wrote:
> Am 15.04.2016 um 09:15 schrieb Julia Lawall:
> > Move constants to the right of binary operators.
> >
> > Generated by: scripts/coccinelle/misc/compare_const_fl.cocci
> >
> > Signed-off-by: Fengguang Wu
> > Signed-off-by: Julia Lawall
>
> In general the patch looks ok, but do we have a documented preference where to
> place constants in the coding style docs?
>
> While it's not so much of a problem any more with modern compilers, some
> people still prefer to have it on the left side to catch accidental value
> assignments.
I don't know if it is documented. Joe Perches suggested that on the right
was better in general - maybe he knows if this is written somewhere.
There are 504 occurrences of NULL == in the kernel, and 19524 occurrences
of == NULL.
julia
>
> Regards,
> Christian.
>
> > ---
> >
> > Could be nice to put the thing being tested first.
> >
> > amdgpu_grph_object_id_helpers.c |4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_grph_object_id_helpers.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_grph_object_id_helpers.c
> > @@ -169,11 +169,11 @@ struct graphics_object_id amdgpu_object_
> > struct graphics_object_id go_id = { 0 };
> > type = object_type_from_bios_object_id(bios_object_id);
> > - if (OBJECT_TYPE_UNKNOWN == type)
> > + if (type == OBJECT_TYPE_UNKNOWN)
> > return go_id;
> > enum_id = enum_id_from_bios_object_id(bios_object_id);
> > - if (ENUM_ID_UNKNOWN == enum_id)
> > + if (enum_id == ENUM_ID_UNKNOWN)
> > return go_id;
> > go_id = display_graphics_object_id_init(
>
>
[PATCH] drm/amdgpu: fix compare_const_fl.cocci warnings
On 15 April 2016 at 15:20, Julia Lawall wrote: > On Fri, 15 Apr 2016, Christian König wrote: >> Am 15.04.2016 um 09:15 schrieb Julia Lawall: >> > Move constants to the right of binary operators. >> > >> > Generated by: scripts/coccinelle/misc/compare_const_fl.cocci >> > >> > Signed-off-by: Fengguang Wu >> > Signed-off-by: Julia Lawall >> >> In general the patch looks ok, but do we have a documented preference where >> to >> place constants in the coding style docs? >> >> While it's not so much of a problem any more with modern compilers, some >> people still prefer to have it on the left side to catch accidental value >> assignments. > > I don't know if it is documented. Joe Perches suggested that on the right > was better in general - maybe he knows if this is written somewhere. > > There are 504 occurrences of NULL == in the kernel, and 19524 occurrences > of == NULL. > To throw in some more numbers: >From drivers/gpu/drm/amd/ - ~40 for "NULL *== *" and ~400 for " *== *NULL" ;-) -Emil
[PATCH v2 2/2] drm/i915/vlv: Reset the ADPA in vlv_display_power_well_init()
While VGA hotplugging worked(ish) before, it looks like that was mainly
because we'd unintentionally enable it in
valleyview_crt_detect_hotplug() when we did a force trigger. This
doesn't work reliably enough because whenever the display powerwell on
vlv gets disabled, the values set in VLV_ADPA get cleared and
consequently VGA hotplugging gets disabled. This causes bugs such as one
we found on an Intel NUC, where doing the following sequence of
hotplugs:
- Disconnect all monitors
- Connect VGA
- Disconnect VGA
- Connect HDMI
Would result in VGA hotplugging becoming disabled, due to the powerwells
getting toggled in the process of connecting HDMI.
Changes since v1:
- Instead of handling the register writes ourself, we just reuse
intel_crt_detect()
- Instead of resetting the ADPA during display IRQ installation, we now
reset them in vlv_display_power_well_init()
CC: stable at vger.kernel.org
Signed-off-by: Lyude
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 80e8bd4..c7d195f 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -902,6 +902,7 @@ static bool vlv_power_well_enabled(struct drm_i915_private
*dev_priv,
static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
{
+ struct drm_encoder *encoder, *vga_encoder = NULL;
enum pipe pipe;
/*
@@ -935,6 +936,17 @@ static void vlv_display_power_well_init(struct
drm_i915_private *dev_priv)
intel_hpd_init(dev_priv);
+ /* Re-enable the ADPA, if we have one */
+ drm_for_each_encoder(encoder, dev_priv->dev) {
+ if (encoder->encoder_type == DRM_MODE_ENCODER_DAC) {
+ vga_encoder = encoder;
+ break;
+ }
+ }
+
+ if (vga_encoder && vga_encoder->funcs->reset)
+ vga_encoder->funcs->reset(vga_encoder);
+
i915_redisable_vga_power_on(dev_priv->dev);
}
--
2.5.5
[PATCH v2 1/2] drm/i915/vlv: Make intel_crt_reset() per-encoder
This lets call intel_crt_reset() in contexts where IRQs are disabled and
as such, can't hold the locks required to work with the connectors.
CC: stable at vger.kernel.org
Signed-off-by: Lyude
---
drivers/gpu/drm/i915/intel_crt.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index a2a31fd..220ca91 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -707,11 +707,11 @@ static int intel_crt_set_property(struct drm_connector
*connector,
return 0;
}
-static void intel_crt_reset(struct drm_connector *connector)
+static void intel_crt_reset(struct drm_encoder *encoder)
{
- struct drm_device *dev = connector->dev;
+ struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crt *crt = intel_attached_crt(connector);
+ struct intel_crt *crt = intel_encoder_to_crt(to_intel_encoder(encoder));
if (INTEL_INFO(dev)->gen >= 5) {
u32 adpa;
@@ -733,7 +733,6 @@ static void intel_crt_reset(struct drm_connector *connector)
*/
static const struct drm_connector_funcs intel_crt_connector_funcs = {
- .reset = intel_crt_reset,
.dpms = drm_atomic_helper_connector_dpms,
.detect = intel_crt_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
@@ -751,6 +750,7 @@ static const struct drm_connector_helper_funcs
intel_crt_connector_helper_funcs
};
static const struct drm_encoder_funcs intel_crt_enc_funcs = {
+ .reset = intel_crt_reset,
.destroy = intel_encoder_destroy,
};
@@ -896,5 +896,5 @@ void intel_crt_init(struct drm_device *dev)
dev_priv->fdi_rx_config = I915_READ(FDI_RX_CTL(PIPE_A)) &
fdi_config;
}
- intel_crt_reset(connector);
+ intel_crt_reset(&crt->base.base);
}
--
2.5.5
[PATCH v3 04/19] clk: sunxi: Add TCON channel1 clock
On 03/23, Maxime Ripard wrote: > The TCON is a controller generating the timings to output videos signals, > acting like both a CRTC and an encoder. > > It has two channels depending on the output, each channel being driven by > its own clock (and own clock controller). > > Add a driver for the channel 1 clock. > > Signed-off-by: Maxime Ripard > Acked-by: Rob Herring > --- Acked-by: Stephen Boyd -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v3 03/19] clk: sunxi: Add PLL3 clock
On 03/23, Maxime Ripard wrote: > The A10 SoCs and relatives have a PLL controller to drive the PLL3 and > PLL7, clocked from a 3MHz oscillator, that drives the display related > clocks (GPU, display engine, TCON, etc.) > > Add a driver for it. > > Acked-by: Rob Herring > Acked-by: Chen-Yu Tsai > Signed-off-by: Maxime Ripard > --- Acked-by: Stephen Boyd -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v3 02/19] clk: sunxi: Add display and TCON0 clocks driver
On 03/23, Maxime Ripard wrote:
> diff --git a/drivers/clk/sunxi/clk-sun4i-display.c
> b/drivers/clk/sunxi/clk-sun4i-display.c
> new file mode 100644
> index ..af7d1faebdec
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-sun4i-display.c
> @@ -0,0 +1,262 @@
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct sun4i_a10_display_clk_data {
> + boolhas_div;
> + u8 has_rst;
Can this be num_resets? It's not a bool but name starts with
"has".
> + u8 parents;
> +
> + u8 offset_en;
> + u8 offset_div;
> + u8 offset_mux;
> + u8 offset_rst;
> +
> + u8 width_div;
> + u8 width_mux;
> +};
> +
> +
> +static int sun4i_a10_display_reset_xlate(struct reset_controller_dev *rcdev,
> + const struct of_phandle_args *spec)
> +{
> + /* We only have a single reset signal */
> + return 0;
> +}
Is there a default function for this case in the reset framework?
> +
> +static void __init sun4i_a10_display_init(struct device_node *node,
> + struct sun4i_a10_display_clk_data
> *data)
const?
> +{
> + const char *parents[data->parents];
> + const char *clk_name = node->name;
> + struct reset_data *reset_data;
> + struct clk_divider *div = NULL;
> + struct clk_gate *gate;
> + struct resource res;
> + struct clk_mux *mux;
> + void __iomem *reg;
> + struct clk *clk;
> + int ret;
> +
> + of_property_read_string(node, "clock-output-names", &clk_name);
> +
> + reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> + if (IS_ERR(reg)) {
> + pr_err("%s: Could not map the clock registers\n", clk_name);
> + return;
> + }
> +
> + ret = of_clk_parent_fill(node, parents, data->parents);
> + if (ret != data->parents) {
> + pr_err("%s Could not retrieve the parents\n", clk_name);
Missing ':'?
> + goto unmap;
> + }
> +
[..]
> +
> + ret = of_clk_add_provider(node, of_clk_src_simple_get, clk);
> + if (ret) {
> + pr_err("%s: Couldn't register DT provider\n", clk_name);
> + goto free_clk;
> + }
> +
> + if (!data->has_rst)
> + return;
> +
> + reset_data = kzalloc(sizeof(*reset_data), GFP_KERNEL);
> + if (!reset_data)
> + goto free_of_clk;
> +
> + reset_data->reg = reg;
> + reset_data->offset = data->offset_rst;
> + reset_data->lock = &sun4i_a10_display_lock;
> + reset_data->rcdev.nr_resets = data->has_rst;
> + reset_data->rcdev.ops = &sun4i_a10_display_reset_ops;
> + reset_data->rcdev.of_node = node;
> +
> + if (data->has_rst == 1) {
> + reset_data->rcdev.of_reset_n_cells = 0;
> + reset_data->rcdev.of_xlate = &sun4i_a10_display_reset_xlate;
> + } else {
> + reset_data->rcdev.of_reset_n_cells = 1;
> + }
> +
> + if (reset_controller_register(&reset_data->rcdev)) {
> + pr_err("%s: Couldn't register the reset controller\n",
> +clk_name);
> + goto free_reset;
> + }
> +
> + return;
> +
> +free_reset:
> + kfree(reset_data);
> +free_of_clk:
> + of_clk_del_provider(node);
> +free_clk:
> + clk_unregister_composite(clk);
> +free_div:
> + if (data->has_div)
Do you need this check? div is NULL so I think no.
> + kfree(div);
> +free_gate:
> + kfree(gate);
> +free_mux:
> + kfree(mux);
> +unmap:
> + iounmap(reg);
> + of_address_to_resource(node, 0, &res);
> + release_mem_region(res.start, resource_size(&res));
> +}
> +
> +static struct sun4i_a10_display_clk_data sun4i_a10_tcon_ch0_data = {
const?
> + .has_rst= 2,
> + .parents= 4,
> + .offset_en = 31,
> + .offset_rst = 29,
> + .offset_mux = 24,
> + .width_mux = 2,
> +};
> +
> +static void __init sun4i_a10_tcon_ch0_setup(struct device_node *node)
> +{
> + sun4i_a10_display_init(node, &sun4i_a10_tcon_ch0_data);
> +}
> +CLK_OF_DECLARE(sun4i_a10_tcon_ch0, "allwinner,sun4i-a10-tcon-ch0-clk",
> +sun4i_a10_tcon_ch0_setup);
> +
> +static struct sun4i_a10_display_clk_data sun4i_a10_display_data = {
const?
> + .has_div= true,
> + .has_rst= 1,
> + .parents= 3,
> + .offset_en = 31,
> + .offset_rst = 30,
> + .offset_mux = 24,
> + .offset_div = 0,
> + .width_mux = 2,
> + .width_div = 4,
> +};
> +
> +static void __init sun4i_a10_display_setup(struct device_node *node)
> +{
> + sun4i_a10_display_init(node, &sun4i_a10_display_data);
> +}
> +CLK_OF_DECLARE(sun4i_a10_display, "allwinner,sun4i-a10-display-clk",
> +sun4i_a10_display_setup);
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v2 05/11] drm/i915: Allow mmio updates on all platforms, v2.
On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
> and unpinning.
I think the rename should be a separate patch.
Ander
> Use flip_queued_req to hold the wait request in the
> mmio case and allow the vblank interrupt to complete mmio work to
> have mmio flips run correctly on g4 and earlier.
>
> Changes since v1:
> - Add smp_mb__after_atomic() to __intel_pageflip_stall_check,
> to match the smp_mb__before_atomic() in pipe_update_end().
> - Check for cur != flip_queued_vblank in pageflip_stall_check.
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 4 +-
> drivers/gpu/drm/i915/intel_display.c | 271 +++---
> -
> drivers/gpu/drm/i915/intel_drv.h | 18 +--
> drivers/gpu/drm/i915/intel_sprite.c | 8 +-
> 4 files changed, 95 insertions(+), 206 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index c3b029e7bffd..5662cd5a1a9d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -574,10 +574,10 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
> for_each_intel_crtc(dev, crtc) {
> const char pipe = pipe_name(crtc->pipe);
> const char plane = plane_name(crtc->plane);
> - struct intel_unpin_work *work;
> + struct intel_flip_work *work;
>
> spin_lock_irq(&dev->event_lock);
> - work = crtc->unpin_work;
> + work = crtc->flip_work;
> if (work == NULL) {
> seq_printf(m, "No flip due on pipe %c (plane %c)\n",
> pipe, plane);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f1a895153e64..b614f118b973 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -48,6 +48,11 @@
> #include
> #include
>
> +static bool is_mmio_work(struct intel_flip_work *work)
> +{
> + return work->mmio_work.func;
> +}
> +
> /* Primary plane formats for gen <= 3 */
> static const uint32_t i8xx_primary_formats[] = {
> DRM_FORMAT_C8,
> @@ -3285,7 +3290,7 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc
> *crtc)
> return false;
>
> spin_lock_irq(&dev->event_lock);
> - pending = to_intel_crtc(crtc)->unpin_work != NULL;
> + pending = to_intel_crtc(crtc)->flip_work != NULL;
> spin_unlock_irq(&dev->event_lock);
>
> return pending;
> @@ -3864,7 +3869,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
> if (atomic_read(&crtc->unpin_work_count) == 0)
> continue;
>
> - if (crtc->unpin_work)
> + if (crtc->flip_work)
> intel_wait_for_vblank(dev, crtc->pipe);
>
> return true;
> @@ -3876,11 +3881,11 @@ bool intel_has_pending_fb_unpin(struct drm_device
> *dev)
> static void page_flip_completed(struct intel_crtc *intel_crtc)
> {
> struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> - struct intel_unpin_work *work = intel_crtc->unpin_work;
> + struct intel_flip_work *work = intel_crtc->flip_work;
>
> - /* ensure that the unpin work is consistent wrt ->pending. */
> + /* ensure that the flip work is consistent wrt ->pending. */
> smp_rmb();
> - intel_crtc->unpin_work = NULL;
> + intel_crtc->flip_work = NULL;
>
> if (work->event)
> drm_send_vblank_event(intel_crtc->base.dev,
> @@ -3890,7 +3895,7 @@ static void page_flip_completed(struct intel_crtc
> *intel_crtc)
> drm_crtc_vblank_put(&intel_crtc->base);
>
> wake_up_all(&dev_priv->pending_flip_queue);
> - queue_work(dev_priv->wq, &work->work);
> + queue_work(dev_priv->wq, &work->unpin_work);
>
> trace_i915_flip_complete(intel_crtc->plane,
>work->pending_flip_obj);
> @@ -3914,9 +3919,11 @@ static int intel_crtc_wait_for_pending_flips(struct
> drm_crtc *crtc)
>
> if (ret == 0) {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_flip_work *work;
>
> spin_lock_irq(&dev->event_lock);
> - if (intel_crtc->unpin_work) {
> + work = intel_crtc->flip_work;
> + if (work && !is_mmio_work(work)) {
> WARN_ONCE(1, "Removing stuck page flip\n");
> page_flip_completed(intel_crtc);
> }
> @@ -6308,7 +6315,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc
> *crtc)
> return;
>
> if (to_intel_plane_state(crtc->primary->state)->visible) {
> - WARN_ON(intel_crtc->unpin_work);
> + WARN_ON(intel_crtc->flip_work);
>
> intel_pre_disable_primary_noatom
[PATCH v3 05/19] dt-bindings: clk: sun5i: add DRAM gates compatible
On 03/23, Maxime Ripard wrote: > The Allwinner SoCs have a gate controller to gate the access to the DRAM > clock to the some devices that need to access the DRAM directly (mostly > display / image related IPs). > > Use a simple gates driver to support the one found in the A13 / R8 SoCs. > > Signed-off-by: Maxime Ripard > Acked-by: Chen-Yu Tsai > Acked-by: Rob Herring > --- Acked-by: Stephen Boyd -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v3 01/19] clk: composite: Add unregister function
On 03/23, Maxime Ripard wrote:
> The composite clock didn't have any unregistration function, which forced
> us to use clk_unregister directly on it.
>
> While it was already not great from an API point of view, it also meant
> that we were leaking the clk_composite structure allocated in
> clk_register_composite.
>
> Add a clk_unregister_composite function to fix this.
>
> Signed-off-by: Maxime Ripard
> ---
I'm currently attempting to change the way clks are registered so
that we don't return clk pointers from clk_register and have
users add OF clk providers that return clk_hw pointers instead of
clk pointers. Just a note, that this whole thing should be
deleted in the next cycle if I can convert everything!
> drivers/clk/clk-composite.c | 15 +++
> include/linux/clk-provider.h | 1 +
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> index 1f903e1f86a2..b0f3b84ebd13 100644
> --- a/drivers/clk/clk-composite.c
> +++ b/drivers/clk/clk-composite.c
> @@ -286,3 +286,18 @@ err:
> kfree(composite);
> return clk;
> }
> +
> +void clk_unregister_composite(struct clk *clk)
> +{
> + struct clk_composite *composite;
> + struct clk_hw *hw;
> +
> + hw = __clk_get_hw(clk);
> + if (!hw)
> + return;
> +
> + composite = to_clk_composite(hw);
> +
> + clk_unregister(clk);
> + kfree(composite);
> +}
EXPORT_SYMBOL_GPL?
Do I need to pick this up?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
[Bug 94933] [RV610] Freeze when turning on an external monitor after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=94933 --- Comment #10 from Nicola Mori --- Yes, disabling dpm the problem vanishes. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/88563d4c/attachment.html>
[PATCH v2 03/11] drm/i915: Remove intel_prepare_page_flip.
On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> Do it in 1 step instead, use atomic_read since INTEL_FLIP_COMPLETE
> is no longer useful.
What's the deal with "use atomic_read"? I couldn't find where that is different
from the old code.
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 3 --
> drivers/gpu/drm/i915/i915_irq.c | 18 ++-
> drivers/gpu/drm/i915/intel_display.c | 96 ++-
> -
> drivers/gpu/drm/i915/intel_drv.h | 2 -
> 4 files changed, 40 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index df8073a2ffbe..c3b029e7bffd 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -589,9 +589,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void
> *data)
> if (pending == INTEL_FLIP_INACTIVE) {
> seq_printf(m, "Flip ioctl preparing on pipe
> %c (plane %c)\n",
> pipe, plane);
> - } else if (pending >= INTEL_FLIP_COMPLETE) {
> - seq_printf(m, "Flip queued on pipe %c (plane
> %c)\n",
> -pipe, plane);
> } else {
> seq_printf(m, "Flip pending (waiting for
> vsync) on pipe %c (plane %c)\n",
> pipe, plane);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 679f08c944ef..5ed2f73a7ea9 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1707,10 +1707,8 @@ static void valleyview_pipestat_irq_handler(struct
> drm_device *dev, u32 iir)
> intel_pipe_handle_vblank(dev, pipe))
> intel_check_page_flip(dev, pipe);
>
> - if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV) {
> - intel_prepare_page_flip(dev, pipe);
> + if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
> intel_finish_page_flip(dev, pipe);
> - }
>
> if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
> i9xx_pipe_crc_irq_handler(dev, pipe);
> @@ -2109,10 +2107,8 @@ static void ilk_display_irq_handler(struct drm_device
> *dev, u32 de_iir)
> i9xx_pipe_crc_irq_handler(dev, pipe);
>
> /* plane/pipes map 1:1 on ilk+ */
> - if (de_iir & DE_PLANE_FLIP_DONE(pipe)) {
> - intel_prepare_page_flip(dev, pipe);
> + if (de_iir & DE_PLANE_FLIP_DONE(pipe))
> intel_finish_page_flip_plane(dev, pipe);
> - }
> }
>
> /* check event from PCH */
> @@ -2156,10 +2152,8 @@ static void ivb_display_irq_handler(struct drm_device
> *dev, u32 de_iir)
> intel_check_page_flip(dev, pipe);
>
> /* plane/pipes map 1:1 on ilk+ */
> - if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) {
> - intel_prepare_page_flip(dev, pipe);
> + if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
> intel_finish_page_flip_plane(dev, pipe);
> - }
> }
>
> /* check event from PCH */
> @@ -2363,10 +2357,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv,
> u32 master_ctl)
> else
> flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
>
> - if (flip_done) {
> - intel_prepare_page_flip(dev, pipe);
> + if (flip_done)
> intel_finish_page_flip_plane(dev, pipe);
> - }
>
> if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
> hsw_pipe_crc_irq_handler(dev, pipe);
> @@ -4024,7 +4016,6 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
> if (I915_READ16(ISR) & flip_pending)
> goto check_page_flip;
>
> - intel_prepare_page_flip(dev, plane);
> intel_finish_page_flip(dev, pipe);
> return true;
>
> @@ -4215,7 +4206,6 @@ static bool i915_handle_vblank(struct drm_device *dev,
> if (I915_READ(ISR) & flip_pending)
> goto check_page_flip;
>
> - intel_prepare_page_flip(dev, plane);
> intel_finish_page_flip(dev, pipe);
> return true;
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 618e034a7a5e..dc42335409b3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3101,7 +3101,6 @@ static void intel_complete_page_flips(struct drm_device
> *dev)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> enum plane plane = intel_crtc->plane;
>
> - intel_prepare_page_flip(dev, plane);
> intel_fin
[PATCH 15/15] drm/radeon/dp_mst: use connector ref counting
From: Dave Airlie
Use connector reference counting in radeon mst code.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/radeon/radeon_dp_mst.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c
b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index 43cffb5..4f1792a 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -242,6 +242,7 @@ radeon_dp_mst_connector_destroy(struct drm_connector
*connector)
drm_encoder_cleanup(&radeon_encoder->base);
kfree(radeon_encoder);
+
drm_connector_cleanup(connector);
kfree(radeon_connector);
}
@@ -313,11 +314,9 @@ static void radeon_dp_destroy_mst_connector(struct
drm_dp_mst_topology_mgr *mgr,
drm_modeset_lock_all(dev);
/* dpms off */
radeon_fb_remove_connector(rdev, connector);
-
- drm_connector_cleanup(connector);
drm_modeset_unlock_all(dev);
- kfree(connector);
+ drm_connector_unreference(connector);
DRM_DEBUG_KMS("\n");
}
--
2.5.5
[PATCH 14/15] drm/i915/mst: use reference counted connectors.
From: Dave Airlie
Don't just free the connector when we get the destroy callback.
Drop a reference to it, and set it's mst_port to NULL so
no more mst work is done on it.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/i915/intel_dp_mst.c | 46 ++---
drivers/gpu/drm/i915/intel_drv.h| 2 +-
2 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index a2bd698..2e730b6 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -113,7 +113,7 @@ static void intel_mst_disable_dp(struct intel_encoder
*encoder)
DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
- drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, intel_mst->port);
+ drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr,
intel_mst->connector->port);
ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
if (ret) {
@@ -138,10 +138,12 @@ static void intel_mst_post_disable_dp(struct
intel_encoder *encoder)
/* and this can also fail */
drm_dp_update_payload_part2(&intel_dp->mst_mgr);
- drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, intel_mst->port);
+ drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr,
intel_mst->connector->port);
intel_dp->active_mst_links--;
- intel_mst->port = NULL;
+
+ drm_connector_unreference(&intel_mst->connector->base);
+ intel_mst->connector = NULL;
if (intel_dp->active_mst_links == 0) {
intel_dig_port->base.post_disable(&intel_dig_port->base);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
@@ -181,7 +183,9 @@ static void intel_mst_pre_enable_dp(struct intel_encoder
*encoder)
found->encoder = encoder;
DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
- intel_mst->port = found->port;
+
+ intel_mst->connector = found;
+ drm_connector_reference(&intel_mst->connector->base);
if (intel_dp->active_mst_links == 0) {
intel_prepare_ddi_buffer(&intel_dig_port->base);
@@ -199,7 +203,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder
*encoder)
}
ret = drm_dp_mst_allocate_vcpi(&intel_dp->mst_mgr,
- intel_mst->port,
+ intel_mst->connector->port,
intel_crtc->config->pbn, &slots);
if (ret == false) {
DRM_ERROR("failed to allocate vcpi\n");
@@ -248,7 +252,7 @@ static bool intel_dp_mst_enc_get_hw_state(struct
intel_encoder *encoder,
{
struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
*pipe = intel_mst->pipe;
- if (intel_mst->port)
+ if (intel_mst->connector)
return true;
return false;
}
@@ -312,6 +316,11 @@ static int intel_dp_mst_get_ddc_modes(struct drm_connector
*connector)
struct edid *edid;
int ret;
+ if (!intel_connector->port || !intel_dp) {
+ ret = intel_connector_update_modes(connector, NULL);
+ return ret;
+ }
+
edid = drm_dp_mst_get_edid(connector, &intel_dp->mst_mgr,
intel_connector->port);
if (!edid)
return 0;
@@ -328,6 +337,8 @@ intel_dp_mst_detect(struct drm_connector *connector, bool
force)
struct intel_connector *intel_connector = to_intel_connector(connector);
struct intel_dp *intel_dp = intel_connector->mst_port;
+ if (!intel_connector->port || !intel_dp)
+ return connector_status_disconnected;
return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr,
intel_connector->port);
}
@@ -393,6 +404,8 @@ static struct drm_encoder
*intel_mst_atomic_best_encoder(struct drm_connector *c
struct intel_dp *intel_dp = intel_connector->mst_port;
struct intel_crtc *crtc = to_intel_crtc(state->crtc);
+ if (!intel_dp)
+ return NULL;
return &intel_dp->mst_encoders[crtc->pipe]->base.base;
}
@@ -400,6 +413,8 @@ static struct drm_encoder *intel_mst_best_encoder(struct
drm_connector *connecto
{
struct intel_connector *intel_connector = to_intel_connector(connector);
struct intel_dp *intel_dp = intel_connector->mst_port;
+ if (!intel_dp)
+ return NULL;
return &intel_dp->mst_encoders[0]->base.base;
}
@@ -506,29 +521,14 @@ static void intel_dp_destroy_mst_connector(struct
drm_dp_mst_topology_mgr *mgr,
struct intel_connector *intel_connector = to_intel_connector(connector);
struct drm_device *dev = connector->dev;
- /* need to nuke the connector */
- drm_modeset_lock_all(dev);
- if (connector->state->crtc) {
- struct drm_mode_set set;
- int ret;
-
- memset(&set, 0, sizeof(set));
- set.crtc = connector->state->crtc,
-
- ret = drm_atomic_
[PATCH 13/15] drm: take references to connectors used in a modeset.
From: Dave Airlie
As suggested by Daniel, if we are actively using the connector in a modeset
we don't want it to disappear from underneath us. This takes a reference
to the connector in the atomic paths when we are setting the state up,
and in the non-atomic paths when binding the encoder.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_atomic.c | 11 ++-
drivers/gpu/drm/drm_crtc_helper.c | 6 ++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 9d5e3c8..d899dac 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1159,7 +1159,7 @@ drm_atomic_set_crtc_for_connector(struct
drm_connector_state *conn_state,
struct drm_crtc *crtc)
{
struct drm_crtc_state *crtc_state;
-
+ bool had_crtc = conn_state->crtc ? true : false;
if (conn_state->crtc && conn_state->crtc != crtc) {
crtc_state =
drm_atomic_get_existing_crtc_state(conn_state->state,
conn_state->crtc);
@@ -1179,6 +1179,15 @@ drm_atomic_set_crtc_for_connector(struct
drm_connector_state *conn_state,
conn_state->crtc = crtc;
+ /* If we had no crtc then got one, add a reference,
+* if we had a crtc and are going to none, drop a reference,
+* otherwise just keep the reference we have.
+*/
+ if (!had_crtc && crtc)
+ drm_connector_reference(conn_state->connector);
+ else if (!crtc && had_crtc)
+drm_connector_unreference(conn_state->connector);
+
if (crtc)
DRM_DEBUG_ATOMIC("Link connector state %p to [CRTC:%d:%s]\n",
conn_state, crtc->base.id, crtc->name);
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index 79555d2..71b6c72 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -456,6 +456,9 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
* between them is henceforth no longer available.
*/
connector->dpms = DRM_MODE_DPMS_OFF;
+
+ /* we keep a reference while the encoder is bound */
+ drm_connector_unreference(connector);
}
}
@@ -635,9 +638,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
mode_changed = true;
/* If the encoder is reused for another connector, then
* the appropriate crtc will be set later.
+* take a reference only if we haven't had an encoder
before.
*/
if (connector->encoder)
connector->encoder->crtc = NULL;
+ else
+ drm_connector_reference(connector);
connector->encoder = new_encoder;
}
}
--
2.5.5
[PATCH 12/15] drm/modes: add connector reference counting.
From: Dave Airlie
This uses the previous changes to add reference counting to the
drm connector objects.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_atomic.c| 19 +--
drivers/gpu/drm/drm_crtc.c | 39 ++-
drivers/gpu/drm/drm_fb_helper.c | 38 ++
include/drm/drm_crtc.h | 13 -
4 files changed, 65 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8ee1db8..9d5e3c8 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -154,6 +154,7 @@ void drm_atomic_state_default_clear(struct drm_atomic_state
*state)
state->connector_states[i]);
state->connectors[i] = NULL;
state->connector_states[i] = NULL;
+ drm_connector_unreference(connector);
}
for (i = 0; i < config->num_crtc; i++) {
@@ -924,6 +925,7 @@ drm_atomic_get_connector_state(struct drm_atomic_state
*state,
if (!connector_state)
return ERR_PTR(-ENOMEM);
+ drm_connector_reference(connector);
state->connector_states[index] = connector_state;
state->connectors[index] = connector;
connector_state->state = state;
@@ -1614,12 +1616,19 @@ retry:
}
obj = drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_ANY);
- if (!obj || !obj->properties) {
+ if (!obj) {
+ ret = -ENOENT;
+ goto out;
+ }
+
+ if (!obj->properties) {
+ drm_mode_object_unreference(obj);
ret = -ENOENT;
goto out;
}
if (get_user(count_props, count_props_ptr + copied_objs)) {
+ drm_mode_object_unreference(obj);
ret = -EFAULT;
goto out;
}
@@ -1632,12 +1641,14 @@ retry:
struct drm_property *prop;
if (get_user(prop_id, props_ptr + copied_props)) {
+ drm_mode_object_unreference(obj);
ret = -EFAULT;
goto out;
}
prop = drm_property_find(dev, prop_id);
if (!prop) {
+ drm_mode_object_unreference(obj);
ret = -ENOENT;
goto out;
}
@@ -1645,13 +1656,16 @@ retry:
if (copy_from_user(&prop_value,
prop_values_ptr + copied_props,
sizeof(prop_value))) {
+ drm_mode_object_unreference(obj);
ret = -EFAULT;
goto out;
}
ret = atomic_set_prop(state, obj, prop, prop_value);
- if (ret)
+ if (ret) {
+ drm_mode_object_unreference(obj);
goto out;
+ }
copied_props++;
}
@@ -1662,6 +1676,7 @@ retry:
plane_mask |= (1 << drm_plane_index(plane));
plane->old_fb = plane->fb;
}
+ drm_mode_object_unreference(obj);
}
if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f6bf828..90bc597 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -861,6 +861,16 @@ static void drm_connector_get_cmdline_mode(struct
drm_connector *connector)
mode->interlace ? " interlaced" : "");
}
+static void drm_connector_free(struct kref *kref)
+{
+ struct drm_connector *connector =
+ container_of(kref, struct drm_connector, base.refcount);
+ struct drm_device *dev = connector->dev;
+
+ drm_mode_object_unregister(dev, &connector->base);
+ connector->funcs->destroy(connector);
+}
+
/**
* drm_connector_init - Init a preallocated connector
* @dev: DRM device
@@ -886,7 +896,7 @@ int drm_connector_init(struct drm_device *dev,
drm_modeset_lock_all(dev);
- ret = drm_mode_object_get_reg(dev, &connector->base,
DRM_MODE_OBJECT_CONNECTOR, false, NULL);
+ ret = drm_mode_object_get_reg(dev, &connector->base,
DRM_MODE_OBJECT_CONNECTOR, false, drm_connector_free);
if (ret)
goto out_unlock;
@@ -2106,7 +2116,7 @@ int drm_mode_getconnector(struct drm_device *dev, void
*data,
mutex_lock(&dev->mode_config.mutex);
- connector = drm_connec
[PATCH 11/15] drm/modes: stop handling framebuffer special
From: Dave Airlie
Since ref counting is in the object now we can just call the
normal interfaces.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_crtc.c | 17 ++---
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 46f32f2..f6bf828 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4810,19 +4810,7 @@ bool drm_property_change_valid_get(struct drm_property
*property,
if (value == 0)
return true;
- /* handle refcnt'd objects specially: */
- if (property->values[0] == DRM_MODE_OBJECT_FB) {
- struct drm_framebuffer *fb;
- fb = drm_framebuffer_lookup(property->dev, value);
- if (fb) {
- *ref = &fb->base;
- return true;
- } else {
- return false;
- }
- } else {
- return _object_find(property->dev, value,
property->values[0]) != NULL;
- }
+ return _object_find(property->dev, value, property->values[0])
!= NULL;
}
for (i = 0; i < property->num_values; i++)
@@ -4838,8 +4826,7 @@ void drm_property_change_valid_put(struct drm_property
*property,
return;
if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
- if (property->values[0] == DRM_MODE_OBJECT_FB)
- drm_framebuffer_unreference(obj_to_fb(ref));
+ drm_mode_object_unreference(ref);
} else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
drm_property_unreference_blob(obj_to_blob(ref));
}
--
2.5.5
[PATCH 10/15] drm/modes: reduce fb_lock to just protecting lists
From: Dave Airlie This reduces the fb_lock to just protecting the num_fb/fb_list. I'd like to have some discussion on if this opens up any race conditions. Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_crtc.c | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e47c4a2..46f32f2 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -433,9 +433,7 @@ static void drm_framebuffer_free(struct kref *kref) * The lookup idr holds a weak reference, which has not necessarily been * removed at this point. Check for that. */ - mutex_lock(&dev->mode_config.fb_lock); drm_mode_object_unregister(dev, &fb->base); - mutex_unlock(&dev->mode_config.fb_lock); fb->funcs->destroy(fb); } @@ -475,9 +473,9 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb, mutex_lock(&dev->mode_config.fb_lock); dev->mode_config.num_fb++; list_add(&fb->head, &dev->mode_config.fb_list); + mutex_unlock(&dev->mode_config.fb_lock); drm_mode_object_register(dev, &fb->base); - mutex_unlock(&dev->mode_config.fb_lock); out: return ret; } @@ -498,12 +496,9 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev, struct drm_mode_object *obj; struct drm_framebuffer *fb = NULL; - mutex_lock(&dev->mode_config.fb_lock); obj = _object_find(dev, id, DRM_MODE_OBJECT_FB); if (obj) fb = obj_to_fb(obj); - mutex_unlock(&dev->mode_config.fb_lock); - return fb; } EXPORT_SYMBOL(drm_framebuffer_lookup); @@ -526,10 +521,8 @@ void drm_framebuffer_unregister_private(struct drm_framebuffer *fb) dev = fb->dev; - mutex_lock(&dev->mode_config.fb_lock); /* Mark fb as reaped and drop idr ref. */ drm_mode_object_unregister(dev, &fb->base); - mutex_unlock(&dev->mode_config.fb_lock); } EXPORT_SYMBOL(drm_framebuffer_unregister_private); -- 2.5.5
[PATCH 09/15] drm/modes: move reference taking into object lookup.
From: Dave Airlie
When we lookup an ref counted object we now take a proper reference
using kref_get_unless_zero.
Framebuffer lookup no longer needs do this itself.
Convert rmfb to using framebuffer lookup and deal with the fact
it now gets an extra reference that we have to cleanup. This should
mean we can avoid holding fb_lock across rmfb. (if I'm wrong let me
know).
We also now only hold the fbs_lock around the list manipulation.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_crtc.c | 37 -
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 21cb998..e47c4a2 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -364,6 +364,11 @@ static struct drm_mode_object *_object_find(struct
drm_device *dev,
if (obj &&
obj->type == DRM_MODE_OBJECT_BLOB)
obj = NULL;
+
+ if (obj && obj->free_cb) {
+ if (!kref_get_unless_zero(&obj->refcount))
+ obj = NULL;
+ }
mutex_unlock(&dev->mode_config.idr_mutex);
return obj;
@@ -495,11 +500,8 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct
drm_device *dev,
mutex_lock(&dev->mode_config.fb_lock);
obj = _object_find(dev, id, DRM_MODE_OBJECT_FB);
- if (obj) {
+ if (obj)
fb = obj_to_fb(obj);
- if (!kref_get_unless_zero(&fb->base.refcount))
- fb = NULL;
- }
mutex_unlock(&dev->mode_config.fb_lock);
return fb;
@@ -3434,37 +3436,38 @@ int drm_mode_rmfb(struct drm_device *dev,
{
struct drm_framebuffer *fb = NULL;
struct drm_framebuffer *fbl = NULL;
- struct drm_mode_object *obj;
uint32_t *id = data;
int found = 0;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL;
+ fb = drm_framebuffer_lookup(dev, *id);
+ if (!fb)
+ return -ENOENT;
+
mutex_lock(&file_priv->fbs_lock);
- mutex_lock(&dev->mode_config.fb_lock);
- obj = _object_find(dev, *id, DRM_MODE_OBJECT_FB);
- if (!obj)
- goto fail_lookup;
- fb = obj_to_fb(obj);
list_for_each_entry(fbl, &file_priv->fbs, filp_head)
if (fb == fbl)
found = 1;
- if (!found)
- goto fail_lookup;
+ if (!found) {
+ mutex_unlock(&file_priv->fbs_lock);
+ goto fail_unref;
+ }
list_del_init(&fb->filp_head);
- mutex_unlock(&dev->mode_config.fb_lock);
mutex_unlock(&file_priv->fbs_lock);
+ /* we now own the reference that was stored in the fbs list */
drm_framebuffer_unreference(fb);
- return 0;
+ /* drop the reference we picked up in framebuffer lookup */
+ drm_framebuffer_unreference(fb);
-fail_lookup:
- mutex_unlock(&dev->mode_config.fb_lock);
- mutex_unlock(&file_priv->fbs_lock);
+ return 0;
+fail_unref:
+ drm_framebuffer_unreference(fb);
return -ENOENT;
}
--
2.5.5
[PATCH 08/15] drm/mode: reduce lock hold in addfb2
From: Dave Airlie
No need to hold the lock while assigning the variable.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_crtc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 1863879..21cb998 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3405,11 +3405,11 @@ int drm_mode_addfb2(struct drm_device *dev,
if (IS_ERR(fb))
return PTR_ERR(fb);
- /* Transfer ownership to the filp for reaping on close */
-
DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id);
- mutex_lock(&file_priv->fbs_lock);
r->fb_id = fb->base.id;
+
+ /* Transfer ownership to the filp for reaping on close */
+ mutex_lock(&file_priv->fbs_lock);
list_add(&fb->filp_head, &file_priv->fbs);
mutex_unlock(&file_priv->fbs_lock);
--
2.5.5
[PATCH 07/15] drm/mode: reduce scope of fb_lock in framebuffer init
From: Dave Airlie
We don't need to hold the fb lock around the initialisation,
only around the list manipulaton.
So do the lock hold only around the register for now.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_crtc.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 0d75517..1863879 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -458,21 +458,22 @@ int drm_framebuffer_init(struct drm_device *dev, struct
drm_framebuffer *fb,
{
int ret;
- mutex_lock(&dev->mode_config.fb_lock);
INIT_LIST_HEAD(&fb->filp_head);
fb->dev = dev;
fb->funcs = funcs;
ret = drm_mode_object_get_reg(dev, &fb->base, DRM_MODE_OBJECT_FB,
- true, drm_framebuffer_free);
+ false, drm_framebuffer_free);
if (ret)
goto out;
+ mutex_lock(&dev->mode_config.fb_lock);
dev->mode_config.num_fb++;
list_add(&fb->head, &dev->mode_config.fb_list);
-out:
- mutex_unlock(&dev->mode_config.fb_lock);
+ drm_mode_object_register(dev, &fb->base);
+ mutex_unlock(&dev->mode_config.fb_lock);
+out:
return ret;
}
EXPORT_SYMBOL(drm_framebuffer_init);
--
2.5.5
[PATCH 06/15] drm/mode: use _object_find to find framebuffers.
From: Dave Airlie
No point have this code dupliated at this point, use the
_object_find code instead now.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_crtc.c | 35 ++-
1 file changed, 10 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 75a45e9..0d75517 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -362,8 +362,7 @@ static struct drm_mode_object *_object_find(struct
drm_device *dev,
obj = NULL;
/* don't leak out unref'd fb's */
if (obj &&
- (obj->type == DRM_MODE_OBJECT_FB ||
-obj->type == DRM_MODE_OBJECT_BLOB))
+ obj->type == DRM_MODE_OBJECT_BLOB)
obj = NULL;
mutex_unlock(&dev->mode_config.idr_mutex);
@@ -478,23 +477,6 @@ out:
}
EXPORT_SYMBOL(drm_framebuffer_init);
-static struct drm_framebuffer *__drm_framebuffer_lookup(struct drm_device *dev,
- uint32_t id)
-{
- struct drm_mode_object *obj = NULL;
- struct drm_framebuffer *fb;
-
- mutex_lock(&dev->mode_config.idr_mutex);
- obj = idr_find(&dev->mode_config.crtc_idr, id);
- if (!obj || (obj->type != DRM_MODE_OBJECT_FB) || (obj->id != id))
- fb = NULL;
- else
- fb = obj_to_fb(obj);
- mutex_unlock(&dev->mode_config.idr_mutex);
-
- return fb;
-}
-
/**
* drm_framebuffer_lookup - look up a drm framebuffer and grab a reference
* @dev: drm device
@@ -507,11 +489,13 @@ static struct drm_framebuffer
*__drm_framebuffer_lookup(struct drm_device *dev,
struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
uint32_t id)
{
- struct drm_framebuffer *fb;
+ struct drm_mode_object *obj;
+ struct drm_framebuffer *fb = NULL;
mutex_lock(&dev->mode_config.fb_lock);
- fb = __drm_framebuffer_lookup(dev, id);
- if (fb) {
+ obj = _object_find(dev, id, DRM_MODE_OBJECT_FB);
+ if (obj) {
+ fb = obj_to_fb(obj);
if (!kref_get_unless_zero(&fb->base.refcount))
fb = NULL;
}
@@ -3449,6 +3433,7 @@ int drm_mode_rmfb(struct drm_device *dev,
{
struct drm_framebuffer *fb = NULL;
struct drm_framebuffer *fbl = NULL;
+ struct drm_mode_object *obj;
uint32_t *id = data;
int found = 0;
@@ -3457,10 +3442,10 @@ int drm_mode_rmfb(struct drm_device *dev,
mutex_lock(&file_priv->fbs_lock);
mutex_lock(&dev->mode_config.fb_lock);
- fb = __drm_framebuffer_lookup(dev, *id);
- if (!fb)
+ obj = _object_find(dev, *id, DRM_MODE_OBJECT_FB);
+ if (!obj)
goto fail_lookup;
-
+ fb = obj_to_fb(obj);
list_for_each_entry(fbl, &file_priv->fbs, filp_head)
if (fb == fbl)
found = 1;
--
2.5.5
[PATCH 05/15] drm/mode: move framebuffer reference into object.
From: Dave Airlie
This is the initial code to add references to some mode objects.
In the future we need to start reference counting connectors so
firstly I want to reorganise the code so the framebuffer ref counting
uses the same paths.
This patch shouldn't change any functionality, just moves the kref.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_crtc.c | 72 --
include/drm/drm_crtc.h | 20 ++---
2 files changed, 54 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 8616737..75a45e9 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -275,7 +275,8 @@ EXPORT_SYMBOL(drm_get_format_name);
static int drm_mode_object_get_reg(struct drm_device *dev,
struct drm_mode_object *obj,
uint32_t obj_type,
- bool register_obj)
+ bool register_obj,
+ void (*obj_free_cb)(struct kref *kref))
{
int ret;
@@ -288,6 +289,10 @@ static int drm_mode_object_get_reg(struct drm_device *dev,
*/
obj->id = ret;
obj->type = obj_type;
+ if (obj_free_cb) {
+ obj->free_cb = obj_free_cb;
+ kref_init(&obj->refcount);
+ }
}
mutex_unlock(&dev->mode_config.idr_mutex);
@@ -311,7 +316,7 @@ static int drm_mode_object_get_reg(struct drm_device *dev,
int drm_mode_object_get(struct drm_device *dev,
struct drm_mode_object *obj, uint32_t obj_type)
{
- return drm_mode_object_get_reg(dev, obj, obj_type, true);
+ return drm_mode_object_get_reg(dev, obj, obj_type, true, NULL);
}
static void drm_mode_object_register(struct drm_device *dev,
@@ -389,10 +394,35 @@ struct drm_mode_object *drm_mode_object_find(struct
drm_device *dev,
}
EXPORT_SYMBOL(drm_mode_object_find);
+void drm_mode_object_unreference(struct drm_mode_object *obj)
+{
+ if (obj->free_cb) {
+ DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id,
atomic_read(&obj->refcount.refcount));
+ kref_put(&obj->refcount, obj->free_cb);
+ }
+}
+EXPORT_SYMBOL(drm_mode_object_unreference);
+
+/**
+ * drm_mode_object_reference - incr the fb refcnt
+ * @obj: mode_object
+ *
+ * This function operates only on refcounted objects.
+ * This functions increments the object's refcount.
+ */
+void drm_mode_object_reference(struct drm_mode_object *obj)
+{
+ if (obj->free_cb) {
+ DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id,
atomic_read(&obj->refcount.refcount));
+ kref_get(&obj->refcount);
+ }
+}
+EXPORT_SYMBOL(drm_mode_object_reference);
+
static void drm_framebuffer_free(struct kref *kref)
{
struct drm_framebuffer *fb =
- container_of(kref, struct drm_framebuffer, refcount);
+ container_of(kref, struct drm_framebuffer,
base.refcount);
struct drm_device *dev = fb->dev;
/*
@@ -430,12 +460,12 @@ int drm_framebuffer_init(struct drm_device *dev, struct
drm_framebuffer *fb,
int ret;
mutex_lock(&dev->mode_config.fb_lock);
- kref_init(&fb->refcount);
INIT_LIST_HEAD(&fb->filp_head);
fb->dev = dev;
fb->funcs = funcs;
- ret = drm_mode_object_get(dev, &fb->base, DRM_MODE_OBJECT_FB);
+ ret = drm_mode_object_get_reg(dev, &fb->base, DRM_MODE_OBJECT_FB,
+ true, drm_framebuffer_free);
if (ret)
goto out;
@@ -482,7 +512,7 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct
drm_device *dev,
mutex_lock(&dev->mode_config.fb_lock);
fb = __drm_framebuffer_lookup(dev, id);
if (fb) {
- if (!kref_get_unless_zero(&fb->refcount))
+ if (!kref_get_unless_zero(&fb->base.refcount))
fb = NULL;
}
mutex_unlock(&dev->mode_config.fb_lock);
@@ -492,32 +522,6 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct
drm_device *dev,
EXPORT_SYMBOL(drm_framebuffer_lookup);
/**
- * drm_framebuffer_unreference - unref a framebuffer
- * @fb: framebuffer to unref
- *
- * This functions decrements the fb's refcount and frees it if it drops to
zero.
- */
-void drm_framebuffer_unreference(struct drm_framebuffer *fb)
-{
- DRM_DEBUG("%p: FB ID: %d (%d)\n", fb, fb->base.id,
atomic_read(&fb->refcount.refcount));
- kref_put(&fb->refcount, drm_framebuffer_free);
-}
-EXPORT_SYMBOL(drm_framebuffer_unreference);
-
-/**
- * drm_framebuffer_reference - incr the fb refcnt
- * @fb: framebuffer
- *
- * This functions increments the fb's refcount.
- */
-void drm_framebuffer_reference(struct drm_framebuffer *fb)
-{
- DRM_DEBUG("%p: FB ID: %d (%d)\n", fb, fb->base.id,
atomic_read(&fb->refcount.refcount));
-
[PATCH 04/15] drm/mode: introduce wrapper to read framebuffer refcount.
From: Dave Airlie
Avoids drivers knowing where the kref is stored.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_crtc.c | 2 +-
drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
drivers/gpu/drm/msm/msm_fb.c| 2 +-
drivers/gpu/drm/tegra/drm.c | 2 +-
include/drm/drm_crtc.h | 5 +
5 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 0ad1a92..8616737 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -612,7 +612,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
* in-use fb with fb-id == 0. Userspace is allowed to shoot its own foot
* in this manner.
*/
- if (atomic_read(&fb->refcount.refcount) > 1) {
+ if (drm_framebuffer_read_refcount(fb) > 1) {
drm_modeset_lock_all(dev);
/* remove from any CRTC */
drm_for_each_crtc(crtc, dev) {
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index a0f1bd7..20d9300 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1908,7 +1908,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m,
void *data)
fbdev_fb->base.depth,
fbdev_fb->base.bits_per_pixel,
fbdev_fb->base.modifier[0],
- atomic_read(&fbdev_fb->base.refcount.refcount));
+ drm_framebuffer_read_refcount(&fbdev_fb->base));
describe_obj(m, fbdev_fb->obj);
seq_putc(m, '\n');
}
@@ -1926,7 +1926,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m,
void *data)
fb->base.depth,
fb->base.bits_per_pixel,
fb->base.modifier[0],
- atomic_read(&fb->base.refcount.refcount));
+ drm_framebuffer_read_refcount(&fb->base));
describe_obj(m, fb->obj);
seq_putc(m, '\n');
}
diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c
index a474d6c..17e0c9e 100644
--- a/drivers/gpu/drm/msm/msm_fb.c
+++ b/drivers/gpu/drm/msm/msm_fb.c
@@ -77,7 +77,7 @@ void msm_framebuffer_describe(struct drm_framebuffer *fb,
struct seq_file *m)
seq_printf(m, "fb: %dx%d@%4.4s (%2d, ID:%d)\n",
fb->width, fb->height, (char *)&fb->pixel_format,
- fb->refcount.refcount.counter, fb->base.id);
+ drm_framebuffer_read_refcount(fb), fb->base.id);
for (i = 0; i < n; i++) {
seq_printf(m, " %d: offset=%d pitch=%d, obj: ",
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 8e6b18c..2be88eb 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -878,7 +878,7 @@ static int tegra_debugfs_framebuffers(struct seq_file *s,
void *data)
seq_printf(s, "%3d: user size: %d x %d, depth %d, %d bpp,
refcount %d\n",
fb->base.id, fb->width, fb->height, fb->depth,
fb->bits_per_pixel,
- atomic_read(&fb->refcount.refcount));
+ drm_framebuffer_read_refcount(fb));
}
mutex_unlock(&drm->mode_config.fb_lock);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index e0170bf..99a12f0 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -2608,6 +2608,11 @@ static inline uint32_t drm_color_lut_extract(uint32_t
user_input,
return clamp_val(val, 0, max);
}
+static inline uint32_t drm_framebuffer_read_refcount(struct drm_framebuffer
*fb)
+{
+ return atomic_read(&fb->refcount.refcount);
+}
+
/* Plane list iterator for legacy (overlay only) planes. */
#define drm_for_each_legacy_plane(plane, dev) \
list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
--
2.5.5
[PATCH 03/15] drm/modes: drop __drm_framebuffer_unregister.
From: Dave Airlie
Just use the generic function.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_crtc.c | 16 ++--
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e69aac4..0ad1a92 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -389,15 +389,6 @@ struct drm_mode_object *drm_mode_object_find(struct
drm_device *dev,
}
EXPORT_SYMBOL(drm_mode_object_find);
-/* dev->mode_config.fb_lock must be held! */
-static void __drm_framebuffer_unregister(struct drm_device *dev,
-struct drm_framebuffer *fb)
-{
- drm_mode_object_put(dev, &fb->base);
-
- fb->base.id = 0;
-}
-
static void drm_framebuffer_free(struct kref *kref)
{
struct drm_framebuffer *fb =
@@ -409,10 +400,7 @@ static void drm_framebuffer_free(struct kref *kref)
* removed at this point. Check for that.
*/
mutex_lock(&dev->mode_config.fb_lock);
- if (fb->base.id) {
- /* Mark fb as reaped and drop idr ref. */
- __drm_framebuffer_unregister(dev, fb);
- }
+ drm_mode_object_unregister(dev, &fb->base);
mutex_unlock(&dev->mode_config.fb_lock);
fb->funcs->destroy(fb);
@@ -549,7 +537,7 @@ void drm_framebuffer_unregister_private(struct
drm_framebuffer *fb)
mutex_lock(&dev->mode_config.fb_lock);
/* Mark fb as reaped and drop idr ref. */
- __drm_framebuffer_unregister(dev, fb);
+ drm_mode_object_unregister(dev, &fb->base);
mutex_unlock(&dev->mode_config.fb_lock);
}
EXPORT_SYMBOL(drm_framebuffer_unregister_private);
--
2.5.5
[PATCH 02/15] drm/mode: move framebuffer_free up above framebuffer_init
From: Dave Airlie
A later patch will use it in framebuffer_init, and I want
to keep the diff cleaner.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_crtc.c | 58 +++---
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 21cb8dc..e69aac4 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -389,6 +389,35 @@ struct drm_mode_object *drm_mode_object_find(struct
drm_device *dev,
}
EXPORT_SYMBOL(drm_mode_object_find);
+/* dev->mode_config.fb_lock must be held! */
+static void __drm_framebuffer_unregister(struct drm_device *dev,
+struct drm_framebuffer *fb)
+{
+ drm_mode_object_put(dev, &fb->base);
+
+ fb->base.id = 0;
+}
+
+static void drm_framebuffer_free(struct kref *kref)
+{
+ struct drm_framebuffer *fb =
+ container_of(kref, struct drm_framebuffer, refcount);
+ struct drm_device *dev = fb->dev;
+
+ /*
+* The lookup idr holds a weak reference, which has not necessarily been
+* removed at this point. Check for that.
+*/
+ mutex_lock(&dev->mode_config.fb_lock);
+ if (fb->base.id) {
+ /* Mark fb as reaped and drop idr ref. */
+ __drm_framebuffer_unregister(dev, fb);
+ }
+ mutex_unlock(&dev->mode_config.fb_lock);
+
+ fb->funcs->destroy(fb);
+}
+
/**
* drm_framebuffer_init - initialize a framebuffer
* @dev: DRM device
@@ -431,35 +460,6 @@ out:
}
EXPORT_SYMBOL(drm_framebuffer_init);
-/* dev->mode_config.fb_lock must be held! */
-static void __drm_framebuffer_unregister(struct drm_device *dev,
-struct drm_framebuffer *fb)
-{
- drm_mode_object_put(dev, &fb->base);
-
- fb->base.id = 0;
-}
-
-static void drm_framebuffer_free(struct kref *kref)
-{
- struct drm_framebuffer *fb =
- container_of(kref, struct drm_framebuffer, refcount);
- struct drm_device *dev = fb->dev;
-
- /*
-* The lookup idr holds a weak reference, which has not necessarily been
-* removed at this point. Check for that.
-*/
- mutex_lock(&dev->mode_config.fb_lock);
- if (fb->base.id) {
- /* Mark fb as reaped and drop idr ref. */
- __drm_framebuffer_unregister(dev, fb);
- }
- mutex_unlock(&dev->mode_config.fb_lock);
-
- fb->funcs->destroy(fb);
-}
-
static struct drm_framebuffer *__drm_framebuffer_lookup(struct drm_device *dev,
uint32_t id)
{
--
2.5.5
[PATCH 01/15] drm/mode: rework drm_mode_object_put to drm_mode_object_unregister.
From: Dave Airlie
This changes the code to handle being called multiple times without
side effects. The new names seems more suitable for what it does.
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/drm_crtc.c | 37 +
drivers/gpu/drm/drm_crtc_internal.h | 4 ++--
drivers/gpu/drm/drm_modes.c | 2 +-
3 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e08f962..21cb8dc 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -323,19 +323,24 @@ static void drm_mode_object_register(struct drm_device
*dev,
}
/**
- * drm_mode_object_put - free a modeset identifer
+ * drm_mode_object_unregister - free a modeset identifer
* @dev: DRM device
* @object: object to free
*
- * Free @id from @dev's unique identifier pool. Note that despite the _get
- * postfix modeset identifiers are _not_ reference counted. Hence don't use
this
+ * Free @id from @dev's unique identifier pool.
+ * This function can be called multiple times, and guards against
+ * multiple removals.
+ * These modeset identifiers are _not_ reference counted. Hence don't use this
* for reference counted modeset objects like framebuffers.
*/
-void drm_mode_object_put(struct drm_device *dev,
+void drm_mode_object_unregister(struct drm_device *dev,
struct drm_mode_object *object)
{
mutex_lock(&dev->mode_config.idr_mutex);
- idr_remove(&dev->mode_config.crtc_idr, object->id);
+ if (object->id) {
+ idr_remove(&dev->mode_config.crtc_idr, object->id);
+ object->id = 0;
+ }
mutex_unlock(&dev->mode_config.idr_mutex);
}
@@ -705,7 +710,7 @@ int drm_crtc_init_with_planes(struct drm_device *dev,
struct drm_crtc *crtc,
drm_num_crtcs(dev));
}
if (!crtc->name) {
- drm_mode_object_put(dev, &crtc->base);
+ drm_mode_object_unregister(dev, &crtc->base);
return -ENOMEM;
}
@@ -747,7 +752,7 @@ void drm_crtc_cleanup(struct drm_crtc *crtc)
drm_modeset_lock_fini(&crtc->mutex);
- drm_mode_object_put(dev, &crtc->base);
+ drm_mode_object_unregister(dev, &crtc->base);
list_del(&crtc->head);
dev->mode_config.num_crtc--;
@@ -972,7 +977,7 @@ out_put_id:
ida_remove(&config->connector_ida, connector->connector_id);
out_put:
if (ret)
- drm_mode_object_put(dev, &connector->base);
+ drm_mode_object_unregister(dev, &connector->base);
out_unlock:
drm_modeset_unlock_all(dev);
@@ -1010,7 +1015,7 @@ void drm_connector_cleanup(struct drm_connector
*connector)
connector->connector_id);
kfree(connector->display_info.bus_formats);
- drm_mode_object_put(dev, &connector->base);
+ drm_mode_object_unregister(dev, &connector->base);
kfree(connector->name);
connector->name = NULL;
list_del(&connector->head);
@@ -1138,7 +1143,7 @@ int drm_encoder_init(struct drm_device *dev,
out_put:
if (ret)
- drm_mode_object_put(dev, &encoder->base);
+ drm_mode_object_unregister(dev, &encoder->base);
out_unlock:
drm_modeset_unlock_all(dev);
@@ -1181,7 +1186,7 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
struct drm_device *dev = encoder->dev;
drm_modeset_lock_all(dev);
- drm_mode_object_put(dev, &encoder->base);
+ drm_mode_object_unregister(dev, &encoder->base);
kfree(encoder->name);
list_del(&encoder->head);
dev->mode_config.num_encoder--;
@@ -1242,7 +1247,7 @@ int drm_universal_plane_init(struct drm_device *dev,
struct drm_plane *plane,
GFP_KERNEL);
if (!plane->format_types) {
DRM_DEBUG_KMS("out of memory when allocating plane\n");
- drm_mode_object_put(dev, &plane->base);
+ drm_mode_object_unregister(dev, &plane->base);
return -ENOMEM;
}
@@ -1258,7 +1263,7 @@ int drm_universal_plane_init(struct drm_device *dev,
struct drm_plane *plane,
}
if (!plane->name) {
kfree(plane->format_types);
- drm_mode_object_put(dev, &plane->base);
+ drm_mode_object_unregister(dev, &plane->base);
return -ENOMEM;
}
@@ -1338,7 +1343,7 @@ void drm_plane_cleanup(struct drm_plane *plane)
drm_modeset_lock_all(dev);
kfree(plane->format_types);
- drm_mode_object_put(dev, &plane->base);
+ drm_mode_object_unregister(dev, &plane->base);
BUG_ON(list_empty(&plane->head));
@@ -4029,7 +4034,7 @@ void drm_property_destroy(struct drm_device *dev, struct
drm_property *property)
if (property->num_values)
kfree(property->values);
- drm_mode_object_put(dev,
drm reference counter connectors and fix lifetimes
I've been trolled since I did MST that I really didn't do a good job on the connector lifetimes, so I finally felt guilty and had time to try and fix this up. This is a set of patches to handle connector lifetimes so that the connectors don't go away in the middle of us doing something. I've done some testing on this with kms_flip on my haswell laptop while undocking etc. Daniel has been pestering me to finish it off, so I've cleaned up the last few things in the mst patches at least for i915 and decided to send it out. Dave.
[Bug 94951] [BXT] Not all primary planes and sprites are supported
https://bugs.freedesktop.org/show_bug.cgi?id=94951 --- Comment #1 from Jairo Miramontes --- Created attachment 122971 --> https://bugs.freedesktop.org/attachment.cgi?id=122971&action=edit output of the "modetest" execution -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/8797bd61/attachment-0001.html>
[Bug 94951] [BXT] Not all primary planes and sprites are supported
https://bugs.freedesktop.org/show_bug.cgi?id=94951 Bug ID: 94951 Summary: [BXT] Not all primary planes and sprites are supported Product: DRI Version: DRI git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: libdrm Assignee: dri-devel at lists.freedesktop.org Reporter: jairo.daniel.miramontes.caton at intel.com Created attachment 122970 --> https://bugs.freedesktop.org/attachment.cgi?id=122970&action=edit dmesg.log While executing APL features on beta testing we found the following issue. The following test is failing: "Check primary plane and sprite planes supported" Description: "Support common YUV and RGB formats in sprite and primary planes" Steps to reproduce: 1. clone drm sources from git://anongit.freedesktop.org/mesa/drm 2. Build drm with ./autogen.sh & make 3. go to drm/tests/modetest 4. Execute modetest and look at the list of plane formats: ./modetest Expected result: You should see the following formats: RGB565, XRGB, XBGR, ARGB, ABGR, , XBGR2101010, YUYV, YVYU, UYVY, VYUY, RGB565, ABGR, ARGB, XBGR, XRGB, YUYV, YVYU, UYVY, VYUY, Actual result: formats: RG16 AB24 AR24 XB24 XR24 YUYV YVYU UYVY VYUY Configuration: ++ Platform: BXT-P ++ Motherboard model : Broxton P ++ Motherboard type: NOTEBOOK Hand Held ++ Motherboard manufacturer: Intel Corp. ++ CPU family : Other ++ CPU information : 06/5c ++ GPU Card: Intel Corporation Device 5a84 (rev 03) (prog-if 00 [VGA controller]) ++ Memory ram : 8 GB ++ Maximum memory ram allowed : 16 GB ++ Display resolution : ++ CPU's number: 4 ++ Hard drive capacity : 120 GB ++ Kernel version : 4.6.0-rc3-nightly+ ++ Linux distribution : Ubuntu 15.10 ++ Architecture: 64-bit ++ xf86-video-intel version: 2.99.917 ++ Xorg-Xserver version: 1.17.2 ++ DRM version : 2.4.64 ++ Cairo version : 1.14.2 ++ Intel GPU Tools version : Tag [intel-gpu-tools-1.14-133-gc89e8db] / Commit [c89e8db] ++ Kernel driver in use: i915 ++ Hardware acceleration : ++ Bios revision : 131.10 ++ KSC revision: 1.12 Find attached the output of the test and dmesg.log -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/2a67cb1c/attachment.html>
[PATCH] drm/dp/mst: Restore primary hub guid on resume
On Fri, Apr 15, 2016 at 08:41:30AM -0400, Harry Wentland wrote:
> Patch makes sense to me. It looks like when the receiver detects an upstream
> disconnect it will reset its internal state, at least somewhat. We've seen
> that happen when system enters S3 and GPU loses power.
>
> Reviewed-by: Harry Wentland
Applied to drm-misc, thanks for patch&review.
-Daniel
>
> Harry
>
> On 2016-04-13 04:50 PM, Lyude wrote:
> >Some hubs are forgetful, and end up forgetting whatever GUID we set
> >previously after we do a suspend/resume cycle. This can lead to
> >hotplugging breaking (along with probably other things) since the hub
> >will start sending connection notifications with the wrong GUID. As
> >such, we need to check on resume whether or not the GUID the hub is
> >giving us is valid.
> >
> >Signed-off-by: Lyude
> >Signed-off-by: Dave Airlie
> >---
> > drivers/gpu/drm/drm_dp_mst_topology.c | 12
> > 1 file changed, 12 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> >b/drivers/gpu/drm/drm_dp_mst_topology.c
> >index 27fbd79..d2efd78 100644
> >--- a/drivers/gpu/drm/drm_dp_mst_topology.c
> >+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> >@@ -2121,6 +2121,8 @@ int drm_dp_mst_topology_mgr_resume(struct
> >drm_dp_mst_topology_mgr *mgr)
> > if (mgr->mst_primary) {
> > int sret;
> >+u8 guid[16];
> >+
> > sret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
> > DP_RECEIVER_CAP_SIZE);
> > if (sret != DP_RECEIVER_CAP_SIZE) {
> > DRM_DEBUG_KMS("dpcd read failed - undocked during
> > suspend?\n");
> >@@ -2135,6 +2137,16 @@ int drm_dp_mst_topology_mgr_resume(struct
> >drm_dp_mst_topology_mgr *mgr)
> > ret = -1;
> > goto out_unlock;
> > }
> >+
> >+/* Some hubs forget their guids after they resume */
> >+sret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
> >+if (sret != 16) {
> >+DRM_DEBUG_KMS("dpcd read failed - undocked during
> >suspend?\n");
> >+ret = -1;
> >+goto out_unlock;
> >+}
> >+drm_dp_check_mstb_guid(mgr->mst_primary, guid);
> >+
> > ret = 0;
> > } else
> > ret = -1;
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Bug 94933] [RV610] Freeze when turning on an external monitor after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=94933 --- Comment #9 from Alex Deucher --- Does disabling dpm fix the issue? remove radeon.dpm=1 from your kernel command line in grub. dpm is disabled by default on r6xx due to stability issues. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/631da645/attachment.html>
[PATCH v3] drm: Release driver references to handle before making it available again
On Fri, Apr 15, 2016 at 12:55:08PM +0100, Chris Wilson wrote:
> When userspace closes a handle, we remove it from the file->object_idr
> and then tell the driver to drop its references to that file/handle.
> However, as the file/handle is already available again for reuse, it may
> be reallocated back to userspace and active on a new object before the
> driver has had a chance to drop the old file/handle references.
>
> Whilst calling back into the driver, we have to drop the
> file->table_lock spinlock and so to prevent reusing the closed handle we
> mark that handle as stale in the idr, perform the callback and then
> remove the handle. We set the stale handle to point to the NULL object,
> then any idr_find() whilst the driver is removing the handle will return
> NULL, just as if the handle is already removed from idr.
>
> v2: Use NULL rather than an ERR_PTR to avoid having to adjust callers.
> idr_alloc() tracks existing handles using an internal bitmap, so we are
> free to use the NULL object as our stale identifier.
> v3: Needed to update the return value check after changing from using
> the stale error pointer to NULL.
>
> Signed-off-by: Chris Wilson
> Cc: dri-devel at lists.freedesktop.org
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Rob Clark
> Cc: Ville Syrjälä
> Cc: Thierry Reding
I added a note about the intended use-case of this and merged it do
drm-misc.
-Daniel
> ---
> drivers/gpu/drm/drm_gem.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index da0c5320789f..e97b7a99807b 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -279,7 +279,6 @@ drm_gem_object_release_handle(int id, void *ptr, void
> *data)
> int
> drm_gem_handle_delete(struct drm_file *filp, u32 handle)
> {
> - struct drm_device *dev;
> struct drm_gem_object *obj;
>
> /* This is gross. The idr system doesn't let us try a delete and
> @@ -294,18 +293,19 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
> spin_lock(&filp->table_lock);
>
> /* Check if we currently have a reference on the object */
> - obj = idr_find(&filp->object_idr, handle);
> - if (obj == NULL) {
> - spin_unlock(&filp->table_lock);
> + obj = idr_replace(&filp->object_idr, NULL, handle);
> + spin_unlock(&filp->table_lock);
> + if (IS_ERR_OR_NULL(obj))
> return -EINVAL;
> - }
> - dev = obj->dev;
>
> - /* Release reference and decrement refcount. */
> + /* Release driver's reference and decrement refcount. */
> + drm_gem_object_release_handle(handle, obj, filp);
> +
> + /* And finally make the handle available for future allocations. */
> + spin_lock(&filp->table_lock);
> idr_remove(&filp->object_idr, handle);
> spin_unlock(&filp->table_lock);
>
> - drm_gem_object_release_handle(handle, obj, filp);
> return 0;
> }
> EXPORT_SYMBOL(drm_gem_handle_delete);
> --
> 2.8.0.rc3
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Nouveau] [PATCH 1/2] nouveau/bl: Assign different names to interfaces
On Fri, Apr 15, 2016 at 11:25 AM, Ilia Mirkin wrote:
> On Fri, Apr 15, 2016 at 11:22 AM, Pierre Moreau
> wrote:
> > On 11:06 AM - Apr 15 2016, Ilia Mirkin wrote:
> >> On Fri, Apr 15, 2016 at 10:57 AM, Pierre Moreau
> wrote:
> >> > Currently, every backlight interface created by Nouveau uses the same
> name,
> >> > nv_backlight. This leads to a sysfs warning as it tries to create an
> already
> >> > existing folder. This patch adds a incremented number to the name,
> but keeps
> >> > the initial name as nv_backlight, to avoid possibly breaking
> userspace; the
> >> > second interface will be named nv_backlight1, and so on.
> >> >
> >> > Fixes: fdo#86539
>
I believe Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86539 is
the preferred format. I think this is picked up by the mesa release scripts
or some such.
> >> > Signed-off-by: Pierre Moreau
> >> > ---
> >> > drm/nouveau/nouveau_backlight.c | 35
> +--
> >> > 1 file changed, 33 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drm/nouveau/nouveau_backlight.c
> b/drm/nouveau/nouveau_backlight.c
> >> > index 89eb460..914e2cb 100644
> >> > --- a/drm/nouveau/nouveau_backlight.c
> >> > +++ b/drm/nouveau/nouveau_backlight.c
> >> > @@ -36,6 +36,10 @@
> >> > #include "nouveau_reg.h"
> >> > #include "nouveau_encoder.h"
> >> >
> >> > +static atomic_t bl_interfaces_nb = { 0 };
> >>
> >> static data is initialized to 0, this should be unnecessary.
> >
> > I didnât know that. But on the other hand, I like having it explicit,
> and it
> > should not add any overhead.
>
> It increases the size of the object file. I believe it's kernel policy
> to avoid static initializations to 0. (Note that this doesn't hold in
> regular user applications, just the kernel.)
>
> -ilia
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
>
-- next part --
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/8b33c66f/attachment-0001.html>
[RFC 1/8] dma-buf/fence: add fence_collection fences
On Fri, Apr 15, 2016 at 11:03 AM, Christian König wrote: > Might be that how amdgpu uses the fence context and sequence number is a bit > questionable, but this will completely break it. You mean it tries to qualesce fences in the same context down to just the last one? That's how it's supposed to be done, and fence_collections do break this somewhat. Without fixing up fence_is_later and friends. Sounds like amdgpu is a good use case to make sure the changes in semantics in these functions result in sensible code. In a way a fence_collection is a fence where the timeline never matches with any other timeline (since it's a combiation). And yeah I think fence_collection should probably compress down the fences to 1 per timeline. But then that's just an implementation detail we can fix later on. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/i915/vlv: Enable/disable VGA hotplugging properly
Huh, neither am I now. I seem to be able to reproduce the problem just fine
again. Anyway I'll send the new versions of the patches in a little bit
On Fri, 2016-04-15 at 18:49 +0300, Ville Syrjälä wrote:
> On Fri, Apr 15, 2016 at 09:47:51AM -0400, Lyude Paul wrote:
> >
> > Looks like we might not need to worry about this patch anymore actually,
> > looks
> > like this problem got fixed by accident by one of the other vlv fixes you
> > pushed.
> Not sure what exactly changed for you, but we definitely need to
> reinitialize ADPA when re-enabling the power well.
>
> >
> > Now it's not always modesetting on hotplug when it was before though :(,
> > so I'll get to work on bisecting that.
> >
> > On Thu, 2016-04-14 at 20:59 +0300, Ville Syrjälä wrote:
> > >
> > > On Tue, Mar 29, 2016 at 04:46:30PM -0400, Lyude wrote:
> > > >
> > > >
> > > > On Valleyview, VGA hotplugging is controlled through a seperate register
> > > > than everything else, VLV_ADPA, which must be explicitly set.
> > > >
> > > > While VGA hotplugging worked(ish) before, it looks like that was mainly
> > > > because we'd unintentionally enable it in
> > > > valleyview_crt_detect_hotplug() when we did a force trigger. This
> > > > doesn't work reliably enough because whenever the display powerwell on
> > > > vlv gets disabled, the values set in VLV_ADPA get cleared and
> > > > consequently VGA hotplugging gets disabled. This causes bugs such as one
> > > > we found on an Intel NUC, where doing the following sequence of
> > > > hotplugs:
> > > >
> > > > - Disconnect all monitors
> > > > - Connect VGA
> > > > - Disconnect VGA
> > > > - Connect HDMI
> > > >
> > > > Would result in hotplugging getting disabled, due to the display
> > > > powerwells getting toggled in the process of connecting HDMI.
> > > >
> > > > CC: stable at vger.kernel.org
> > > > Signed-off-by: Lyude
> > > > ---
> > > > Â drivers/gpu/drm/i915/i915_irq.c | 14 ++
> > > > Â 1 file changed, 14 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > > b/drivers/gpu/drm/i915/i915_irq.c
> > > > index 5aa4239..60592a4 100644
> > > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > > @@ -3611,6 +3611,7 @@ static void valleyview_display_irqs_install(struct
> > > > drm_i915_private *dev_priv)
> > > > Â {
> > > > Â u32 pipestat_mask;
> > > > Â u32 iir_mask;
> > > > + u32 adpa_reg;
> > > > Â enum pipe pipe;
> > > > Â
> > > > Â pipestat_mask = PIPESTAT_INT_STATUS_MASK |
> > > > @@ -3627,6 +3628,12 @@ static void
> > > > valleyview_display_irqs_install(struct
> > > > drm_i915_private *dev_priv)
> > > > Â for_each_pipe(dev_priv, pipe)
> > > > Â Â Â Â Â Â Â i915_enable_pipestat(dev_priv, pipe,
> > > > pipestat_mask);
> > > > Â
> > > > + if (IS_VALLEYVIEW(dev_priv)) {
> > > > + adpa_reg = I915_READ(VLV_ADPA);
> > > > + adpa_reg |= ADPA_CRT_HOTPLUG_ENABLE;
> > > > + I915_WRITE(VLV_ADPA, adpa_reg);
> > > > + }
> > > We might not want to enable that when there's no VGA connector.
> > >
> > > Seems like we should just be calling intel_crt_reset() here. We
> > > definitely don't want to call the reset for hooks for all the other
> > > connectors so drm_mode_config_reset() is out. Also the connector
> > > locking might be problematic here, so I might suggest adjusting
> > > intel_crt_reset() to take an encoder instead of connector, and then
> > > we should be able to walk the encoder list without any troubles.
> > >
> > > >
> > > >
> > > > +
> > > > Â iir_mask = I915_DISPLAY_PORT_INTERRUPT |
> > > > Â Â Â Â I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> > > > Â Â Â Â I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
> > > > @@ -3645,8 +3652,15 @@ static void
> > > > valleyview_display_irqs_uninstall(struct
> > > > drm_i915_private *dev_priv)
> > > > Â {
> > > > Â u32 pipestat_mask;
> > > > Â u32 iir_mask;
> > > > + u32 adpa_reg;
> > > > Â enum pipe pipe;
> > > > Â
> > > > + if (IS_VALLEYVIEW(dev_priv)) {
> > > > + adpa_reg = I915_READ(VLV_ADPA);
> > > > + adpa_reg &= ~ADPA_CRT_HOTPLUG_ENABLE;
> > > > + I915_WRITE(VLV_ADPA, adpa_reg);
> > > > + }
> > > > +
> > > > Â iir_mask = I915_DISPLAY_PORT_INTERRUPT |
> > > > Â Â Â Â I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> > > > Â Â Â Â I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
> > > > --Â
> > > > 2.5.5
> > > >
> > > > ___
> > > > dri-devel mailing list
> > > > dri-devel at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] drm: Release driver references to handle before making it available again
When userspace closes a handle, we remove it from the file->object_idr
and then tell the driver to drop its references to that file/handle.
However, as the file/handle is already available again for reuse, it may
be reallocated back to userspace and active on a new object before the
driver has had a chance to drop the old file/handle references.
Whilst calling back into the driver, we have to drop the
file->table_lock spinlock and so to prevent reusing the closed handle we
mark that handle as stale in the idr, perform the callback and then
remove the handle. We set the stale handle to point to the NULL object,
then any idr_find() whilst the driver is removing the handle will return
NULL, just as if the handle is already removed from idr.
v2: Use NULL rather than an ERR_PTR to avoid having to adjust callers.
idr_alloc() tracks existing handles using an internal bitmap, so we are
free to use the NULL object as our stale identifier.
v3: Needed to update the return value check after changing from using
the stale error pointer to NULL.
Signed-off-by: Chris Wilson
Cc: dri-devel at lists.freedesktop.org
Cc: David Airlie
Cc: Daniel Vetter
Cc: Rob Clark
Cc: Ville Syrjälä
Cc: Thierry Reding
---
drivers/gpu/drm/drm_gem.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index da0c5320789f..e97b7a99807b 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -279,7 +279,6 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
int
drm_gem_handle_delete(struct drm_file *filp, u32 handle)
{
- struct drm_device *dev;
struct drm_gem_object *obj;
/* This is gross. The idr system doesn't let us try a delete and
@@ -294,18 +293,19 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
spin_lock(&filp->table_lock);
/* Check if we currently have a reference on the object */
- obj = idr_find(&filp->object_idr, handle);
- if (obj == NULL) {
- spin_unlock(&filp->table_lock);
+ obj = idr_replace(&filp->object_idr, NULL, handle);
+ spin_unlock(&filp->table_lock);
+ if (IS_ERR_OR_NULL(obj))
return -EINVAL;
- }
- dev = obj->dev;
- /* Release reference and decrement refcount. */
+ /* Release driver's reference and decrement refcount. */
+ drm_gem_object_release_handle(handle, obj, filp);
+
+ /* And finally make the handle available for future allocations. */
+ spin_lock(&filp->table_lock);
idr_remove(&filp->object_idr, handle);
spin_unlock(&filp->table_lock);
- drm_gem_object_release_handle(handle, obj, filp);
return 0;
}
EXPORT_SYMBOL(drm_gem_handle_delete);
--
2.8.0.rc3
[RFC 8/8] drm/fence: add out-fences support
2016-04-15 Daniel Vetter :
> On Thu, Apr 14, 2016 at 06:29:41PM -0700, Gustavo Padovan wrote:
> > From: Gustavo Padovan
> >
> > Support DRM out-fences creating a sync_file with a fence for each crtc
> > update with the DRM_MODE_ATOMIC_OUT_FENCE flag.
> >
> > We then send an struct drm_out_fences array with the out-fences fds back in
> > the drm_atomic_ioctl() as an out arg in the out_fences_ptr field.
> >
> > struct drm_out_fences {
> > __u32 crtc_id;
> > __u32 fd;
> > };
> >
> > Signed-off-by: Gustavo Padovan
> > ---
> > drivers/gpu/drm/drm_atomic.c| 109
> > +++-
> > drivers/gpu/drm/drm_atomic_helper.c | 1 +
> > include/drm/drm_crtc.h | 3 +
> > include/uapi/drm/drm_mode.h | 7 +++
> > 4 files changed, 119 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 0b95526..af6e051 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1560,6 +1560,103 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
> > }
> > EXPORT_SYMBOL(drm_atomic_clean_old_fb);
> >
> > +static int drm_atomic_get_out_fences(struct drm_device *dev,
> > +struct drm_atomic_state *state,
> > +uint32_t __user *out_fences_ptr,
> > +uint64_t count_out_fences,
> > +uint64_t user_data)
> > +{
> > + struct drm_crtc *crtc;
> > + struct drm_crtc_state *crtc_state;
> > + struct drm_out_fences *out_fences;
> > + struct sync_file **sync_file;
> > + int num_fences = 0;
> > + int i, ret;
> > +
> > + out_fences = kcalloc(count_out_fences, sizeof(*out_fences),
> > +GFP_KERNEL);
> > + if (!out_fences)
> > + return -ENOMEM;
> > +
> > + sync_file = kcalloc(count_out_fences, sizeof(*sync_file),
> > +GFP_KERNEL);
> > + if (!sync_file) {
> > + kfree(out_fences);
> > + return -ENOMEM;
> > + }
> > +
> > + for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > + struct drm_pending_vblank_event *e;
> > + struct fence *fence;
> > + char name[32];
> > + int fd;
> > +
> > + fence = sync_timeline_create_fence(crtc->timeline,
> > + crtc->fence_seqno);
> > + if (!fence) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + snprintf(name, sizeof(name), "crtc-%d_%lu",
> > +drm_crtc_index(crtc), crtc->fence_seqno++);
> > +
> > + sync_file[i] = sync_file_create(name, fence);
> > + if(!sync_file[i]) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + fd = get_unused_fd_flags(O_CLOEXEC);
> > + if (fd < 0) {
> > + ret = fd;
> > + goto out;
> > + }
> > +
> > + sync_file_install(sync_file[i], fd);
> > +
> > + if (crtc_state->event) {
> > + crtc_state->event->base.fence = fence;
> > + } else {
> > + e = create_vblank_event(dev, NULL, fence, user_data);
> > + if (!e) {
> > + put_unused_fd(fd);
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + crtc_state->event = e;
> > + }
> > + if (num_fences > count_out_fences) {
> > + put_unused_fd(fd);
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + fence_get(fence);
> > +
> > + out_fences[num_fences].crtc_id = crtc->base.id;
> > + out_fences[num_fences].fd = fd;
> > + num_fences++;
> > + }
> > +
> > + if (copy_to_user(out_fences_ptr, out_fences,
> > +num_fences * sizeof(*out_fences))) {
> > + ret = -EFAULT;
> > + goto out;
> > + }
> > +
> > + return 0;
> > +
> > +out:
> > + for (i = 0 ; i < count_out_fences ; i++) {
> > + if (sync_file[i])
> > + sync_file_put(sync_file[i]);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > int drm_mode_atomic_ioctl(struct drm_device *dev,
> > void *data, struct drm_file *file_priv)
> > {
> > @@ -1568,6 +1665,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > uint32_t __user *count_props_ptr = (uint32_t __user *)(unsigned
> > long)(arg->count_props_ptr);
> > uint32_t __user *props_ptr = (uint32_t __user *)(unsigned
> > long)(arg->props_ptr);
> > uint64_t __user *prop_values_ptr = (uint64_t __user *)(unsigned
> > long)(arg->prop_values_ptr);
> > + uint32_t __user *out_fences_ptr = (uint32_t __user *)(unsigned
[RFC 6/8] drm/fence: create DRM_MODE_ATOMIC_OUT_FENCE flag
2016-04-14 Rob Clark :
> On Thu, Apr 14, 2016 at 9:29 PM, Gustavo Padovan
> wrote:
> > From: Gustavo Padovan
> >
> > This flag tells drm_atomic_ioctl that we want to get a per-crtc out-fence
> > fd back.
> >
> > Signed-off-by: Gustavo Padovan
> > ---
> > include/uapi/drm/drm_mode.h | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 7a7856e..39905cc 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -582,13 +582,15 @@ struct drm_mode_destroy_dumb {
> > #define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
> > #define DRM_MODE_ATOMIC_NONBLOCK 0x0200
> > #define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400
> > +#define DRM_MODE_ATOMIC_OUT_FENCE 0x0800
> >
> > #define DRM_MODE_ATOMIC_FLAGS (\
> > DRM_MODE_PAGE_FLIP_EVENT |\
> > DRM_MODE_PAGE_FLIP_ASYNC |\
> > DRM_MODE_ATOMIC_TEST_ONLY |\
> > DRM_MODE_ATOMIC_NONBLOCK |\
> > - DRM_MODE_ATOMIC_ALLOW_MODESET)
> > + DRM_MODE_ATOMIC_ALLOW_MODESET |\
> > + DRM_MODE_ATOMIC_OUT_FENCE)
>
> just to be pedantic / bisectable, perhaps this should be squashed in
> to patch that actually starts using this flag? Otherwise there is an
> intermediate state in git where the flag is accepted but ignored..
Sure, I totally agree.
Gustavo
[PATCH 2/2] drm/amdgpu: remove sorting of CS BOs
On Fri, Apr 15, 2016 at 11:19 AM, Christian König
wrote:
> From: Christian König
>
> Not needed any more.
Applied the series.
Alex
>
> Signed-off-by: Christian König
> Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 23 ---
> 1 file changed, 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 9392e50..00cf74a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -24,7 +24,6 @@
> * Authors:
> *Jerome Glisse
> */
> -#include
> #include
> #include
> #include
> @@ -527,16 +526,6 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser
> *p)
> return 0;
> }
>
> -static int cmp_size_smaller_first(void *priv, struct list_head *a,
> - struct list_head *b)
> -{
> - struct amdgpu_bo_list_entry *la = list_entry(a, struct
> amdgpu_bo_list_entry, tv.head);
> - struct amdgpu_bo_list_entry *lb = list_entry(b, struct
> amdgpu_bo_list_entry, tv.head);
> -
> - /* Sort A before B if A is smaller. */
> - return (int)la->robj->tbo.num_pages - (int)lb->robj->tbo.num_pages;
> -}
> -
> /**
> * cs_parser_fini() - clean parser states
> * @parser:parser structure holding parsing context.
> @@ -553,18 +542,6 @@ static void amdgpu_cs_parser_fini(struct
> amdgpu_cs_parser *parser, int error, bo
> if (!error) {
> amdgpu_vm_move_pt_bos_in_lru(parser->adev, &fpriv->vm);
>
> - /* Sort the buffer list from the smallest to largest buffer,
> -* which affects the order of buffers in the LRU list.
> -* This assures that the smallest buffers are added first
> -* to the LRU list, so they are likely to be later evicted
> -* first, instead of large buffers whose eviction is more
> -* expensive.
> -*
> -* This slightly lowers the number of bytes moved by TTM
> -* per frame under memory pressure.
> -*/
> - list_sort(NULL, &parser->validated, cmp_size_smaller_first);
> -
> ttm_eu_fence_buffer_objects(&parser->ticket,
> &parser->validated,
> parser->fence);
> --
> 2.5.0
>
[RFC 5/8] drm/fence: add fence to drm_pending_event
2016-04-15 Daniel Vetter :
> On Thu, Apr 14, 2016 at 06:29:38PM -0700, Gustavo Padovan wrote:
> > From: Gustavo Padovan
> >
> > Now a drm_pending_event can either send a real drm_event or signal a
> > fence, or both. It allow us to signal via fences when the buffer is
> > displayed on the screen. Which in turn means that the previous buffer
> > is not in use anymore and can be freed or sent back to another driver
> > for processing.
> >
> > Signed-off-by: Gustavo Padovan
>
> Do you have atomic igt testcase that exercise all the combinations of
> drm_event and in/out-fences and make sure it all keeps working? Tomeu
> already converted that over to be a generic testcase.
Not yet. I'll check for test cases.
>
> > ---
> > drivers/gpu/drm/drm_atomic.c | 19 +--
> > drivers/gpu/drm/drm_fops.c | 5 +++--
> > drivers/gpu/drm/drm_irq.c| 7 +++
> > include/drm/drmP.h | 1 +
> > 4 files changed, 24 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 6702502..0b95526 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1431,7 +1431,8 @@ EXPORT_SYMBOL(drm_atomic_async_commit);
> > */
> >
> > static struct drm_pending_vblank_event *create_vblank_event(
> > - struct drm_device *dev, struct drm_file *file_priv, uint64_t
> > user_data)
> > + struct drm_device *dev, struct drm_file *file_priv,
> > + struct fence *fence, uint64_t user_data)
> > {
> > struct drm_pending_vblank_event *e = NULL;
> > int ret;
> > @@ -1444,12 +1445,17 @@ static struct drm_pending_vblank_event
> > *create_vblank_event(
> > e->event.base.length = sizeof(e->event);
> > e->event.user_data = user_data;
> >
> > - ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
> > - if (ret) {
> > - kfree(e);
> > - return NULL;
> > + if (file_priv) {
> > + ret = drm_event_reserve_init(dev, file_priv, &e->base,
> > +&e->event.base);
> > + if (ret) {
> > + kfree(e);
> > + return NULL;
> > + }
> > }
> >
> > + e->base.fence = fence;
> > +
> > return e;
> > }
> >
> > @@ -1676,7 +1682,8 @@ retry:
> > for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > struct drm_pending_vblank_event *e;
> >
> > - e = create_vblank_event(dev, file_priv, arg->user_data);
> > + e = create_vblank_event(dev, file_priv, NULL,
> > + arg->user_data);
> > if (!e) {
> > ret = -ENOMEM;
> > goto out;
> > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> > index aeef58e..38def49 100644
> > --- a/drivers/gpu/drm/drm_fops.c
> > +++ b/drivers/gpu/drm/drm_fops.c
> > @@ -801,8 +801,9 @@ void drm_send_event_locked(struct drm_device *dev,
> > struct drm_pending_event *e)
> > {
> > assert_spin_locked(&dev->event_lock);
> >
> > - if (!e->file_priv) {
> > - e->destroy(e);
> > + if (!e->file_priv || !e->event) {
>
> This would be a bug: e->file_priv != NULL iff e->event != NULL. How did
> this happen?
Not sure now. But I needed this to prevent a crash, I don't have logs of
it anymore, I'll check this again.
>
> > + if (e->destroy)
> > + e->destroy(e);
> > return;
> > }
> >
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index 3c1a6f1..0c5d7cb 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -41,6 +41,7 @@
> >
> > #include
> > #include
> > +#include
> >
> > /* Access macro for slots in vblank timestamp ringbuffer. */
> > #define vblanktimestamp(dev, pipe, count) \
> > @@ -1124,6 +1125,12 @@ void drm_send_vblank_event(struct drm_device *dev,
> > unsigned int pipe,
> > now = get_drm_timestamp();
> > }
> > e->pipe = pipe;
> > +
> > + if (e->base.fence) {
> > + fence_signal(e->base.fence);
> > + fence_put(e->base.fence);
> > + }
>
> I'd put this into drm_send_event_locked even. In case we send out fences
> for other events too (e.g. exynos uses drm_event for rendering ...).
Okay. Looking into that.
Gustavo
[PATCH 01/14] drm/amdgpu: use drm_crtc_send_vblank_event()
On 15.04.2016 02:48, Gustavo Padovan wrote: > From: Gustavo Padovan > > Replace the legacy drm_send_vblank_event() with the new helper function. > > Signed-off-by: Gustavo Padovan > --- > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > index 6de2ce53..92c5a71 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > @@ -3370,7 +3370,7 @@ static int dce_v10_0_pageflip_irq(struct amdgpu_device > *adev, > > /* wakeup usersapce */ > if (works->event) > - drm_send_vblank_event(adev->ddev, crtc_id, works->event); > + drm_crtc_send_vblank_event(&amdgpu_crtc->base, works->event); > > spin_unlock_irqrestore(&adev->ddev->event_lock, flags); > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > index e9ccc6b..2f784f2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > @@ -3366,7 +3366,7 @@ static int dce_v11_0_pageflip_irq(struct amdgpu_device > *adev, > > /* wakeup usersapce */ > if(works->event) > - drm_send_vblank_event(adev->ddev, crtc_id, works->event); > + drm_crtc_send_vblank_event(&amdgpu_crtc->base, works->event); > > spin_unlock_irqrestore(&adev->ddev->event_lock, flags); > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > index e56b55d..9155e3b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > @@ -3379,7 +3379,7 @@ static int dce_v8_0_pageflip_irq(struct amdgpu_device > *adev, > > /* wakeup usersapce */ > if (works->event) > - drm_send_vblank_event(adev->ddev, crtc_id, works->event); > + drm_crtc_send_vblank_event(&amdgpu_crtc->base, works->event); > > spin_unlock_irqrestore(&adev->ddev->event_lock, flags); > > This patch and patch 8 are Reviewed-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
[RFC 4/8] drm/fence: add in-fences support
2016-04-15 Daniel Vetter :
> On Thu, Apr 14, 2016 at 06:29:37PM -0700, Gustavo Padovan wrote:
> > From: Gustavo Padovan
> >
> > There is now a new property called FENCE_FD attached to every plane
> > state that receives the sync_file fd from userspace via the atomic commit
> > IOCTL.
> >
> > The fd is then translated to a fence (that may be a fence_collection
> > subclass or just a normal fence) and then used by DRM to fence_wait() for
> > all fences in the sync_file to signal. So it only commits when all
> > framebuffers are ready to scanout.
> >
> > Signed-off-by: Gustavo Padovan
> > ---
> > drivers/gpu/drm/Kconfig | 1 +
> > drivers/gpu/drm/drm_atomic.c| 8
> > drivers/gpu/drm/drm_atomic_helper.c | 5 +
> > drivers/gpu/drm/drm_crtc.c | 7 +++
> > include/drm/drm_crtc.h | 1 +
> > 5 files changed, 22 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index f2a74d0..3c987e3 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -12,6 +12,7 @@ menuconfig DRM
> > select I2C
> > select I2C_ALGOBIT
> > select DMA_SHARED_BUFFER
> > + select SYNC_FILE
> > help
> > Kernel-level support for the Direct Rendering Infrastructure (DRI)
> > introduced in XFree86 4.0. If you say Y here, you need to select
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 8ee1db8..6702502 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -30,6 +30,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > /**
> > * drm_atomic_state_default_release -
> > @@ -680,6 +681,11 @@ int drm_atomic_plane_set_property(struct drm_plane
> > *plane,
> > drm_atomic_set_fb_for_plane(state, fb);
> > if (fb)
> > drm_framebuffer_unreference(fb);
> > + } else if (property == config->prop_fence_fd) {
> > + state->fence = sync_file_fences_get(val);
> > + if (!state->fence)
> > + return -EINVAL;
> > + fence_get(state->fence);
>
> Yeah, this fence_get must be part of sync_file_fences_get, this code here
> has a race (exercise for the reader to describe where things go wrong
> here). Also, you need to explicitly filter out -1 here first I think.
Right, I missed this race. Move it sync_file_fences_get() just fixes it.
I'll add the filter for -1 too and a testcase.
>
> Needs an atomic testcase to make sure setting FENCE_FD to -1 doesn't fall
> over (since generic userspace might do this in an attempt to restore all
> the state).
>
> > } else if (property == config->prop_crtc_id) {
> > struct drm_crtc *crtc = drm_crtc_find(dev, val);
> > return drm_atomic_set_crtc_for_plane(state, crtc);
> > @@ -737,6 +743,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> >
> > if (property == config->prop_fb_id) {
> > *val = (state->fb) ? state->fb->base.id : 0;
> > + } else if (property == config->prop_fence_fd) {
> > + *val = -1;
> > } else if (property == config->prop_crtc_id) {
> > *val = (state->crtc) ? state->crtc->base.id : 0;
> > } else if (property == config->prop_crtc_x) {
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index f85ef8c..6ed8339 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -2687,6 +2687,11 @@ void __drm_atomic_helper_plane_destroy_state(struct
> > drm_plane *plane,
> > {
> > if (state->fb)
> > drm_framebuffer_unreference(state->fb);
> > +
> > + if (state->fence) {
> > + fence_put(state->fence);
> > + state->fence = NULL;
>
> No need to set to NULL, we don't do that for ->fb either.
Ok.
Gustavo
[RFC 1/8] dma-buf/fence: add fence_collection fences
2016-04-15 Daniel Vetter : > On Fri, Apr 15, 2016 at 11:03 AM, Christian König > wrote: > > Might be that how amdgpu uses the fence context and sequence number is a bit > > questionable, but this will completely break it. > > You mean it tries to qualesce fences in the same context down to just > the last one? That's how it's supposed to be done, and > fence_collections do break this somewhat. Without fixing up > fence_is_later and friends. Sounds like amdgpu is a good use case to > make sure the changes in semantics in these functions result in > sensible code. In a way a fence_collection is a fence where the > timeline never matches with any other timeline (since it's a > combiation). > > And yeah I think fence_collection should probably compress down the > fences to 1 per timeline. But then that's just an implementation > detail we can fix later on. You mean asking for a new context for every collection? Gustavo
[RFC 1/8] dma-buf/fence: add fence_collection fences
2016-04-15 Christian König :
> Am 15.04.2016 um 10:02 schrieb Daniel Vetter:
> >On Thu, Apr 14, 2016 at 06:29:34PM -0700, Gustavo Padovan wrote:
> >>From: Gustavo Padovan
> >>
> >>struct fence_collection inherits from struct fence and carries a
> >>collection of fences that needs to be waited together.
> >>
> >>It is useful to translate a sync_file to a fence to remove the complexity
> >>of dealing with sync_files from DRM drivers. So even if there are many
> >>fences in the sync_file that needs to waited for a commit to happen
> >>drivers would only worry about a standard struct fence.That means that no
> >>changes needed to any driver besides supporting fences.
> >>
> >>fence_collection's fence doesn't belong to any timeline context.
> >>
> >>Signed-off-by: Gustavo Padovan
> >>---
> >> drivers/dma-buf/Makefile | 2 +-
> >> drivers/dma-buf/fence-collection.c | 138
> >> +
> >> drivers/dma-buf/fence.c| 2 +-
> >> include/linux/fence-collection.h | 56 +++
> >> include/linux/fence.h | 2 +
> >> 5 files changed, 198 insertions(+), 2 deletions(-)
> >> create mode 100644 drivers/dma-buf/fence-collection.c
> >> create mode 100644 include/linux/fence-collection.h
> >>
> >>diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
> >>index 43325a1..30b8464 100644
> >>--- a/drivers/dma-buf/Makefile
> >>+++ b/drivers/dma-buf/Makefile
> >>@@ -1,3 +1,3 @@
> >>-obj-y := dma-buf.o fence.o reservation.o seqno-fence.o
> >>+obj-y := dma-buf.o fence.o reservation.o seqno-fence.o fence-collection.o
> >> obj-$(CONFIG_SYNC_FILE) += sync_file.o sync_timeline.o
> >> sync_debug.o
> >> obj-$(CONFIG_SW_SYNC) += sw_sync.o
> >>diff --git a/drivers/dma-buf/fence-collection.c
> >>b/drivers/dma-buf/fence-collection.c
> >>new file mode 100644
> >>index 000..8a4ecb0
> >>--- /dev/null
> >>+++ b/drivers/dma-buf/fence-collection.c
> >>@@ -0,0 +1,138 @@
> >>+/*
> >>+ * fence-collection: aggregate fences to be waited together
> >>+ *
> >>+ * Copyright (C) 2016 Collabora Ltd
> >>+ * Authors:
> >>+ * Gustavo Padovan
> >>+ *
> >>+ * This program is free software; you can redistribute it and/or modify it
> >>+ * under the terms of the GNU General Public License version 2 as
> >>published by
> >>+ * the Free Software Foundation.
> >>+ *
> >>+ * This program is distributed in the hope that it will be useful, but
> >>WITHOUT
> >>+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> >>+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> >>for
> >>+ * more details.
> >>+ */
> >>+
> >>+#include
> >>+#include
> >>+#include
> >>+
> >>+static const char *fence_collection_get_driver_name(struct fence *fence)
> >>+{
> >>+ struct fence_collection *collection = to_fence_collection(fence);
> >>+ struct fence *f = collection->fences[0].fence;
> >>+
> >>+ return f->ops->get_driver_name(fence);
> >>+}
>
> I would rather return some constant name here instead of relying that the
> collection already has a fence added and that all fences are from the same
> driver.
If we merge _init and _add this will not be a problem anymore and we can
return the actual driver name.
>
> >>+
> >>+static const char *fence_collection_get_timeline_name(struct fence *fence)
> >>+{
> >>+ return "no context";
> >>+}
> >>+
> >>+static bool fence_collection_enable_signaling(struct fence *fence)
> >>+{
> >>+ struct fence_collection *collection = to_fence_collection(fence);
> >>+
> >>+ return atomic_read(&collection->num_pending_fences);
> >>+}
> >>+
> >>+static bool fence_collection_signaled(struct fence *fence)
> >>+{
> >>+ struct fence_collection *collection = to_fence_collection(fence);
> >>+
> >>+ return (atomic_read(&collection->num_pending_fences) == 0);
> >>+}
> >>+
> >>+static void fence_collection_release(struct fence *fence)
> >>+{
> >>+ struct fence_collection *collection = to_fence_collection(fence);
> >>+ int i;
> >>+
> >>+ for (i = 0 ; i < collection->num_fences ; i++) {
> >>+ fence_remove_callback(collection->fences[i].fence,
> >>+ &collection->fences[i].cb);
> >>+ fence_put(collection->fences[i].fence);
> >>+ }
> >>+
> >>+ fence_free(fence);
> >>+}
> >>+
> >>+static signed long fence_collection_wait(struct fence *fence, bool intr,
> >>+signed long timeout)
> >>+{
> >>+ struct fence_collection *collection = to_fence_collection(fence);
> >>+ int i;
> >>+
> >>+ for (i = 0 ; i < collection->num_fences ; i++) {
> >>+ timeout = fence_wait(collection->fences[i].fence, intr);
> >>+ if (timeout < 0)
> >>+ return timeout;
> >>+ }
> >>+
> >>+ return timeout;
> >>+}
> >>+
> >>+static const struct fence_ops fence_collection_ops = {
> >>+ .get_driver_name = fence_collection_get_driver_name,
> >>+ .get_timeline_name = fence_collection_ge
[Nouveau] [PATCH 1/2] nouveau/bl: Assign different names to interfaces
On Fri, Apr 15, 2016 at 11:22 AM, Pierre Moreau
wrote:
> On 11:06 AM - Apr 15 2016, Ilia Mirkin wrote:
>> On Fri, Apr 15, 2016 at 10:57 AM, Pierre Moreau
>> wrote:
>> > Currently, every backlight interface created by Nouveau uses the same name,
>> > nv_backlight. This leads to a sysfs warning as it tries to create an
>> > already
>> > existing folder. This patch adds a incremented number to the name, but
>> > keeps
>> > the initial name as nv_backlight, to avoid possibly breaking userspace; the
>> > second interface will be named nv_backlight1, and so on.
>> >
>> > Fixes: fdo#86539
>> > Signed-off-by: Pierre Moreau
>> > ---
>> > drm/nouveau/nouveau_backlight.c | 35 +--
>> > 1 file changed, 33 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drm/nouveau/nouveau_backlight.c
>> > b/drm/nouveau/nouveau_backlight.c
>> > index 89eb460..914e2cb 100644
>> > --- a/drm/nouveau/nouveau_backlight.c
>> > +++ b/drm/nouveau/nouveau_backlight.c
>> > @@ -36,6 +36,10 @@
>> > #include "nouveau_reg.h"
>> > #include "nouveau_encoder.h"
>> >
>> > +static atomic_t bl_interfaces_nb = { 0 };
>>
>> static data is initialized to 0, this should be unnecessary.
>
> I didnât know that. But on the other hand, I like having it explicit, and it
> should not add any overhead.
It increases the size of the object file. I believe it's kernel policy
to avoid static initializations to 0. (Note that this doesn't hold in
regular user applications, just the kernel.)
-ilia
[Nouveau] [PATCH 1/2] nouveau/bl: Assign different names to interfaces
On Fri, Apr 15, 2016 at 10:57 AM, Pierre Moreau
wrote:
> Currently, every backlight interface created by Nouveau uses the same name,
> nv_backlight. This leads to a sysfs warning as it tries to create an already
> existing folder. This patch adds a incremented number to the name, but keeps
> the initial name as nv_backlight, to avoid possibly breaking userspace; the
> second interface will be named nv_backlight1, and so on.
>
> Fixes: fdo#86539
> Signed-off-by: Pierre Moreau
> ---
> drm/nouveau/nouveau_backlight.c | 35 +--
> 1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drm/nouveau/nouveau_backlight.c b/drm/nouveau/nouveau_backlight.c
> index 89eb460..914e2cb 100644
> --- a/drm/nouveau/nouveau_backlight.c
> +++ b/drm/nouveau/nouveau_backlight.c
> @@ -36,6 +36,10 @@
> #include "nouveau_reg.h"
> #include "nouveau_encoder.h"
>
> +static atomic_t bl_interfaces_nb = { 0 };
static data is initialized to 0, this should be unnecessary.
I'd also call it "backlights" or something like that. No need for
multiple words...
> +
> +static char* nouveau_get_backlight_name(void);
Please organize the code to avoid forward declarations.
> +
> static int
> nv40_get_intensity(struct backlight_device *bd)
> {
> @@ -74,6 +78,7 @@ nv40_backlight_init(struct drm_connector *connector)
> struct nvif_object *device = &drm->device.object;
> struct backlight_properties props;
> struct backlight_device *bd;
> + char* backlight_name = NULL;
>
> if (!(nvif_rd32(device, NV40_PMC_BACKLIGHT) &
> NV40_PMC_BACKLIGHT_MASK))
> return 0;
> @@ -81,8 +86,14 @@ nv40_backlight_init(struct drm_connector *connector)
> memset(&props, 0, sizeof(struct backlight_properties));
> props.type = BACKLIGHT_RAW;
> props.max_brightness = 31;
> - bd = backlight_device_register("nv_backlight", connector->kdev, drm,
> + backlight_name = nouveau_get_backlight_name();
> + bd = backlight_device_register(backlight_name , connector->kdev, drm,
>&nv40_bl_ops, &props);
> +
> + // backlight_device_register() makes a copy
> + kfree(backlight_name);
> + backlight_name = NULL;
> +
> if (IS_ERR(bd))
> return PTR_ERR(bd);
> drm->backlight = bd;
> @@ -182,6 +193,7 @@ nv50_backlight_init(struct drm_connector *connector)
> struct backlight_properties props;
> struct backlight_device *bd;
> const struct backlight_ops *ops;
> + char* backlight_name = NULL;
>
> nv_encoder = find_encoder(connector, DCB_OUTPUT_LVDS);
> if (!nv_encoder) {
> @@ -203,8 +215,14 @@ nv50_backlight_init(struct drm_connector *connector)
> memset(&props, 0, sizeof(struct backlight_properties));
> props.type = BACKLIGHT_RAW;
> props.max_brightness = 100;
> - bd = backlight_device_register("nv_backlight", connector->kdev,
> + backlight_name = nouveau_get_backlight_name();
> + bd = backlight_device_register(backlight_name, connector->kdev,
>nv_encoder, ops, &props);
> +
> + // backlight_device_register() makes a copy
> + kfree(backlight_name);
> + backlight_name = NULL;
> +
> if (IS_ERR(bd))
> return PTR_ERR(bd);
>
> @@ -252,3 +270,16 @@ nouveau_backlight_exit(struct drm_device *dev)
> drm->backlight = NULL;
> }
> }
> +
> +static char*
> +nouveau_get_backlight_name(void)
> +{
> + // 12 chars for "nv_backlight" + 2 for two digits number + 1 for '\0'
> + char* backlight_name = (char*)kmalloc(sizeof(char[15]), GFP_KERNEL);
Making this stack-allocated in the caller would be so much simpler...
> + const int nb = atomic_inc_return(&bl_interfaces_nb) - 1;
This kinda sucks if you reload nouveau a bunch. How about using an
"ida". Have a look in drivers/gpu/drm/drm_crtc.c for how I use that
one.
> + if (nb > 0 && nb < 100)
> + sprintf(backlight_name, "nv_backlight%d", nb);
> + else
> + sprintf(backlight_name, "nv_backlight");
> + return backlight_name;
> +}
> --
> 2.8.0
>
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
[RFC 1/8] dma-buf/fence: add fence_collection fences
Am 15.04.2016 um 10:02 schrieb Daniel Vetter:
> On Thu, Apr 14, 2016 at 06:29:34PM -0700, Gustavo Padovan wrote:
>> From: Gustavo Padovan
>>
>> struct fence_collection inherits from struct fence and carries a
>> collection of fences that needs to be waited together.
>>
>> It is useful to translate a sync_file to a fence to remove the complexity
>> of dealing with sync_files from DRM drivers. So even if there are many
>> fences in the sync_file that needs to waited for a commit to happen
>> drivers would only worry about a standard struct fence.That means that no
>> changes needed to any driver besides supporting fences.
>>
>> fence_collection's fence doesn't belong to any timeline context.
>>
>> Signed-off-by: Gustavo Padovan
>> ---
>> drivers/dma-buf/Makefile | 2 +-
>> drivers/dma-buf/fence-collection.c | 138
>> +
>> drivers/dma-buf/fence.c| 2 +-
>> include/linux/fence-collection.h | 56 +++
>> include/linux/fence.h | 2 +
>> 5 files changed, 198 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/dma-buf/fence-collection.c
>> create mode 100644 include/linux/fence-collection.h
>>
>> diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
>> index 43325a1..30b8464 100644
>> --- a/drivers/dma-buf/Makefile
>> +++ b/drivers/dma-buf/Makefile
>> @@ -1,3 +1,3 @@
>> -obj-y := dma-buf.o fence.o reservation.o seqno-fence.o
>> +obj-y := dma-buf.o fence.o reservation.o seqno-fence.o fence-collection.o
>> obj-$(CONFIG_SYNC_FILE)+= sync_file.o sync_timeline.o
>> sync_debug.o
>> obj-$(CONFIG_SW_SYNC) += sw_sync.o
>> diff --git a/drivers/dma-buf/fence-collection.c
>> b/drivers/dma-buf/fence-collection.c
>> new file mode 100644
>> index 000..8a4ecb0
>> --- /dev/null
>> +++ b/drivers/dma-buf/fence-collection.c
>> @@ -0,0 +1,138 @@
>> +/*
>> + * fence-collection: aggregate fences to be waited together
>> + *
>> + * Copyright (C) 2016 Collabora Ltd
>> + * Authors:
>> + * Gustavo Padovan
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published
>> by
>> + * the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +
>> +static const char *fence_collection_get_driver_name(struct fence *fence)
>> +{
>> +struct fence_collection *collection = to_fence_collection(fence);
>> +struct fence *f = collection->fences[0].fence;
>> +
>> +return f->ops->get_driver_name(fence);
>> +}
I would rather return some constant name here instead of relying that
the collection already has a fence added and that all fences are from
the same driver.
>> +
>> +static const char *fence_collection_get_timeline_name(struct fence *fence)
>> +{
>> +return "no context";
>> +}
>> +
>> +static bool fence_collection_enable_signaling(struct fence *fence)
>> +{
>> +struct fence_collection *collection = to_fence_collection(fence);
>> +
>> +return atomic_read(&collection->num_pending_fences);
>> +}
>> +
>> +static bool fence_collection_signaled(struct fence *fence)
>> +{
>> +struct fence_collection *collection = to_fence_collection(fence);
>> +
>> +return (atomic_read(&collection->num_pending_fences) == 0);
>> +}
>> +
>> +static void fence_collection_release(struct fence *fence)
>> +{
>> +struct fence_collection *collection = to_fence_collection(fence);
>> +int i;
>> +
>> +for (i = 0 ; i < collection->num_fences ; i++) {
>> +fence_remove_callback(collection->fences[i].fence,
>> + &collection->fences[i].cb);
>> +fence_put(collection->fences[i].fence);
>> +}
>> +
>> +fence_free(fence);
>> +}
>> +
>> +static signed long fence_collection_wait(struct fence *fence, bool intr,
>> + signed long timeout)
>> +{
>> +struct fence_collection *collection = to_fence_collection(fence);
>> +int i;
>> +
>> +for (i = 0 ; i < collection->num_fences ; i++) {
>> +timeout = fence_wait(collection->fences[i].fence, intr);
>> +if (timeout < 0)
>> +return timeout;
>> +}
>> +
>> +return timeout;
>> +}
>> +
>> +static const struct fence_ops fence_collection_ops = {
>> +.get_driver_name = fence_collection_get_driver_name,
>> +.get_timeline_name = fence_collection_get_timeline_name,
>> +.enable_signaling = fence_collection_enable_signaling,
>> +.signaled = fence_collection_signaled,
>> +.wait = fence_collection_wait,
>> +.release = fence_collection_release,
>> +};
>> +
>> +static void collection_chec
[PATCH 2/2] drm/amdgpu: remove sorting of CS BOs
Hello Christian ,
As per below comment large buffer eviction is more expensive. So
removing this code will solve the same problem?
- /* Sort the buffer list from the smallest to largest buffer,
-* which affects the order of buffers in the LRU list.
-* This assures that the smallest buffers are added first
-* to the LRU list, so they are likely to be later evicted
-* first, instead of large buffers whose eviction is more
-* expensive.
-*
-* This slightly lowers the number of bytes moved by TTM
-* per frame under memory pressure.
-*/
- list_sort(NULL, &parser->validated, cmp_size_smaller_first);
-
Thanks,
Ayyappa.
On Thu, Apr 14, 2016 at 6:24 PM, Christian König
wrote:
> From: Christian König
>
> Not needed any more.
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 23 ---
> 1 file changed, 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 9392e50..00cf74a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -24,7 +24,6 @@
> * Authors:
> *Jerome Glisse
> */
> -#include
> #include
> #include
> #include
> @@ -527,16 +526,6 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser
> *p)
> return 0;
> }
>
> -static int cmp_size_smaller_first(void *priv, struct list_head *a,
> - struct list_head *b)
> -{
> - struct amdgpu_bo_list_entry *la = list_entry(a, struct
> amdgpu_bo_list_entry, tv.head);
> - struct amdgpu_bo_list_entry *lb = list_entry(b, struct
> amdgpu_bo_list_entry, tv.head);
> -
> - /* Sort A before B if A is smaller. */
> - return (int)la->robj->tbo.num_pages - (int)lb->robj->tbo.num_pages;
> -}
> -
> /**
> * cs_parser_fini() - clean parser states
> * @parser:parser structure holding parsing context.
> @@ -553,18 +542,6 @@ static void amdgpu_cs_parser_fini(struct
> amdgpu_cs_parser *parser, int error, bo
> if (!error) {
> amdgpu_vm_move_pt_bos_in_lru(parser->adev, &fpriv->vm);
>
> - /* Sort the buffer list from the smallest to largest buffer,
> -* which affects the order of buffers in the LRU list.
> -* This assures that the smallest buffers are added first
> -* to the LRU list, so they are likely to be later evicted
> -* first, instead of large buffers whose eviction is more
> -* expensive.
> -*
> -* This slightly lowers the number of bytes moved by TTM
> -* per frame under memory pressure.
> -*/
> - list_sort(NULL, &parser->validated, cmp_size_smaller_first);
> -
> ttm_eu_fence_buffer_objects(&parser->ticket,
> &parser->validated,
> parser->fence);
> --
> 2.5.0
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: fix compare_const_fl.cocci warnings
Am 15.04.2016 um 09:15 schrieb Julia Lawall:
> Move constants to the right of binary operators.
>
> Generated by: scripts/coccinelle/misc/compare_const_fl.cocci
>
> Signed-off-by: Fengguang Wu
> Signed-off-by: Julia Lawall
In general the patch looks ok, but do we have a documented preference
where to place constants in the coding style docs?
While it's not so much of a problem any more with modern compilers, some
people still prefer to have it on the left side to catch accidental
value assignments.
Regards,
Christian.
> ---
>
> Could be nice to put the thing being tested first.
>
> amdgpu_grph_object_id_helpers.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_grph_object_id_helpers.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_grph_object_id_helpers.c
> @@ -169,11 +169,11 @@ struct graphics_object_id amdgpu_object_
> struct graphics_object_id go_id = { 0 };
>
> type = object_type_from_bios_object_id(bios_object_id);
> - if (OBJECT_TYPE_UNKNOWN == type)
> + if (type == OBJECT_TYPE_UNKNOWN)
> return go_id;
>
> enum_id = enum_id_from_bios_object_id(bios_object_id);
> - if (ENUM_ID_UNKNOWN == enum_id)
> + if (enum_id == ENUM_ID_UNKNOWN)
> return go_id;
>
> go_id = display_graphics_object_id_init(
[Bug 116251] radeon 5a427809cd9143bef89ee3110f45e84f37484218 "drm/radeon: disable runtime pm on PX laptop" makes dGPU never stop
https://bugzilla.kernel.org/show_bug.cgi?id=116251 --- Comment #5 from Eugene Shalygin --- With the quirk it works, thank you. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v6 3/4] drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read()
This is part of a patch series to migrate all of the workarounds for
commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's
DP helper.
Some sinks will just return garbage for the first aux tranaction they
receive when coming out of sleep mode, so we need to perform an additional
read before the actual read to workaround this.
Changes since v5
- If the throwaway read in drm_dp_dpcd_read() fails, return the error
from that instead of continuing. This follows the same logic we do in
drm_dp_dpcd_access() (e.g. the error from the first transaction may
differ from the errors that proceeding attempts might return).
Signed-off-by: Lyude
---
drivers/gpu/drm/drm_dp_helper.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 540c3e4..eeaf5a7 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -248,6 +248,25 @@ unlock:
ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
void *buffer, size_t size)
{
+ int ret;
+
+ /*
+* HP ZR24w corrupts the first DPCD access after entering power save
+* mode. Eg. on a read, the entire buffer will be filled with the same
+* byte. Do a throw away read to avoid corrupting anything we care
+* about. Afterwards things will work correctly until the monitor
+* gets woken up and subsequently re-enters power save mode.
+*
+* The user pressing any button on the monitor is enough to wake it
+* up, so there is no particularly good place to do the workaround.
+* We just have to do it before any DPCD access and hope that the
+* monitor doesn't power down exactly after the throw away read.
+*/
+ ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, DP_DPCD_REV, buffer,
+1);
+ if (ret != 1)
+ return ret;
+
return drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, buffer,
size);
}
--
2.5.5
[RFC 8/8] drm/fence: add out-fences support
On Thu, Apr 14, 2016 at 06:29:41PM -0700, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Support DRM out-fences creating a sync_file with a fence for each crtc
> update with the DRM_MODE_ATOMIC_OUT_FENCE flag.
>
> We then send an struct drm_out_fences array with the out-fences fds back in
> the drm_atomic_ioctl() as an out arg in the out_fences_ptr field.
>
> struct drm_out_fences {
> __u32 crtc_id;
> __u32 fd;
> };
>
> Signed-off-by: Gustavo Padovan
> ---
> drivers/gpu/drm/drm_atomic.c| 109
> +++-
> drivers/gpu/drm/drm_atomic_helper.c | 1 +
> include/drm/drm_crtc.h | 3 +
> include/uapi/drm/drm_mode.h | 7 +++
> 4 files changed, 119 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 0b95526..af6e051 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1560,6 +1560,103 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>
> +static int drm_atomic_get_out_fences(struct drm_device *dev,
> + struct drm_atomic_state *state,
> + uint32_t __user *out_fences_ptr,
> + uint64_t count_out_fences,
> + uint64_t user_data)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *crtc_state;
> + struct drm_out_fences *out_fences;
> + struct sync_file **sync_file;
> + int num_fences = 0;
> + int i, ret;
> +
> + out_fences = kcalloc(count_out_fences, sizeof(*out_fences),
> + GFP_KERNEL);
> + if (!out_fences)
> + return -ENOMEM;
> +
> + sync_file = kcalloc(count_out_fences, sizeof(*sync_file),
> + GFP_KERNEL);
> + if (!sync_file) {
> + kfree(out_fences);
> + return -ENOMEM;
> + }
> +
> + for_each_crtc_in_state(state, crtc, crtc_state, i) {
> + struct drm_pending_vblank_event *e;
> + struct fence *fence;
> + char name[32];
> + int fd;
> +
> + fence = sync_timeline_create_fence(crtc->timeline,
> +crtc->fence_seqno);
> + if (!fence) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + snprintf(name, sizeof(name), "crtc-%d_%lu",
> + drm_crtc_index(crtc), crtc->fence_seqno++);
> +
> + sync_file[i] = sync_file_create(name, fence);
> + if(!sync_file[i]) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + fd = get_unused_fd_flags(O_CLOEXEC);
> + if (fd < 0) {
> + ret = fd;
> + goto out;
> + }
> +
> + sync_file_install(sync_file[i], fd);
> +
> + if (crtc_state->event) {
> + crtc_state->event->base.fence = fence;
> + } else {
> + e = create_vblank_event(dev, NULL, fence, user_data);
> + if (!e) {
> + put_unused_fd(fd);
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + crtc_state->event = e;
> + }
> + if (num_fences > count_out_fences) {
> + put_unused_fd(fd);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + fence_get(fence);
> +
> + out_fences[num_fences].crtc_id = crtc->base.id;
> + out_fences[num_fences].fd = fd;
> + num_fences++;
> + }
> +
> + if (copy_to_user(out_fences_ptr, out_fences,
> + num_fences * sizeof(*out_fences))) {
> + ret = -EFAULT;
> + goto out;
> + }
> +
> + return 0;
> +
> +out:
> + for (i = 0 ; i < count_out_fences ; i++) {
> + if (sync_file[i])
> + sync_file_put(sync_file[i]);
> + }
> +
> + return ret;
> +}
> +
> int drm_mode_atomic_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv)
> {
> @@ -1568,6 +1665,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> uint32_t __user *count_props_ptr = (uint32_t __user *)(unsigned
> long)(arg->count_props_ptr);
> uint32_t __user *props_ptr = (uint32_t __user *)(unsigned
> long)(arg->props_ptr);
> uint64_t __user *prop_values_ptr = (uint64_t __user *)(unsigned
> long)(arg->prop_values_ptr);
> + uint32_t __user *out_fences_ptr = (uint32_t __user *)(unsigned
> long)(arg->out_fences_ptr);
> unsigned int copied_objs, copied_props;
> struct drm_atomic_state *state;
> struct drm_m
[RFC 4/8] drm/fence: add in-fences support
On Thu, Apr 14, 2016 at 06:29:37PM -0700, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> There is now a new property called FENCE_FD attached to every plane
> state that receives the sync_file fd from userspace via the atomic commit
> IOCTL.
>
> The fd is then translated to a fence (that may be a fence_collection
> subclass or just a normal fence) and then used by DRM to fence_wait() for
> all fences in the sync_file to signal. So it only commits when all
> framebuffers are ready to scanout.
>
> Signed-off-by: Gustavo Padovan
> ---
> drivers/gpu/drm/Kconfig | 1 +
> drivers/gpu/drm/drm_atomic.c| 8
> drivers/gpu/drm/drm_atomic_helper.c | 5 +
> drivers/gpu/drm/drm_crtc.c | 7 +++
> include/drm/drm_crtc.h | 1 +
> 5 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index f2a74d0..3c987e3 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -12,6 +12,7 @@ menuconfig DRM
> select I2C
> select I2C_ALGOBIT
> select DMA_SHARED_BUFFER
> + select SYNC_FILE
> help
> Kernel-level support for the Direct Rendering Infrastructure (DRI)
> introduced in XFree86 4.0. If you say Y here, you need to select
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 8ee1db8..6702502 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
>
> /**
> * drm_atomic_state_default_release -
> @@ -680,6 +681,11 @@ int drm_atomic_plane_set_property(struct drm_plane
> *plane,
> drm_atomic_set_fb_for_plane(state, fb);
> if (fb)
> drm_framebuffer_unreference(fb);
> + } else if (property == config->prop_fence_fd) {
Need to fence_put if state->fence is non-NULL already. Could happen if
userspace sets FENCE_FD more than once (another case for your testcase I'd
say).
-Daniel
> + state->fence = sync_file_fences_get(val);
> + if (!state->fence)
> + return -EINVAL;
> + fence_get(state->fence);
> } else if (property == config->prop_crtc_id) {
> struct drm_crtc *crtc = drm_crtc_find(dev, val);
> return drm_atomic_set_crtc_for_plane(state, crtc);
> @@ -737,6 +743,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>
> if (property == config->prop_fb_id) {
> *val = (state->fb) ? state->fb->base.id : 0;
> + } else if (property == config->prop_fence_fd) {
> + *val = -1;
> } else if (property == config->prop_crtc_id) {
> *val = (state->crtc) ? state->crtc->base.id : 0;
> } else if (property == config->prop_crtc_x) {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index f85ef8c..6ed8339 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2687,6 +2687,11 @@ void __drm_atomic_helper_plane_destroy_state(struct
> drm_plane *plane,
> {
> if (state->fb)
> drm_framebuffer_unreference(state->fb);
> +
> + if (state->fence) {
> + fence_put(state->fence);
> + state->fence = NULL;
> + }
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 55ffde5..65212ce 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1278,6 +1278,7 @@ int drm_universal_plane_init(struct drm_device *dev,
> struct drm_plane *plane,
>
> if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> drm_object_attach_property(&plane->base, config->prop_fb_id, 0);
> + drm_object_attach_property(&plane->base, config->prop_fence_fd,
> -1);
> drm_object_attach_property(&plane->base, config->prop_crtc_id,
> 0);
> drm_object_attach_property(&plane->base, config->prop_crtc_x,
> 0);
> drm_object_attach_property(&plane->base, config->prop_crtc_y,
> 0);
> @@ -1533,6 +1534,12 @@ static int drm_mode_create_standard_properties(struct
> drm_device *dev)
> return -ENOMEM;
> dev->mode_config.prop_fb_id = prop;
>
> + prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC,
> + "FENCE_FD", -1, INT_MAX);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.prop_fence_fd = prop;
> +
> prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
> "CRTC_ID", DRM_MODE_OBJECT_CRTC);
> if (!prop)
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 8cb377c..5ba3cda 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -2122,6 +2122,7 @@ struct drm_mode_config {
> struct drm_property *prop_crtc_w;
>
[RFC 5/8] drm/fence: add fence to drm_pending_event
On Thu, Apr 14, 2016 at 06:29:38PM -0700, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Now a drm_pending_event can either send a real drm_event or signal a
> fence, or both. It allow us to signal via fences when the buffer is
> displayed on the screen. Which in turn means that the previous buffer
> is not in use anymore and can be freed or sent back to another driver
> for processing.
>
> Signed-off-by: Gustavo Padovan
Do you have atomic igt testcase that exercise all the combinations of
drm_event and in/out-fences and make sure it all keeps working? Tomeu
already converted that over to be a generic testcase.
> ---
> drivers/gpu/drm/drm_atomic.c | 19 +--
> drivers/gpu/drm/drm_fops.c | 5 +++--
> drivers/gpu/drm/drm_irq.c| 7 +++
> include/drm/drmP.h | 1 +
> 4 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 6702502..0b95526 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1431,7 +1431,8 @@ EXPORT_SYMBOL(drm_atomic_async_commit);
> */
>
> static struct drm_pending_vblank_event *create_vblank_event(
> - struct drm_device *dev, struct drm_file *file_priv, uint64_t
> user_data)
> + struct drm_device *dev, struct drm_file *file_priv,
> + struct fence *fence, uint64_t user_data)
> {
> struct drm_pending_vblank_event *e = NULL;
> int ret;
> @@ -1444,12 +1445,17 @@ static struct drm_pending_vblank_event
> *create_vblank_event(
> e->event.base.length = sizeof(e->event);
> e->event.user_data = user_data;
>
> - ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
> - if (ret) {
> - kfree(e);
> - return NULL;
> + if (file_priv) {
> + ret = drm_event_reserve_init(dev, file_priv, &e->base,
> + &e->event.base);
> + if (ret) {
> + kfree(e);
> + return NULL;
> + }
> }
>
> + e->base.fence = fence;
> +
> return e;
> }
>
> @@ -1676,7 +1682,8 @@ retry:
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> struct drm_pending_vblank_event *e;
>
> - e = create_vblank_event(dev, file_priv, arg->user_data);
> + e = create_vblank_event(dev, file_priv, NULL,
> + arg->user_data);
> if (!e) {
> ret = -ENOMEM;
> goto out;
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index aeef58e..38def49 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -801,8 +801,9 @@ void drm_send_event_locked(struct drm_device *dev, struct
> drm_pending_event *e)
> {
> assert_spin_locked(&dev->event_lock);
>
> - if (!e->file_priv) {
> - e->destroy(e);
> + if (!e->file_priv || !e->event) {
This would be a bug: e->file_priv != NULL iff e->event != NULL. How did
this happen?
> + if (e->destroy)
> + e->destroy(e);
> return;
> }
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 3c1a6f1..0c5d7cb 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -41,6 +41,7 @@
>
> #include
> #include
> +#include
>
> /* Access macro for slots in vblank timestamp ringbuffer. */
> #define vblanktimestamp(dev, pipe, count) \
> @@ -1124,6 +1125,12 @@ void drm_send_vblank_event(struct drm_device *dev,
> unsigned int pipe,
> now = get_drm_timestamp();
> }
> e->pipe = pipe;
> +
> + if (e->base.fence) {
> + fence_signal(e->base.fence);
> + fence_put(e->base.fence);
> + }
I'd put this into drm_send_event_locked even. In case we send out fences
for other events too (e.g. exynos uses drm_event for rendering ...).
-Daniel
> +
> send_vblank_event(dev, e, seq, &now);
> }
> EXPORT_SYMBOL(drm_send_vblank_event);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 3c8422c..8f83c2a 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -282,6 +282,7 @@ struct drm_ioctl_desc {
> /* Event queued up for userspace to read */
> struct drm_pending_event {
> struct drm_event *event;
> + struct fence *fence;
> struct list_head link;
> struct list_head pending_link;
> struct drm_file *file_priv;
> --
> 2.5.5
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v2 02/11] drm/i915: Remove stallcheck special handling.
On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> Re-use unpin_work->pending, but also set vblank count before
> intel_mark_page_flip_active to be sure.
Be sure of what?
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 11 ++-
> drivers/gpu/drm/i915/intel_display.c | 31 ---
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> 3 files changed, 18 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9640738aabf2..df8073a2ffbe 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -582,9 +582,14 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
> seq_printf(m, "No flip due on pipe %c (plane %c)\n",
> pipe, plane);
> } else {
> + u32 pending;
> u32 addr;
>
> - if (atomic_read(&work->pending) <
> INTEL_FLIP_COMPLETE) {
> + pending = atomic_read(&work->pending);
> + if (pending == INTEL_FLIP_INACTIVE) {
> + seq_printf(m, "Flip ioctl preparing on pipe
> %c (plane %c)\n",
> +pipe, plane);
> + } else if (pending >= INTEL_FLIP_COMPLETE) {
> seq_printf(m, "Flip queued on pipe %c (plane
> %c)\n",
> pipe, plane);
> } else {
> @@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
> work->flip_queued_vblank,
> work->flip_ready_vblank,
> drm_crtc_vblank_count(&crtc->base));
> - if (work->enable_stall_check)
> - seq_puts(m, "Stall check enabled, ");
> - else
> - seq_puts(m, "Stall check waiting for page
> flip ioctl, ");
> seq_printf(m, "%d prepares\n", atomic_read(&work
> ->pending));
>
> if (INTEL_INFO(dev)->gen >= 4)
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f2be54a48727..618e034a7a5e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11415,8 +11415,6 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
> *mmio_flip)
> if (work == NULL)
> return;
>
> - intel_mark_page_flip_active(work);
> -
> intel_pipe_update_start(crtc);
>
> if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> @@ -11426,6 +11424,8 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
> *mmio_flip)
> ilk_do_mmio_flip(crtc, work);
>
> intel_pipe_update_end(crtc);
> +
> + intel_mark_page_flip_active(work);
Is this to avoid triggering the stall check during the wait from a vblank
evasion?
> }
>
> static void intel_mmio_flip_work_func(struct work_struct *work)
> @@ -11492,15 +11492,11 @@ static bool __intel_pageflip_stall_check(struct
> drm_device *dev,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_unpin_work *work = intel_crtc->unpin_work;
> u32 addr;
> + u32 pending;
>
> - if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE)
> - return true;
> -
> - if (atomic_read(&work->pending) < INTEL_FLIP_PENDING)
> - return false;
> -
> - if (!work->enable_stall_check)
> - return false;
> + pending = atomic_read(&work->pending);
> + if (pending != INTEL_FLIP_PENDING)
> + return pending == INTEL_FLIP_COMPLETE;
>
> if (work->flip_ready_vblank == 0) {
> if (work->flip_queued_req &&
> @@ -11676,6 +11672,11 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
>*/
> if (!mmio_flip) {
> ret = i915_gem_object_sync(obj, engine, &request);
> + if (!ret && !request) {
> + request = i915_gem_request_alloc(engine, NULL);
> + ret = PTR_ERR_OR_ZERO(request);
> + }
> +
> if (ret)
> goto cleanup_pending;
> }
> @@ -11687,6 +11688,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
> obj, 0);
> work->gtt_offset += intel_crtc->dspaddr_offset;
> + work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>
> if (mmio_flip) {
> ret = intel_queue_mmio_flip(dev, crtc, obj);
> @@ -11696,14 +11698,6 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
> i915_gem_request_assign(&work->flip_queued_req,
>
[RFC 4/8] drm/fence: add in-fences support
On Thu, Apr 14, 2016 at 06:29:37PM -0700, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> There is now a new property called FENCE_FD attached to every plane
> state that receives the sync_file fd from userspace via the atomic commit
> IOCTL.
>
> The fd is then translated to a fence (that may be a fence_collection
> subclass or just a normal fence) and then used by DRM to fence_wait() for
> all fences in the sync_file to signal. So it only commits when all
> framebuffers are ready to scanout.
>
> Signed-off-by: Gustavo Padovan
> ---
> drivers/gpu/drm/Kconfig | 1 +
> drivers/gpu/drm/drm_atomic.c| 8
> drivers/gpu/drm/drm_atomic_helper.c | 5 +
> drivers/gpu/drm/drm_crtc.c | 7 +++
> include/drm/drm_crtc.h | 1 +
> 5 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index f2a74d0..3c987e3 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -12,6 +12,7 @@ menuconfig DRM
> select I2C
> select I2C_ALGOBIT
> select DMA_SHARED_BUFFER
> + select SYNC_FILE
> help
> Kernel-level support for the Direct Rendering Infrastructure (DRI)
> introduced in XFree86 4.0. If you say Y here, you need to select
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 8ee1db8..6702502 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
>
> /**
> * drm_atomic_state_default_release -
> @@ -680,6 +681,11 @@ int drm_atomic_plane_set_property(struct drm_plane
> *plane,
> drm_atomic_set_fb_for_plane(state, fb);
> if (fb)
> drm_framebuffer_unreference(fb);
> + } else if (property == config->prop_fence_fd) {
> + state->fence = sync_file_fences_get(val);
> + if (!state->fence)
> + return -EINVAL;
> + fence_get(state->fence);
Yeah, this fence_get must be part of sync_file_fences_get, this code here
has a race (exercise for the reader to describe where things go wrong
here). Also, you need to explicitly filter out -1 here first I think.
Needs an atomic testcase to make sure setting FENCE_FD to -1 doesn't fall
over (since generic userspace might do this in an attempt to restore all
the state).
> } else if (property == config->prop_crtc_id) {
> struct drm_crtc *crtc = drm_crtc_find(dev, val);
> return drm_atomic_set_crtc_for_plane(state, crtc);
> @@ -737,6 +743,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>
> if (property == config->prop_fb_id) {
> *val = (state->fb) ? state->fb->base.id : 0;
> + } else if (property == config->prop_fence_fd) {
> + *val = -1;
> } else if (property == config->prop_crtc_id) {
> *val = (state->crtc) ? state->crtc->base.id : 0;
> } else if (property == config->prop_crtc_x) {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index f85ef8c..6ed8339 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2687,6 +2687,11 @@ void __drm_atomic_helper_plane_destroy_state(struct
> drm_plane *plane,
> {
> if (state->fb)
> drm_framebuffer_unreference(state->fb);
> +
> + if (state->fence) {
> + fence_put(state->fence);
> + state->fence = NULL;
No need to set to NULL, we don't do that for ->fb either.
> + }
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 55ffde5..65212ce 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1278,6 +1278,7 @@ int drm_universal_plane_init(struct drm_device *dev,
> struct drm_plane *plane,
>
> if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> drm_object_attach_property(&plane->base, config->prop_fb_id, 0);
> + drm_object_attach_property(&plane->base, config->prop_fence_fd,
> -1);
> drm_object_attach_property(&plane->base, config->prop_crtc_id,
> 0);
> drm_object_attach_property(&plane->base, config->prop_crtc_x,
> 0);
> drm_object_attach_property(&plane->base, config->prop_crtc_y,
> 0);
> @@ -1533,6 +1534,12 @@ static int drm_mode_create_standard_properties(struct
> drm_device *dev)
> return -ENOMEM;
> dev->mode_config.prop_fb_id = prop;
>
> + prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC,
> + "FENCE_FD", -1, INT_MAX);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.prop_fence_fd = prop;
> +
> prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
> "CRTC_ID", DRM_MODE_OBJECT_CRTC);
[RFC 1/8] dma-buf/fence: add fence_collection fences
On Thu, Apr 14, 2016 at 06:29:34PM -0700, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> struct fence_collection inherits from struct fence and carries a
> collection of fences that needs to be waited together.
>
> It is useful to translate a sync_file to a fence to remove the complexity
> of dealing with sync_files from DRM drivers. So even if there are many
> fences in the sync_file that needs to waited for a commit to happen
> drivers would only worry about a standard struct fence.That means that no
> changes needed to any driver besides supporting fences.
>
> fence_collection's fence doesn't belong to any timeline context.
>
> Signed-off-by: Gustavo Padovan
> ---
> drivers/dma-buf/Makefile | 2 +-
> drivers/dma-buf/fence-collection.c | 138
> +
> drivers/dma-buf/fence.c| 2 +-
> include/linux/fence-collection.h | 56 +++
> include/linux/fence.h | 2 +
> 5 files changed, 198 insertions(+), 2 deletions(-)
> create mode 100644 drivers/dma-buf/fence-collection.c
> create mode 100644 include/linux/fence-collection.h
>
> diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
> index 43325a1..30b8464 100644
> --- a/drivers/dma-buf/Makefile
> +++ b/drivers/dma-buf/Makefile
> @@ -1,3 +1,3 @@
> -obj-y := dma-buf.o fence.o reservation.o seqno-fence.o
> +obj-y := dma-buf.o fence.o reservation.o seqno-fence.o fence-collection.o
> obj-$(CONFIG_SYNC_FILE) += sync_file.o sync_timeline.o
> sync_debug.o
> obj-$(CONFIG_SW_SYNC)+= sw_sync.o
> diff --git a/drivers/dma-buf/fence-collection.c
> b/drivers/dma-buf/fence-collection.c
> new file mode 100644
> index 000..8a4ecb0
> --- /dev/null
> +++ b/drivers/dma-buf/fence-collection.c
> @@ -0,0 +1,138 @@
> +/*
> + * fence-collection: aggregate fences to be waited together
> + *
> + * Copyright (C) 2016 Collabora Ltd
> + * Authors:
> + * Gustavo Padovan
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#include
> +#include
> +#include
> +
> +static const char *fence_collection_get_driver_name(struct fence *fence)
> +{
> + struct fence_collection *collection = to_fence_collection(fence);
> + struct fence *f = collection->fences[0].fence;
> +
> + return f->ops->get_driver_name(fence);
> +}
> +
> +static const char *fence_collection_get_timeline_name(struct fence *fence)
> +{
> + return "no context";
> +}
> +
> +static bool fence_collection_enable_signaling(struct fence *fence)
> +{
> + struct fence_collection *collection = to_fence_collection(fence);
> +
> + return atomic_read(&collection->num_pending_fences);
> +}
> +
> +static bool fence_collection_signaled(struct fence *fence)
> +{
> + struct fence_collection *collection = to_fence_collection(fence);
> +
> + return (atomic_read(&collection->num_pending_fences) == 0);
> +}
> +
> +static void fence_collection_release(struct fence *fence)
> +{
> + struct fence_collection *collection = to_fence_collection(fence);
> + int i;
> +
> + for (i = 0 ; i < collection->num_fences ; i++) {
> + fence_remove_callback(collection->fences[i].fence,
> + &collection->fences[i].cb);
> + fence_put(collection->fences[i].fence);
> + }
> +
> + fence_free(fence);
> +}
> +
> +static signed long fence_collection_wait(struct fence *fence, bool intr,
> + signed long timeout)
> +{
> + struct fence_collection *collection = to_fence_collection(fence);
> + int i;
> +
> + for (i = 0 ; i < collection->num_fences ; i++) {
> + timeout = fence_wait(collection->fences[i].fence, intr);
> + if (timeout < 0)
> + return timeout;
> + }
> +
> + return timeout;
> +}
> +
> +static const struct fence_ops fence_collection_ops = {
> + .get_driver_name = fence_collection_get_driver_name,
> + .get_timeline_name = fence_collection_get_timeline_name,
> + .enable_signaling = fence_collection_enable_signaling,
> + .signaled = fence_collection_signaled,
> + .wait = fence_collection_wait,
> + .release = fence_collection_release,
> +};
> +
> +static void collection_check_cb_func(struct fence *fence, struct fence_cb
> *cb)
> +{
> + struct fence_collection_cb *f_cb;
> + struct fence_collection *collection;
> +
> + f_cb = container_of(cb, struct fence_collection_cb, cb);
> + collection = f_cb->collection;
> +
> + if (atomic_dec_and_test(&collection->num_p
[RFC 2/8] dma-buf/sync_file: add sync_file_fences_get()
On Thu, Apr 14, 2016 at 06:29:35PM -0700, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Creates a function that given an sync file descriptor returns a
> fence_collection containing all fences in the sync_file.
>
> If there is only one fence in the sync_file this fence itself is returned,
> however if there is more than one, a fence_collection fence is returned.
>
> Signed-off-by: Gustavo Padovan
> ---
> drivers/dma-buf/sync_file.c | 37 +
> include/linux/sync_file.h | 10 ++
> 2 files changed, 47 insertions(+)
>
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 4d2af24..926fafa 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -122,6 +123,39 @@ void sync_file_install(struct sync_file *sync_file, int
> fd)
> }
> EXPORT_SYMBOL(sync_file_install);
>
> +struct fence *sync_file_fences_get(int fd)
> +{
> + struct sync_file *sync_file;
> + struct fence_collection *collection;
> + int i;
> +
> + sync_file = sync_file_fdget(fd);
> + if (!sync_file)
> + return NULL;
> +
> + if (sync_file->num_fences == 1) {
> + struct fence *fence = sync_file->cbs[0].fence;
> +
> + sync_file_put(sync_file);
> + return fence;
> + }
> +
> + collection = fence_collection_init(sync_file->num_fences);
> + if (!collection) {
> + sync_file_put(sync_file);
> + return NULL;
> + }
> +
> + for (i = 0 ; i < sync_file->num_fences ; i++)
> + fence_collection_add(collection, sync_file->cbs[i].fence);
> +
> + sync_file->collection = collection;
> + sync_file_put(sync_file);
> +
> + return &collection->base;
> +}
This function should probably acquire a reference for the returned fence
for the caller.
-Daniel
> +EXPORT_SYMBOL(sync_file_fences_get);
> +
> static void sync_file_add_pt(struct sync_file *sync_file, int *i,
>struct fence *fence)
> {
> @@ -200,6 +234,9 @@ static void sync_file_free(struct kref *kref)
>kref);
> int i;
>
> + if (sync_file->collection)
> + fence_collection_put(sync_file->collection);
> +
> for (i = 0; i < sync_file->num_fences; ++i) {
> fence_remove_callback(sync_file->cbs[i].fence,
> &sync_file->cbs[i].cb);
> diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h
> index 7b7a89d..2cb0486 100644
> --- a/include/linux/sync_file.h
> +++ b/include/linux/sync_file.h
> @@ -49,6 +49,7 @@ struct sync_file {
> wait_queue_head_t wq;
> atomic_tstatus;
>
> + struct fence_collection *collection;
> struct sync_file_cb cbs[];
> };
>
> @@ -103,4 +104,13 @@ void sync_file_put(struct sync_file *sync_file);
> */
> void sync_file_install(struct sync_file *sync_file, int fd);
>
> +/**
> + * sync_file_fences_get - get the fence related to the fd
> + * @fd: file descriptor to look for a fence collection
> + *
> + * Ensures @fd references a valid sync_file and returns the base object
> + * of the fence_collection that contains all fences in the sync_file
> + * or NULL in case of error.
> + */
> +struct fence *sync_file_fences_get(int fd);
> #endif /* _LINUX_SYNC_H */
> --
> 2.5.5
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/i915/vlv: Enable/disable VGA hotplugging properly
Looks like we might not need to worry about this patch anymore actually, looks
like this problem got fixed by accident by one of the other vlv fixes you
pushed. Now it's not always modesetting on hotplug when it was before though :(,
so I'll get to work on bisecting that.
On Thu, 2016-04-14 at 20:59 +0300, Ville Syrjälä wrote:
> On Tue, Mar 29, 2016 at 04:46:30PM -0400, Lyude wrote:
> >
> > On Valleyview, VGA hotplugging is controlled through a seperate register
> > than everything else, VLV_ADPA, which must be explicitly set.
> >
> > While VGA hotplugging worked(ish) before, it looks like that was mainly
> > because we'd unintentionally enable it in
> > valleyview_crt_detect_hotplug() when we did a force trigger. This
> > doesn't work reliably enough because whenever the display powerwell on
> > vlv gets disabled, the values set in VLV_ADPA get cleared and
> > consequently VGA hotplugging gets disabled. This causes bugs such as one
> > we found on an Intel NUC, where doing the following sequence of
> > hotplugs:
> >
> > - Disconnect all monitors
> > - Connect VGA
> > - Disconnect VGA
> > - Connect HDMI
> >
> > Would result in hotplugging getting disabled, due to the display
> > powerwells getting toggled in the process of connecting HDMI.
> >
> > CC: stable at vger.kernel.org
> > Signed-off-by: Lyude
> > ---
> > Â drivers/gpu/drm/i915/i915_irq.c | 14 ++
> > Â 1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 5aa4239..60592a4 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3611,6 +3611,7 @@ static void valleyview_display_irqs_install(struct
> > drm_i915_private *dev_priv)
> > Â {
> > Â u32 pipestat_mask;
> > Â u32 iir_mask;
> > + u32 adpa_reg;
> > Â enum pipe pipe;
> > Â
> > Â pipestat_mask = PIPESTAT_INT_STATUS_MASK |
> > @@ -3627,6 +3628,12 @@ static void valleyview_display_irqs_install(struct
> > drm_i915_private *dev_priv)
> > Â for_each_pipe(dev_priv, pipe)
> > Â Â Â Â Â Â Â i915_enable_pipestat(dev_priv, pipe, pipestat_mask);
> > Â
> > + if (IS_VALLEYVIEW(dev_priv)) {
> > + adpa_reg = I915_READ(VLV_ADPA);
> > + adpa_reg |= ADPA_CRT_HOTPLUG_ENABLE;
> > + I915_WRITE(VLV_ADPA, adpa_reg);
> > + }
> We might not want to enable that when there's no VGA connector.
>
> Seems like we should just be calling intel_crt_reset() here. We
> definitely don't want to call the reset for hooks for all the other
> connectors so drm_mode_config_reset() is out. Also the connector
> locking might be problematic here, so I might suggest adjusting
> intel_crt_reset() to take an encoder instead of connector, and then
> we should be able to walk the encoder list without any troubles.
>
> >
> > +
> > Â iir_mask = I915_DISPLAY_PORT_INTERRUPT |
> > Â Â Â Â I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> > Â Â Â Â I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
> > @@ -3645,8 +3652,15 @@ static void valleyview_display_irqs_uninstall(struct
> > drm_i915_private *dev_priv)
> > Â {
> > Â u32 pipestat_mask;
> > Â u32 iir_mask;
> > + u32 adpa_reg;
> > Â enum pipe pipe;
> > Â
> > + if (IS_VALLEYVIEW(dev_priv)) {
> > + adpa_reg = I915_READ(VLV_ADPA);
> > + adpa_reg &= ~ADPA_CRT_HOTPLUG_ENABLE;
> > + I915_WRITE(VLV_ADPA, adpa_reg);
> > + }
> > +
> > Â iir_mask = I915_DISPLAY_PORT_INTERRUPT |
> > Â Â Â Â I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> > Â Â Â Â I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
> > --Â
> > 2.5.5
> >
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC 3/8] drm/fence: allow fence waiting to be interrupted by userspace
On Thu, Apr 14, 2016 at 06:29:36PM -0700, Gustavo Padovan wrote: > From: Gustavo Padovan > > If userspace is running an synchronously atomic commit and interrupts the > atomic operation during fence_wait() it will hang until the timer expires, > so here we change the wait to be interruptible so it stop immediately when > userspace wants to quit. > > Signed-off-by: Gustavo Padovan > --- > drivers/gpu/drm/drm_atomic_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index 7bf678e..f85ef8c 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1007,7 +1007,7 @@ void drm_atomic_helper_wait_for_fences(struct > drm_device *dev, > > WARN_ON(!plane->state->fb); > > - fence_wait(plane->state->fence, false); > + fence_wait(plane->state->fence, true); You need to propagate the error code when allowing the wait to be interrupted - we can't proceed with the atomic update in that case, but need to bail out. And userspace needs to restart the ioctl. Also, needs a testcase I think. -Daniel > fence_put(plane->state->fence); > plane->state->fence = NULL; > } > -- > 2.5.5 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[Intel-gfx] [PATCH 04/14] drm/i915: use drm_crtc_send_vblank_event()
On Thu, Apr 14, 2016 at 10:48:15AM -0700, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Replace the legacy drm_send_vblank_event() with the new helper function.
>
> Signed-off-by: Gustavo Padovan
Applied to drm-intel.git, thanks.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 29aa64b..181ee3c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3802,9 +3802,7 @@ static void page_flip_completed(struct intel_crtc
> *intel_crtc)
> intel_crtc->unpin_work = NULL;
>
> if (work->event)
> - drm_send_vblank_event(intel_crtc->base.dev,
> - intel_crtc->pipe,
> - work->event);
> + drm_crtc_send_vblank_event(&intel_crtc->base, work->event);
>
> drm_crtc_vblank_put(&intel_crtc->base);
>
> @@ -11699,7 +11697,7 @@ retry:
>
> if (ret == 0 && event) {
> spin_lock_irq(&dev->event_lock);
> - drm_send_vblank_event(dev, pipe, event);
> + drm_crtc_send_vblank_event(crtc, event);
> spin_unlock_irq(&dev->event_lock);
> }
> }
> --
> 2.5.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/amdgpu: fix compare_const_fl.cocci warnings
Move constants to the right of binary operators.
Generated by: scripts/coccinelle/misc/compare_const_fl.cocci
Signed-off-by: Fengguang Wu
Signed-off-by: Julia Lawall
---
Could be nice to put the thing being tested first.
amdgpu_grph_object_id_helpers.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_grph_object_id_helpers.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_grph_object_id_helpers.c
@@ -169,11 +169,11 @@ struct graphics_object_id amdgpu_object_
struct graphics_object_id go_id = { 0 };
type = object_type_from_bios_object_id(bios_object_id);
- if (OBJECT_TYPE_UNKNOWN == type)
+ if (type == OBJECT_TYPE_UNKNOWN)
return go_id;
enum_id = enum_id_from_bios_object_id(bios_object_id);
- if (ENUM_ID_UNKNOWN == enum_id)
+ if (enum_id == ENUM_ID_UNKNOWN)
return go_id;
go_id = display_graphics_object_id_init(
[Bug 94933] [RV610] Freeze when turning on an external monitor after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=94933 --- Comment #8 from Nicola Mori --- Created attachment 122962 --> https://bugs.freedesktop.org/attachment.cgi?id=122962&action=edit Xorg.0.log for screen resize -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/a05b429e/attachment.html>
[Bug 94933] [RV610] Freeze when turning on an external monitor after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=94933 --- Comment #7 from Nicola Mori --- Created attachment 122961 --> https://bugs.freedesktop.org/attachment.cgi?id=122961&action=edit dmesg on screen resize -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/3943b2f5/attachment.html>
[Bug 94933] [RV610] Freeze when turning on an external monitor after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=94933 --- Comment #6 from Nicola Mori --- Changing the desktop size screws the screen but the system is still responsive. I can go back to a fully working situation with xrandr --fb 1280x800 I attach the relevant parts of dmesg and Xorg.0.log. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/7d760b3a/attachment.html>
[PATCH] drm/amdgpu: fix compare_const_fl.cocci warnings
On Fri, 2016-04-15 at 16:20 +0200, Julia Lawall wrote:
> On Fri, 15 Apr 2016, Christian König wrote:
> > Am 15.04.2016 um 09:15 schrieb Julia Lawall:
> > > Move constants to the right of binary operators.
> > >
> > > Generated by: scripts/coccinelle/misc/compare_const_fl.cocci
> > >
> > > Signed-off-by: Fengguang Wu
> > > Signed-off-by: Julia Lawall
> >
> > In general the patch looks ok, but do we have a documented preference where
> > to
> > place constants in the coding style docs?
> >
> > While it's not so much of a problem any more with modern compilers, some
> > people still prefer to have it on the left side to catch accidental value
> > assignments.
>
> I don't know if it is documented. Joe Perches suggested that on the right
> was better in general - maybe he knows if this is written somewhere.
>
> There are 504 occurrences of NULL == in the kernel, and 19524 occurrences
> of == NULL.
A long time ago Linus wrote:
> On Wed, 2004-03-10 at 18:36, Linus Torvalds wrote:
> > > The kind of bug that the "0 == x" syntax protects against
> > > is LESS LIKELY to happen than the kind of bug it tends to cause.
> >Â
> > Not my experience.  I'd personally prefer a stated preference for.
> >Â
> > (lvalue == rvalue) vs (rvalue == lvalue)
>Â
> The thing is, your "vs" above makes no sense.
>Â
> Quite often, both sides are rvalues, or lvalues. In fact, often you may
> not even know from a simple syntactic look which one either side is, sinceÂ
> they can be macros etc.
>Â
> So asking for consistency is just impossible, because the exact sameÂ
> expression may semantically parse as either depending on things likeÂ
> macros that have architectural dependencies.Â
>Â
> So the rule would have to be something like this:
>Â Â - if one side is _obviously_ a lvalue, and the other side is _obviously_Â
>Â Â Â Â a rvalue, then do X.
>Â
> That kind of rule makes very little sense, but if you want a stated
> preference, then my preference is to say that in that obvious case, the
> lvalue comes on the left side, and the rvalue comes on the right side.
>Â
> Why? Because that is literally how people think. People have been taughtÂ
> since before first grade to think "if I have 8 apples, and I give Tom oneÂ
> apple, how many apples do I have".
>Â
> Notice how I didn't say "if 8 applies is what I have.."
>Â
> The reason for "if (x == 8)" comes from the way we're taught to think.Â
> Arguing against that _fact_ is just totally non-productive, and you haveÂ
> to _force_ yourself to write it the other way around.
>Â
> And that just means that you will do other mistakes. You'll spend yourÂ
> time thinking about trying to express your conditionals in strange ways,Â
> and then not think about the _real_ issue.
>Â
> So let's make a few rules:
>Â
>Â Â - write your logical expressions the way people EXPECT them to beÂ
>Â Â Â Â written. No silly rules that make no sense.
>Â
>Â Â Â Â Ergo:
>Â
>Â Â Â Â Â Â Â Â Â if (x == 8)
>Â
>Â Â Â Â is the ONE AND ONLY SANE WAY.
>Â
>Â Â - avoid using assignment inside logical expressions unless you have aÂ
>Â Â Â Â damn good reason to.
>Â
>Â Â Â Â Ergo: write
>Â
>Â Â Â Â Â Â Â Â Â error = myfunction()
>Â Â Â Â Â Â Â Â Â if (error) {
>Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ...
>Â
>Â Â Â Â instead of writing
>Â
>Â Â Â Â Â Â Â Â Â if (error = myfunction())
>Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â
>Â
>Â Â Â Â which is just unreadable and stupid.
>Â
>Â Â - Don't get hung about stupid rules.Â
>Â
>Â Â Â Â Ergo: sometimes assignments in conditionals make sense, especially in
>Â Â Â Â loops. Don't avoid them just because of some silly rule. But strive to
>Â Â Â Â use an explicit equality test when you do so:
>Â
>Â Â Â Â Â Â Â Â Â while ((a = function(b)) != 0)Â
>Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ...
>Â
>Â Â Â Â is fine.
>Â
>Â Â - The compiler warns about the mistakes that remain, if you follow theseÂ
>Â Â Â Â rules.
>Â
>Â Â - mistakes happen. Deal with it. Having tons of rules just makes themÂ
>Â Â Â Â more likely. Expect mistakes, and make sure they are fixed quickly.
>Â
> Is that "stated preference" enough?
>Â
drm/amdgpu: start using graphics object ids from DAL.
Makes sense to me. Reviewed-by: Harry Wentland Harry On 2016-04-14 04:25 AM, Christian König wrote: > Am 14.04.2016 um 04:56 schrieb Dave Airlie: >> DAL has a concept of storing the graphics object ids in a special >> small struct, and adding type safety to them. >> >> I'm starting to contemplate bringing some pieces of DAL into the >> mainline modesetting code (like the bios parser for a start), >> and I think this is the best first step in that direction. >> >> This series converts connectors, encoders and crtcs id to the >> DAL objects. I haven't done PLLs yet they are a bit messier and >> I probably need to think about them a bit more. >> >> Also DAL doesn't support any of the DVO/external encoders, I've no idea >> if they even exist on DCE8 or newer GPUs, so I've done a separate >> patch to drop them. > > I'm not so deep into the display engine stuff or DAL, but this looks > like a good start to me to better integrate some of the DAL code into > the existing code base. > > So the patches are Acked-by: Christian König . > > I will ask around about the DVO stuff. Let me know if you need > anything else while Alex is still asleep. > > Regards, > Christian. > >> Dave. >> >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 94933] [RV610] Freeze when turning on an external monitor after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=94933 --- Comment #5 from Michel D�nzer --- The X desktop size changes from 1280x800 to 3200x1080 around the time the GPU hangs. Can you also reproduce the problem by only changing the desktop size, without changing anything about the external monitor, e.g. using something like xrandr --fb 3200x1080 ? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/7d0566e7/attachment.html>
[PATCH] drm/dp/mst: Restore primary hub guid on resume
Patch makes sense to me. It looks like when the receiver detects an
upstream disconnect it will reset its internal state, at least somewhat.
We've seen that happen when system enters S3 and GPU loses power.
Reviewed-by: Harry Wentland
Harry
On 2016-04-13 04:50 PM, Lyude wrote:
> Some hubs are forgetful, and end up forgetting whatever GUID we set
> previously after we do a suspend/resume cycle. This can lead to
> hotplugging breaking (along with probably other things) since the hub
> will start sending connection notifications with the wrong GUID. As
> such, we need to check on resume whether or not the GUID the hub is
> giving us is valid.
>
> Signed-off-by: Lyude
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 27fbd79..d2efd78 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2121,6 +2121,8 @@ int drm_dp_mst_topology_mgr_resume(struct
> drm_dp_mst_topology_mgr *mgr)
>
> if (mgr->mst_primary) {
> int sret;
> + u8 guid[16];
> +
> sret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
> DP_RECEIVER_CAP_SIZE);
> if (sret != DP_RECEIVER_CAP_SIZE) {
> DRM_DEBUG_KMS("dpcd read failed - undocked during
> suspend?\n");
> @@ -2135,6 +2137,16 @@ int drm_dp_mst_topology_mgr_resume(struct
> drm_dp_mst_topology_mgr *mgr)
> ret = -1;
> goto out_unlock;
> }
> +
> + /* Some hubs forget their guids after they resume */
> + sret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16);
> + if (sret != 16) {
> + DRM_DEBUG_KMS("dpcd read failed - undocked during
> suspend?\n");
> + ret = -1;
> + goto out_unlock;
> + }
> + drm_dp_check_mstb_guid(mgr->mst_primary, guid);
> +
> ret = 0;
> } else
> ret = -1;
[Bug 94933] [RV610] Freeze when turning on an external monitor after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=94933 --- Comment #4 from Nicola Mori --- The freeze is only related to X, I guess. If, after triggering it, I'm quick enough to switch to a tty then the system is still responsive, I can log in and input commands to the shell (this is how I got the dmesg after the freeze). If instead I wait a bit after the freeze then the keyboard and the mouse become unresponsive, and pressing Ctrl+Alt+F2 has no effect. The system gets stuck with a black screen, the mouse cursor can still be moved around but clicks have no effect. The same happens when switching back to X from the tty after the "quick switch to tty" mentioned above. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/587dd145/attachment-0001.html>
[Bug 94933] [RV610] Freeze when turning on an external monitor after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=94933 --- Comment #3 from Nicola Mori --- Created attachment 122959 --> https://bugs.freedesktop.org/attachment.cgi?id=122959&action=edit Xorg.0.log -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/a6e0fa12/attachment-0001.html>
[Bug 94933] [RV610] Freeze when turning on an external monitor after resume from suspend
https://bugs.freedesktop.org/show_bug.cgi?id=94933 --- Comment #2 from Nicola Mori --- Created attachment 122958 --> https://bugs.freedesktop.org/attachment.cgi?id=122958&action=edit dmesg -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160415/635f03d7/attachment.html>
[git pull] drm fixes
Hi Linus, This contains fixes for exynos, amdgpu, radeon, i915 and qxl. It also contains some fixes to the core drm edid parser. The qxl fix for a cursor hotspot issue, and the radeon are some MST fixes that I've been running locally and make my monitor a bit happier. The exynos ones fix some regressions and build fixes. amdgpu has a couple of small fixes. and i915 has two DP MST fixes and a couple of other regression fixes. Nothing to out of the ordinary or surprising at this point. Dave. The following changes since commit bf16200689118d19de1b8d2a3c314fc21f5dc7bb: Linux 4.6-rc3 (2016-04-10 17:58:30 -0700) are available in the git repository at: git://people.freedesktop.org/~airlied/linux drm-fixes for you to fetch changes up to ff3e84e8e479c3ba7148f8dc35a56cf091ab56d9: Merge branch 'exynos-drm-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-fixes (2016-04-14 13:06:19 +1000) Andrzej Hajda (2): drm/exynos: fix adjusted_mode pointer in exynos_plane_mode_set drm/exynos: build fbdev code conditionally Arnd Bergmann (1): drm/exynos: fix error handling in exynos_drm_subdrv_open Bjørn Mork (1): drm/i915: fix deadlock on lid open Chris Wilson (1): drm/i915: Exit cherryview_irq_handler() after one pass Chunming Zhou (1): drm/amdgpu: add invisible pin size statistic Dan Carpenter (2): drm/exynos: mic: fix an error code drm/exynos: fix a warning message Dave Airlie (6): drm/radeon/mst: port some MST setup code from DAL. drm/radeon: use helper for mst connector dpms. Merge tag 'topic/drm-fixes-2016-04-07' of git://anongit.freedesktop.org/drm-intel into drm-fixes Merge tag 'drm-intel-fixes-2016-04-07' of git://anongit.freedesktop.org/drm-intel into drm-fixes Merge branch 'drm-fixes-4.6' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Merge branch 'exynos-drm-fixes' of git://git.kernel.org/.../daeinki/drm-exynos into drm-fixes Javier Martinez Canillas (1): drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency John Keeping (1): drm/qxl: fix cursor position with non-zero hotspot Junwei Zhang (1): drm/amd/amdgpu: fix irq domain remove for tonga ih Lyude (2): drm/i915: Fix race condition in intel_dp_destroy_mst_connector() drm/i915: Call intel_dp_mst_resume() before resuming displays Marek Szyprowski (1): drm/exynos: fimd: fix broken dp_clock control Paul Parsons (3): drm/edid: Fix EDID Established Timings I and II drm/edid: Fix parsing of EDID 1.4 Established Timings III descriptor drm/edid: Fix DMT 1024x768 at 43Hz (interlaced) timings drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 drivers/gpu/drm/amd/amdgpu/tonga_ih.c | 2 +- drivers/gpu/drm/drm_edid.c | 10 +- drivers/gpu/drm/exynos/Kconfig | 2 +- drivers/gpu/drm/exynos/Makefile| 6 +++--- drivers/gpu/drm/exynos/exynos_drm_core.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_fb.c | 11 --- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 11 +++ drivers/gpu/drm/exynos/exynos_drm_fbdev.h | 23 +- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_mic.c| 3 ++- drivers/gpu/drm/exynos/exynos_drm_plane.c | 12 +++- drivers/gpu/drm/i915/i915_drv.c| 4 ++-- drivers/gpu/drm/i915/i915_irq.c| 4 ++-- drivers/gpu/drm/i915/intel_dp_mst.c| 6 ++ drivers/gpu/drm/i915/intel_lvds.c | 5 + drivers/gpu/drm/qxl/qxl_display.c | 13 + drivers/gpu/drm/qxl/qxl_drv.h | 2 ++ drivers/gpu/drm/radeon/ni_reg.h| 2 ++ drivers/gpu/drm/radeon/radeon_dp_mst.c | 31 +++--- 22 files changed, 104 insertions(+), 62 deletions(-)
