date:20180220

Re: [PATCH RFC] drm/bridge: panel: Add device_link between panel and master drm device

2018-02-20 Thread Lukas Wunner

On Wed, Feb 21, 2018 at 12:21:50AM +0200, Jyri Sarha wrote:
> @@ -94,6 +114,8 @@ static void panel_bridge_detach(struct drm_bridge *bridge)
>   struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
>  
>   drm_panel_detach(panel_bridge->panel);
> +
> + device_link_del(panel_bridge->link);

No, you've set the DL_FLAG_AUTOREMOVE flag, so you'll end up removing
the link twice, which oopses.

It's either DL_FLAG_AUTOREMOVE or device_link_del(), not both.

> +static int panel_bridge_link_to_master(struct panel_bridge *panel_bridge)
> +{
> + struct device *mdev = panel_bridge->bridge.dev->dev;
> + struct device *pdev = panel_bridge->panel->dev;
> + u32 flags = DL_FLAG_AUTOREMOVE;
> +
> + panel_bridge->link = device_link_add(mdev, pdev, flags);
> + if (!panel_bridge->link) {
> + dev_err(pdev, "failed to link panel %s to %s\n",
> + dev_name(pdev), dev_name(mdev));

You're printing two instances of pdev's name in the log message,
one should be sufficient.

Also, you've mixed up the order: mdev is the consumer, pdev the
supplier.

(Bikeshed:  The DL_FLAG_AUTOREMOVE would still fit within 80 chars
on the line with device_link_add() and the flags variable wouldn't
have to be declared then.  Your call.)

Thanks,

Lukas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tve200: fix kernel-doc documentation comment include

2018-02-20 Thread Jani Nikula

On Wed, 21 Feb 2018, Linus Walleij  wrote:
> On Tue, Feb 20, 2018 at 3:20 PM, Jani Nikula  wrote:
>
>> The DOC: line acts as an identifier for the :doc: include. Fixes:
>>
>> ./drivers/gpu/drm/tve200/tve200_drv.c:1: warning: no structured comments 
>> found
>>
>> Cc: Linus Walleij 
>> Signed-off-by: Jani Nikula 
>
> Whoa, good catch!
>
> Reviewed-by: Linus Walleij 

Pushed to drm-misc-fixes, thanks for the review.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC] drm/bridge: panel: Add module_get/but calls to attached panel driver

2018-02-20 Thread Lukas Wunner

[+cc Rafael]

On Tue, Feb 20, 2018 at 05:04:00PM +0200, Jyri Sarha wrote:
> On 20/02/18 14:03, Thierry Reding wrote:
> > On Tue, Feb 20, 2018 at 01:28:48PM +0200, Jyri Sarha wrote:
> >> On 20/02/18 12:34, Thierry Reding wrote:
>  On Mon, Feb 19, 2018 at 10:06:22PM +0200, Jyri Sarha wrote:
> > Currently there is no way for a master drm driver to protect against an
> > attached panel driver from being unloaded while it is in use. The
> > least we can do is to indicate the usage by incrementing the module
> > reference count.
[...]
> >>> I disagree. module_get() is only going to protect you from unloading a
> >>> module that's in use, but there are other ways to unbind a driver from
> >>> the device and cause subsequent mayhem.
> >>>
> >>> struct device_link was "recently" introduced to fix that issue.
> >>
> >> Unfortunately I do not have time to do full fix for the issue, but in
> >> any case I think this patch is a step to the right direction. The module
> >> reference count should anyway be increased at least to know that the
> >> driver code remains in memory, even if the device is not there any more.
> > 
> > I think device links are actually a lot easier to use and give you a
> > full solution for this. Essentially the only thing you need to do is add
> > a call to device_link_add() in the correct place.
> > 
> > That said, it seems to me like drm_panel_bridge_add() is not a good
> > place to add this, because the panel/bridge does not follow a typical
> > consumer/supplier model. It is rather a helper construct with a well-
> > defined lifetime.
> > 
> > What you do want to protect is the panel (the "parent" of the panel/
> > bridge) from going away unexpectedly. I think the best way to achieve
> > that is to make the link in drm_panel_attach() with something like
> > this:
> > 
> > u32 flags = DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE;
> > struct device *consumer = connector->dev->dev;
> > 
> > link = device_link_add(consumer, panel->dev, flags);
> > if (!link) {
> > dev_err(panel->dev, "failed to link panel %s to %s\n",
> > dev_name(panel->dev), dev_name(consumer));
> > return -EINVAL;
> > }
> > 
> > Ideally I think we'd link the panel to the connector rather than the
> > top-level DRM device, but we don't have a struct device * for that, so
> > this is probably as good as it gets for now.
> 
> Thanks for the hint. Adding the link indeed unbinds the master drm
> device when the panel is unloaded without any complaints.
> 
> The annoying thing is that the master drm device does not probe again
> and bind when the supplier device becomes available again. However,
> forcing a manual bind brings the whole device back without a problem.
> 
> Is there any trivial way to fix this?

How about the below, would this work for you?

Or is the device link added in the consumer's driver, hence is gone when
the consumer is unbound?  If so, it should be added somewhere else,
or it shouldn't be removed on consumer unbind.

@Thierry:  Thanks for shifting the discussion in the right direction,
it's good to see device links getting more adoption, instead of
proliferating yet more kludges.  However I don't understand why you say
we don't have a struct device for the connector, drm_sysfs_connector_add()
does create one.

-- >8 --
Subject: [PATCH] driver core: Ensure consumers are probed when supplier is
 bound

When a device link supplier is unbound (either manually or because one
of its own suppliers was unbound), its consumers are unbound as well.

However when that supplier is subsequently rebound, its consumers should
likewise be given a chance to rebind.  Achieve that by putting them on
the deferred probe list in device_links_driver_bound().  The sole caller
of that function, driver_bound(), triggers deferred probing afterwards.

Reported-by: Jyri Sarha 
Cc: Thierry Reding 
Cc: Rafael J. Wysocki 
Signed-off-by: Lukas Wunner 
---
 drivers/base/base.h | 1 +
 drivers/base/core.c | 4 +++-
 drivers/base/dd.c   | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index d800de6..39370eb 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -114,6 +114,7 @@ extern void device_release_driver_internal(struct device 
*dev,
 
 extern void driver_detach(struct device_driver *drv);
 extern int driver_probe_device(struct device_driver *drv, struct device *dev);
+extern void driver_deferred_probe_add(struct device *dev);
 extern void driver_deferred_probe_del(struct device *dev);
 static inline int driver_match_device(struct device_driver *drv,
  struct device *dev)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 920af22..2869d21 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -402,7 +402,8 @@ int device_links_check_suppliers(struct

Update: Vulkan modifiers extension VK_EXT_image_drm_format_modifier

2018-02-20 Thread Chad Versace

As many of you know, I've been writing a Vulkan extension for DRM format
modifiers, named VK_EXT_image_drm_format_modifier.

The extension is very close to completion. I've submitted a branch to
Khronos for review. It's receiving active review inside Khronos from
some non-Mesa closed-source window-system-integration people, and Mesa
people too (namely jekstrand).

You should take a look at the spec if you care about modifiers and Vulkan.
I try to keep up-to-date urls to everything related to this extension at
.

There remain only two unresolved issues from my perspective:

- The exact definition of members of the array
VkImageDrmFormatModifierExplicitCreateInfoEXT::pPlaneLayouts.
Should the extension re-use VkSubresourceLayout as the array
members? Or should it define a new struct with less fields than
VkSubresourceLayout?

- If an image has a modifier that requires an extra plane (such as
a color-compression plane), should the extension allow such an
image to be disjoint? Specifically, if a modifier requires an
extra plane, should the extension allow the modifier's
drmFormatModifierTilingFeatures to contain
VK_FORMAT_FEATURE_DISJOINT_KHR?

I've tentatively concluded "no": images with extra planes must be
non-disjoint. Though we could lift this restriction in a future
extension.

Branches

I maintain a public branch of the Vulkan spec, branch
1.0-VK_EXT_image_drm_format_modifier, which is synchronized with the
Khronos-internal branch of the same name. I like cgit; other people like
Github; so I keep a mirror at both.

cgit:
http://git.kiwitree.net/cgit/~chadv/vulkan-spec/log?h=1.0-VK_EXT_image_drm_format_modifier
github:
https://github.com/chadversary/vulkan-spec/commits/1.0-VK_EXT_image_drm_format_modifier
khronos-internal:
https://gitlab.khronos.org/vulkan/vulkan/merge_requests/2555

Prebuilt Specs
==
I maintain a public build of the branch. The built headers and HTML
specification are synchronized with the git branch thanks to the magic
of shell scripts.

vulkan.h:
http://git.kiwitree.net/cgit/~chadv/vulkan-spec/tree/src/vulkan/vulkan.h?h=1.0-VK_EXT_image_drm_format_modifier
spec appendix:
http://kiwitree.net/~chadv/vulkan/1.0-VK_EXT_image_drm_format_modifier/html/vkspec.html#VK_EXT_image_drm_format_modifier
full spec:
http://kiwitree.net/~chadv/vulkan/1.0-VK_EXT_image_drm_format_modifier/html/vkspec.html

Where to start reading
==
Here's a short reading guide for people unfamiliar with the extension:

- Don't start with the specification. You'll quickly get lost.

- First, read the VK_EXT_external_memory_dma_buf and
VK_EXT_queue_family_foreign extensions. They're small extensions.
They're intended to be used with VK_EXT_image_drm_format_modifier.
Despite that intent, the the three extensions are independent from
the Vulkan specification's perspective.

- Read the appendix chapter for VK_EXT_image_drm_format_modifier.
I've written an "introduction to modifiers" section in the
appendix. If you're already intimately understand modifiers, then
you can briefly scan this section, skipping over the boring stuff.

- Read the issue section in the appendix.

- Now for the tofu. Study the new structs and functions. Find them
under `#define VK_EXT_image_drm_format_modifier` in vulkan.h.
Also, study the new enums values. Find them by grepping 'DRM.*EXT'
in vulkan.h.

- Finally, go read the specification text for the new structs,
functions, and enums.

How to send feedback

I honestly don't know. You _could_ comment on the merge request
in the Khronos-internal Gitlab. But you probably (and rightfully so)
want to keep the discussion public.

You could provide general feedback by replying to this thread.

You could leave comments on my Github branch. I don't like Github, but
I can't think of a better solution, other than...

I could send my specification patches to mesa-dev. If people want that,
say so.

So... yeah. I don't know how you should provide feedback. Just do it,
and we'll iron out any problems as they arise.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/pl111: Remove reverse dependency on DRM_DUMB_VGA_DAC

2018-02-20 Thread Linus Walleij

On Tue, Feb 20, 2018 at 11:29 AM, Thierry Reding
 wrote:

> From: Thierry Reding 
>
> DRM_DUMB_VGA_DAC is a user-visible symbol. Selecting it can cause unmet
> direct dependencies such as this (on i386, randconfig):
>
> warning: (DRM_PL111) selects DRM_DUMB_VGA_DAC which has unmet direct 
> dependencies (HAS_IOMEM && DRM && DRM_BRIDGE && OF)
>
> This is because DRM_DUMB_VGA_DAC depends on OF while DRM_PL111 does not.
> It does indirectly depend on OF via the ARM and ARM64 dependencies, but
> since it can also be enabled under COMPILE_TEST, randconfig can find a
> case where DRM_PL111 is selected without pulling in OF and not meeting
> the dependency for DRM_DUMB_VGA_DAC.
>
> Since select is "heavy handed", DRM_DUMB_VGA_DAC is going to be enabled
> regardless of the above warning and causes the following build error:
>
> ../drivers/gpu/drm/bridge/dumb-vga-dac.c: In function 
> 'dumb_vga_probe':
> ../drivers/gpu/drm/bridge/dumb-vga-dac.c:207:13: error: 'struct 
> drm_bridge' has no member named 'of_node'
>   vga->bridge.of_node = pdev->dev.of_node;
>
> See Documentation/kbuild/kconfig-language.txt, "reverse dependencies".
>
> Fixes: 49f81d80ab84 ("drm/pl111: Support handling bridge timings")
> Reported-by: Randy Dunlap 
> Cc: Laurent Pinchart 
> Cc: Archit Taneja 
> Cc: Eric Anholt 
> Signed-off-by: Thierry Reding 

OK sorry about that, I just get to let the platforms that need the bridge
select it explicitly.
Reviewed-by: Linus Walleij 

Yours,
Linus Walleij
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tve200: fix kernel-doc documentation comment include

2018-02-20 Thread Linus Walleij

On Tue, Feb 20, 2018 at 3:20 PM, Jani Nikula  wrote:

> The DOC: line acts as an identifier for the :doc: include. Fixes:
>
> ./drivers/gpu/drm/tve200/tve200_drv.c:1: warning: no structured comments found
>
> Cc: Linus Walleij 
> Signed-off-by: Jani Nikula 

Whoa, good catch!

Reviewed-by: Linus Walleij 

Yours,
Linus Walleij
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Mesa-dev] Allocator Nouveau driver, Mesa EXT_external_objects, and DRM metadata import interfaces

2018-02-20 Thread Chad Versace

On Thu 21 Dec 2017, Daniel Vetter wrote:
> On Thu, Dec 21, 2017 at 12:22 AM, Kristian Kristensen  
> wrote:
>> On Wed, Dec 20, 2017 at 12:41 PM, Miguel Angel Vico  
>> wrote:
>>> On Wed, 20 Dec 2017 11:54:10 -0800 Kristian Høgsberg  
>>> wrote:
 I'd like to see concrete examples of actual display controllers
 supporting more format layouts than what can be specified with a 64
 bit modifier.
>>>
>>> The main problem is our tiling and other metadata parameters can't
>>> generally fit in a modifier, so we find passing a blob of metadata a
>>> more suitable mechanism.
>>
>> I understand that you may have n knobs with a total of more than a total of
>> 56 bits that configure your tiling/swizzling for color buffers. What I don't
>> buy is that you need all those combinations when passing buffers around
>> between codecs, cameras and display controllers. Even if you're sharing
>> between the same 3D drivers in different processes, I expect just locking
>> down, say, 64 different combinations (you can add more over time) and
>> assigning each a modifier would be sufficient. I doubt you'd extract
>> meaningful performance gains from going all the way to a blob.

I agree with Kristian above. In my opinion, choosing to encode in
modifiers a precise description of every possible tiling/compression
layout is not technically incorrect, but I believe it misses the point.
The intention behind modifiers is not to exhaustively describe all
possibilites.

I summarized this opinion in VK_EXT_image_drm_format_modifier,
where I wrote an "introdution to modifiers" section. Here's an excerpt:

One goal of modifiers in the Linux ecosystem is to enumerate for each
vendor a reasonably sized set of tiling formats that are appropriate for
images shared across processes, APIs, and/or devices, where each
participating component may possibly be from different vendors.
A non-goal is to enumerate all tiling formats supported by all vendors.
Some tiling formats used internally by vendors are inappropriate for
sharing; no modifiers should be assigned to such tiling formats.

> Tegra just redesigned it's modifier space from an ungodly amount of
> bits to just a few layouts. Not even just the ones in used, but simply
> limiting to the ones that make sense (there's dependencies apparently)
> Also note that the modifier alone doesn't need to describe the layout
> precisely, it only makes sense together with a specific pixel format
> and size. E.g. a bunch of the i915 layouts change layout depending
> upon bpp.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

1 2 >

1 - 100 of 198 matches

Mail list logo