Re: [RFC 1/3] drm: Add DAMAGE_CLIPS property to plane

[Thomas Hellstrom]

On 04/05/2018 12:03 PM, Daniel Vetter wrote:

On Thu, Apr 05, 2018 at 11:00:15AM +0200, Thomas Hellstrom wrote:

On 04/05/2018 09:35 AM, Daniel Vetter wrote:

On Wed, Apr 04, 2018 at 04:49:06PM -0700, Deepak Rawat wrote:

From: Lukasz Spintzyk 

Optional plane property to mark damaged regions on the plane in
framebuffer coordinates of the framebuffer attached to the plane.

The layout of blob data is simply an array of drm_mode_rect with maximum
array size limited by DRM_MODE_FB_DIRTY_MAX_CLIPS. Unlike plane src
coordinates, damage clips are not in 16.16 fixed point.

Damage clips are a hint to kernel as which area of framebuffer has
changed since last page-flip. This should be helpful for some drivers
especially for virtual devices where each framebuffer change needs to
be transmitted over network, usb, etc.

Driver which are interested in enabling DAMAGE_CLIPS property for a
plane should enable this property using drm_plane_enable_damage_clips.

Signed-off-by: Lukasz Spintzyk 
Signed-off-by: Deepak Rawat 

The property uapi section is missing, see:

https://urldefense.proofpoint.com/v2/url?u=https-3A__dri.freedesktop.org_docs_drm_gpu_drm-2Dkms.html-23plane-2Dcomposition-2Dproperties=DwIBAg=uilaK90D4TOVoH58JNXRgQ=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ=J31YNV1uz3IVRaorM-eo48msi9__sQurqRZrig2UE1s=vzsuquF1agbOw54HSC_18Kk2k9j83m8RusmDCtPAFWQ=

Plane composition feels like the best place to put this. Please use that
section to tie all the various bits together, including the helpers you're
adding in the following patches for drivers to use.

Bunch of nitpicks below, but overall I'm agreeing now with just going with
fb coordinate damage rects.

Like you say, the thing needed here now is userspace + driver actually
implementing this. I'd also say the compat helper to map the legacy
fb->dirty to this new atomic way of doing things should be included here
(gives us a lot more testing for these new paths).

Icing on the cake would be an igt to make sure kernel rejects invalid clip
rects correctly.


---
   drivers/gpu/drm/drm_atomic.c| 42 
+
   drivers/gpu/drm/drm_atomic_helper.c |  4 
   drivers/gpu/drm/drm_mode_config.c   |  5 +
   drivers/gpu/drm/drm_plane.c | 12 +++
   include/drm/drm_mode_config.h   | 15 +
   include/drm/drm_plane.h | 16 ++
   include/uapi/drm/drm_mode.h | 15 +
   7 files changed, 109 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7d25c42..9226d24 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -669,6 +669,40 @@ static void drm_atomic_crtc_print_state(struct drm_printer 
*p,
   }
   /**
+ * drm_atomic_set_damage_for_plane - sets the damage clips property to plane
+ * @state: plane state
+ * @blob: damage clips in framebuffer coordinates
+ *
+ * Returns:
+ *
+ * Zero on success, error code on failure.
+ */
+static int drm_atomic_set_damage_for_plane(struct drm_plane_state *state,
+  struct drm_property_blob *blob)
+{
+   if (blob == state->damage_clips)
+   return 0;
+
+   drm_property_blob_put(state->damage_clips);
+   state->damage_clips = NULL;
+
+   if (blob) {
+   uint32_t count = blob->length/sizeof(struct drm_rect);
+
+   if (count > DRM_MODE_FB_DIRTY_MAX_CLIPS)
+   return -EINVAL;
+
+   state->damage_clips = drm_property_blob_get(blob);
+   state->num_clips = count;
+   } else {
+   state->damage_clips = NULL;
+   state->num_clips = 0;
+   }
+
+   return 0;
+}
+
+/**
* drm_atomic_get_plane_state - get plane state
* @state: global atomic state object
* @plane: plane to get state object for
@@ -793,6 +827,12 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
state->color_encoding = val;
} else if (property == plane->color_range_property) {
state->color_range = val;
+   } else if (property == config->prop_damage_clips) {
+   struct drm_property_blob *blob =
+   drm_property_lookup_blob(dev, val);
+   int ret = drm_atomic_set_damage_for_plane(state, blob);

There's already a helper with size-checking built-in, see
drm_atomic_replace_property_blob_from_id(). Wrt computing num_clips I'd
just provide a little inline helper that does the
blob->length/sizeof(drm_rect) conversion (it's just a shift, so fast).


+   drm_property_blob_put(blob);
+   return ret;
} else if (plane->funcs->atomic_set_property) {
return plane->funcs->atomic_set_property(plane, state,
property, val);
@@ -856,6 +896,8 @@ drm_atomic_plane_get_property(struct 

Re: Fwd: DRM MIPI DSI device and I2C device?

[Andrzej Hajda]

On 05.04.2018 12:28, Laurent Pinchart wrote:
> Hi Carsten,
>
> On Wednesday, 4 April 2018 11:41:05 EEST Carsten Behling wrote:
>>> Hi,
>>>
>>> I would like to write a DRM bridge driver that is an I2C device and a DRM
>>> MIPI DSI device.
>>>
>>> It looks like that both - 'i2c-core.c: of_i2c_register_devices' and
>>> 'drm_mipi_dsi.c: mipi_dsi_host_register'  are registering their devices by
>>> iterating over devicetree child nodes with
>>> for_each_available_child_of_node.
>>>
>>> Since I can't make the bridge a child node of both, I don't know how to
>>> resolve it.
>> Found the answer myself. adv7533 driver is a good example. Devicetree
>> exists for qcom apq8016-sbc. No need to make the bridge a MIPI DSI child
>> node.
> This is an issue that has largely been ignored so far in Linux. Both DT and 
> the Linux kernel device model organize devices in a tree structure based on 
> the control buses. Devices that are connected to multiple control buses 
> haven't been taken into account in the design and are thus hard to support.
>
> As you may know, DSI can work in command mode or data mode. In data mode the 
> DSI bus is only use to transfer video data, while in command mode it is also 
> used to control the device (reading and writing registers).

I am not sure what you mean by data and command mode. MIPI DSI specs
says about video and command mode - modes to transfer video data. In
both cases DSI can be used to control the device.

>
> A DSI device operating in data mode and controlled through I2C isn't a 
> problem, as there's a single control bus in that case. The device should be a 
> child of the I2C controller in DT, and will be instantiated through 
> of_i2c_register_devices(). 
> A DSI device operating in command mode without any 
> other control bus isn't a problem either, it will be a child of the DSI 
> master 
> in DT, and will bee instantiated through mipi_dsi_host_register().
>
> A DSI device operating in command mode that also require configuration 
> through 
> a separate control bus (such as I2C, but also SPI) is much more problematic 
> to 
> support. Fortunately those devices are rare. Hopefully your device won't fall 
> in this category. If it does, we can discuss how to best support it.
>
As you have already found adv bridge is a good example. It is not clear
from the emails if DSI is used only to send video, or also to control?
And if to control, is it used to pass only specific commands

or can be used as alternative to i2c interface?


Regards

Andrzej



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

Re: [Patch v2 5/6] drm/omap: Add virtual plane support to omap_plane

[Tomi Valkeinen]

On 26/03/18 19:21, Benoit Parrot wrote:
> Add virtual wide plane support by adding an secondary
> plane_id so that an "omap_plane" can be composed of up to
> two physical planes.
> 
> When at least one 'plane' child node is present in DT then
> omap_plane_init will only use the plane described in DT.
> Some of these nodes may be a virtual wide plane if they are defined
> as two physical planes.
> Planes can also be associated with various crtcs independently.
> Therefore we can restrict the use of virtual plane to specific
> CRTC/video port if need be, if crtc_mask is not specified then
> the plane will be available to all available crtcs.
> Physical planes which are not described will essentially be hidden
> from the driver.
> 
> If no 'plane' child nodes exist then the normal plane
> allocation will take place.

The descs in many of the patches are a bit messy. How do you format
them? At least vim's autoformat gives a more consistent wrapping.

> Signed-off-by: Benoit Parrot 
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c   | 127 +++--
>  drivers/gpu/drm/omapdrm/omap_fb.c|  66 ++-
>  drivers/gpu/drm/omapdrm/omap_fb.h|   4 +-
>  drivers/gpu/drm/omapdrm/omap_plane.c | 151 
> ++-
>  drivers/gpu/drm/omapdrm/omap_plane.h |   4 +-
>  5 files changed, 283 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
> b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 37ee20c773c7..4c43ef239136 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -16,6 +16,7 @@
>   */
>  
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -116,6 +117,112 @@ static void omap_atomic_commit_tail(struct 
> drm_atomic_state *old_state)
>   priv->dispc_ops->runtime_put();
>  }
>  
> +static int omap_atomic_state_zpos_cmp(const void *a, const void *b)
> +{
> + const struct drm_plane_state *sa = *(struct drm_plane_state **)a;
> + const struct drm_plane_state *sb = *(struct drm_plane_state **)b;
> +
> + if (sa->zpos != sb->zpos)
> + return sa->zpos - sb->zpos;
> + else
> + return sa->plane->base.id - sb->plane->base.id;
> +}
> +
> +static int omap_atomic_crtc_normalize_zpos(struct drm_crtc *crtc,
> +struct drm_crtc_state *crtc_state)
> +{
> + struct drm_atomic_state *state = crtc_state->state;
> + struct drm_device *dev = crtc->dev;
> + int total_planes = dev->mode_config.num_total_plane;
> + struct drm_plane_state **states;
> + struct drm_plane *plane;
> + int i, inc, n = 0;
> + int ret = 0;
> +
> + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
> +  crtc->base.id, crtc->name);
> +
> + states = kmalloc_array(total_planes, sizeof(*states), GFP_KERNEL);
> + if (!states)
> + return -ENOMEM;
> +
> + /*
> +  * Normalization process might create new states for planes which
> +  * normalized_zpos has to be recalculated.
> +  */
> + drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
> + struct drm_plane_state *plane_state =
> + drm_atomic_get_plane_state(state, plane);
> + if (IS_ERR(plane_state)) {
> + ret = PTR_ERR(plane_state);
> + goto done;
> + }
> + states[n++] = plane_state;
> + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] processing zpos value %d\n",
> +  plane->base.id, plane->name,
> +  plane_state->zpos);
> + }
> +
> + sort(states, n, sizeof(*states), omap_atomic_state_zpos_cmp, NULL);
> +
> + for (inc = 0, i = 0; i < n; i++) {
> + plane = states[i]->plane;
> +
> + states[i]->normalized_zpos = i + inc;
> + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] normalized zpos value %d\n",
> +  plane->base.id, plane->name, i + inc);
> + /*
> +  * If the current plane is virtual it uses 2 hw planes
> +  * therefore the very next zpos is used by the secondary/aux
> +  * plane so we need to skip one zpos from this point on.
> +  */
> + if (is_omap_plane_virtual(plane))
> + inc++;
> + }
> + crtc_state->zpos_changed = true;
> +
> +done:
> + kfree(states);
> + return ret;
> +}
> +
> +static int omap_atomic_normalize_zpos(struct drm_device *dev,
> +   struct drm_atomic_state *state)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> + int i, ret = 0;
> +
> + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
> new_crtc_state, i) {
> + if (old_crtc_state->plane_mask != new_crtc_state->plane_mask ||
> + new_crtc_state->zpos_changed) {

Re: [PATCH] drm/sti: Depend on OF rather than selecting it

[Benjamin Gaignard]

2018-04-03 7:34 GMT+02:00 Oliver O'Halloran :
> Commit cc6b741c6f63 ("drm: sti: remove useless fields from vtg
> structure") reworked some code inside of this driver and made it select
> CONFIG_OF. This results in the entire OF layer being enabled when
> building an allmodconfig on ia64. OF on ia64 is completely unsupported
> so this isn't a great state of affairs.
>
> The 0day robot noticed a link-time failure on ia64 caused by
> using of_node_to_nid() in an otherwise unrelated driver. The
> generic fallback for of_node_to_nid() only exists when:
>
> defined(CONFIG_OF) && defined(CONFIG_NUMA) == false
>
> Since CONFIG_NUMA is usually selected for IA64 we get the link failure.
> Fix this by making the driver depend on OF rather than selecting it,
> odds are that was the original intent.
>
> Link: https://lists.01.org/pipermail/kbuild-all/2018-March/045172.html
> Fixes: cc6b741c6f63 ("drm: sti: remove useless fields from vtg structure")
> Cc: Benjamin Gaignard 
> Cc: Vincent Abriou 
> Cc: David Airlie 
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-i...@vger.kernel.org
> Cc: sta...@vger.kernel.org
> Signed-off-by: Oliver O'Halloran 

Reviewed-by: Benjamin Gaignard 

> ---
> Cc`ed to stable since the ia64 guys might want it backported. I'm not
> bothered if it just goes into mainline.
> ---
>  drivers/gpu/drm/sti/Kconfig | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
> index cca4b3c9aeb5..1963cc1b1cc5 100644
> --- a/drivers/gpu/drm/sti/Kconfig
> +++ b/drivers/gpu/drm/sti/Kconfig
> @@ -1,6 +1,6 @@
>  config DRM_STI
> tristate "DRM Support for STMicroelectronics SoC stiH4xx Series"
> -   depends on DRM && (ARCH_STI || ARCH_MULTIPLATFORM)
> +   depends on OF && DRM && (ARCH_STI || ARCH_MULTIPLATFORM)
> select RESET_CONTROLLER
> select DRM_KMS_HELPER
> select DRM_GEM_CMA_HELPER
> @@ -8,6 +8,5 @@ config DRM_STI
> select DRM_PANEL
> select FW_LOADER
> select SND_SOC_HDMI_CODEC if SND_SOC
> -   select OF
> help
>   Choose this option to enable DRM on STM stiH4xx chipset
> --
> 2.9.5
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: DRM_UDL and GPU under Xserver

[Daniel Vetter]

On Thu, Apr 5, 2018 at 12:29 PM, Lucas Stach  wrote:
> Am Donnerstag, den 05.04.2018, 11:32 +0200 schrieb Daniel Vetter:
>> On Thu, Apr 5, 2018 at 9:16 AM, Alexey Brodkin
>>  wrote:
>> > Hi Daniel,
>> >
>> > On Thu, 2018-04-05 at 08:18 +0200, Daniel Vetter wrote:
>> > > On Wed, Apr 4, 2018 at 10:06 PM, Alexey Brodkin
>> > >  wrote:
>> > > > Hello,
>> > > >
>> > > > We're trying to use DisplayLink USB2-to-HDMI adapter to render
>> > > > GPU-accelerated graphics.
>> > > > Hardware setup is as simple as a devboard + DisplayLink
>> > > > adapter.
>> > > > Devboards we use for this experiment are:
>> > > >  * Wandboard Quad (based on IMX6 SoC with Vivante GPU) or
>> > > >  * HSDK (based on Synopsys ARC HS38 SoC with Vivante GPU as
>> > > > well)
>> > > >
>> > > > I'm sure any other board with DRM supported GPU will work,
>> > > > those we just used
>> > > > as the very recent Linux kernels could be easily run on them
>> > > > both.
>> > > >
>> > > > Basically the problem is UDL needs to be explicitly notified
>> > > > about new data
>> > > > to be rendered on the screen compared to typical bit-streamers
>> > > > that infinitely
>> > > > scan a dedicated buffer in memory.
>> > > >
>> > > > In case of UDL there're just 2 ways for this notification:
>> > > >  1) DRM_IOCTL_MODE_PAGE_FLIP that calls drm_crtc_funcs-
>> > > > >page_flip()
>> > > >  2) DRM_IOCTL_MODE_DIRTYFB that calls drm_framebuffer_funcs-
>> > > > >dirty()
>> > > >
>> > > > But neither of IOCTLs happen when we run Xserver with xf86-
>> > > > video-armada driver
>> > > > (see https://urldefense.proofpoint.com/v2/url?u=http-3A__git.ar
>> > > > m.linux.org.uk_cgit_xf86-2Dvideo-2Darmada.git_log_-3Fh-
>> > > > 3Dunstable-2Ddevel=DwIBaQ&;
>> > > > c=DPL6_X_6JkXFx7AXWqB0tg=lqdeeSSEes0GFDDl656eViXO7breS55ytWkh
>> > > > pk5R81I=oEAlP64L9vkuUs_k3kGwwwlN1WJbDMJbCo0uDhwKwwk=3ZHj-
>> > > > 6JXZBLSTWg_4KMnL0VNi7z8c0RxHzj2U5ywVIw=).
>> > > >
>> > > > Is it something missing in Xserver or in UDL driver?
>> > >
>> > > Use the -modesetting driverr for UDL, that one works correctly.
>> >
>> > If you're talking about "modesetting" driver of Xserver [1] then
>> > indeed
>> > picture is displayed on the screen. But there I guess won't be any
>> > 3D acceleration.
>> >
>> > At least that's what was suggested to me earlier here [2] by Lucas:
>> > >8---
>> > For 3D acceleration to work under X you need the etnaviv specific
>> > DDX
>> > driver, which can be found here:
>> >
>> > http://git.arm.linux.org.uk/cgit/xf86-video-armada.git/log/?h=unsta
>> > ble-devel
>>
>> You definitely want to use -modesetting for UDL. And I thought with
>> glamour and the corresponding mesa work you should also get
>> accelaration. Insisting that you must use a driver-specific ddx is
>> broken, the world doesn't work like that anymore.
>
> On etnaviv the world definitely still works like this. The etnaviv DDX
> uses the dedicated 2D hardware of the Vivante GPUs, which is much
> faster and efficient than going through Glamor.
> Especially since almost all X accel operations are done on linear
> buffers, while the 3D GPU can only ever do tiled on both sampler and
> render, where some multi-pipe 3D cores can't even read the tiling they
> write out. So Glamor is an endless copy fest using the resolve engine
> on those.

Ah right, I've forgotten about the vivante 2d cores again.

> If using etnaviv with UDL is a use-case that need to be supported, one
> would need to port the UDL specifics from -modesetting to the -armada
> DDX.

I don't think this makes sense.

>> Lucas, can you pls clarify? Also, why does -armada bind against all
>> kms drivers, that's probaly too much.
>
> I think that's a local modification done by Alexey. The armada driver
> only binds to armada and imx-drm by default.

Yeah, that sounds a lot more reasonable than trying to teach -armada
about all the things -modesetting already knows to be able to be a
generic kms driver.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: rcar-du: track dma-buf fences

[Daniel Vetter]

On Wed, Apr 4, 2018 at 12:59 PM, Ucan, Emre (ADITG/ESB)
 wrote:
> Hello Laurent
>
> Thank you for your review.
>
>> -Original Message-
>> From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
>> Sent: Dienstag, 3. April 2018 20:53
>> To: Ucan, Emre (ADITG/ESB)
>> Cc: dri-devel@lists.freedesktop.org
>> Subject: Re: [PATCH] drm: rcar-du: track dma-buf fences
>>
>> Hello Emre,
>>
>> Thank you for the patch.
>>
>> On Tuesday, 3 April 2018 12:14:33 EEST Emre Ucan wrote:
>> > We have to check dma-buf reservation objects
>> > of our framebuffers before we use them.
>> > Otherwise, another driver might be writing
>> > on the same buffer which we are using.
>> > This would cause visible tearing effects
>> > on display.
>> >
>> > We can use existing atomic helper functions
>> > to solve this problem.
>> >
>> > Signed-off-by: Emre Ucan 
>> > ---
>> >  drivers/gpu/drm/rcar-du/rcar_du_kms.c |  2 ++
>> >  drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 20 
>> >  2 files changed, 22 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 0329b35..f3da3d1 100644
>> > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> > @@ -255,6 +255,8 @@ static void rcar_du_atomic_commit_tail(struct
>> > drm_atomic_state *old_state) {
>> > struct drm_device *dev = old_state->dev;
>> >
>> > +   drm_atomic_helper_wait_for_fences(dev, old_state, false);
>> > +
>>
>> The commit_tail() function in drm_atomic_helper.c, which calls our
>> atomic_commit_tail() implementation, already calls
>> drm_atomic_helper_wait_for_fences(). Why is there a need to duplicate the
>> call
>> here ?
>
> You are right. I will remove it in second version.

You can use it in your own hook. Patch to update the kerneldoc to
clarify that would be great.
-Daniel

>
>>
>> > /* Apply the atomic update. */
>> > drm_atomic_helper_commit_modeset_disables(dev, old_state);
>> > drm_atomic_helper_commit_planes(dev, old_state,
>> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>> > b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 2c260c3..482e23c 100644
>> > --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
>> > @@ -18,12 +18,16 @@
>> >  #include 
>> >  #include 
>> >  #include 
>> > +#include 
>> >
>> >  #include 
>> >  #include 
>> > +#include 
>> > +#include 
>> >  #include 
>> >  #include 
>> >  #include 
>> > +#include 
>> >
>> >  #include 
>> >
>> > @@ -203,6 +207,20 @@ static void rcar_du_vsp_plane_setup(struct
>> > rcar_du_vsp_plane *plane) plane->index, );
>> >  }
>> >
>> > +static void rcar_du_vsp_set_fence_for_plane(struct drm_plane_state
>> *state)
>> > +{
>> > +   struct drm_gem_cma_object *gem;
>> > +   struct dma_buf *dma_buf;
>> > +   struct dma_fence *fence;
>> > +
>> > +   gem = drm_fb_cma_get_gem_obj(state->fb, 0);
>> > +   dma_buf = gem->base.dma_buf;
>> > +   if (dma_buf) {
>> > +   fence = reservation_object_get_excl_rcu(dma_buf->resv);
>> > +   drm_atomic_set_fence_for_plane(state, fence);
>>
>> Unless I'm mistaken this is used for implicit fencing only. What is your use
>> case, wouldn't it be better for userspace to use explicit fencing as that is
>> the fence model that has been selected for display ?
>
> We are using Weston on Renesas R-Car H3 SoC. I am using GPU rendered client 
> buffers
> directly as scanout buffer for the display. Weston is not using explicit 
> fencing.
>
>>
>> > +   }
>> > +}
>>
>> This looks very similar to drm_gem_fb_prepare_fb(), couldn't you use that
>> function instead ?
>
> Description of drm_gem_fb_prepare_fb() function states that it can be
> used as the _plane_helper_funcs.prepare_fb hook. But we have
> our own hook function which is called rcar_du_vsp_plane_prepare_fb().
> Therefore, I was not sure if it is correct to use drm_gem_fb_prepare_fb()
> inside our hook function.
>
> I will use it in the second version nevertheless.
>
>>
>> >  static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
>> > struct drm_plane_state *state)
>> >  {
>> > @@ -237,6 +255,8 @@ static int rcar_du_vsp_plane_prepare_fb(struct
>> drm_plane
>> > *plane, }
>> > }
>> >
>> > +   rcar_du_vsp_set_fence_for_plane(state);
>> > +
>> > return 0;
>> >
>> >  fail:
>>
>> --
>> Regards,
>>
>> Laurent Pinchart
>>
>>
>
> Best Regards,
>
> Emre Ucan
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Patch v2 6/6] drm/omap: Allow wider display when a virtual plane is available

[Tomi Valkeinen]

On 26/03/18 19:21, Benoit Parrot wrote:
> Add an exception case when filtering out display mode so that if
> a virtual wide plane is available then display wider than 2048 can be
> supported as long as the required timing parameters can also be met.
> 
> Signed-off-by: Benoit Parrot 
> ---
>  drivers/gpu/drm/omapdrm/omap_connector.c |  3 ++-
>  drivers/gpu/drm/omapdrm/omap_plane.c | 12 
>  drivers/gpu/drm/omapdrm/omap_plane.h |  1 +
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c 
> b/drivers/gpu/drm/omapdrm/omap_connector.c
> index d5e059abb555..517f7fa80ce1 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -203,7 +203,8 @@ static int omap_connector_mode_valid(struct drm_connector 
> *connector,
>   u16 width, height;
>  
>   priv->dispc_ops->ovl_get_max_size(, );
> - if (mode->hdisplay > width)
> + if (mode->hdisplay > width &&
> + !omap_have_any_virtual_plane(dev))

This doesn't sound correct. It's not whether we have any virtual planes,
but whether we have virtual planes for this display.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: DRM_UDL and GPU under Xserver

[Lucas Stach]

Am Donnerstag, den 05.04.2018, 11:32 +0200 schrieb Daniel Vetter:
> On Thu, Apr 5, 2018 at 9:16 AM, Alexey Brodkin
>  wrote:
> > Hi Daniel,
> > 
> > On Thu, 2018-04-05 at 08:18 +0200, Daniel Vetter wrote:
> > > On Wed, Apr 4, 2018 at 10:06 PM, Alexey Brodkin
> > >  wrote:
> > > > Hello,
> > > > 
> > > > We're trying to use DisplayLink USB2-to-HDMI adapter to render
> > > > GPU-accelerated graphics.
> > > > Hardware setup is as simple as a devboard + DisplayLink
> > > > adapter.
> > > > Devboards we use for this experiment are:
> > > >  * Wandboard Quad (based on IMX6 SoC with Vivante GPU) or
> > > >  * HSDK (based on Synopsys ARC HS38 SoC with Vivante GPU as
> > > > well)
> > > > 
> > > > I'm sure any other board with DRM supported GPU will work,
> > > > those we just used
> > > > as the very recent Linux kernels could be easily run on them
> > > > both.
> > > > 
> > > > Basically the problem is UDL needs to be explicitly notified
> > > > about new data
> > > > to be rendered on the screen compared to typical bit-streamers
> > > > that infinitely
> > > > scan a dedicated buffer in memory.
> > > > 
> > > > In case of UDL there're just 2 ways for this notification:
> > > >  1) DRM_IOCTL_MODE_PAGE_FLIP that calls drm_crtc_funcs-
> > > > >page_flip()
> > > >  2) DRM_IOCTL_MODE_DIRTYFB that calls drm_framebuffer_funcs-
> > > > >dirty()
> > > > 
> > > > But neither of IOCTLs happen when we run Xserver with xf86-
> > > > video-armada driver
> > > > (see https://urldefense.proofpoint.com/v2/url?u=http-3A__git.ar
> > > > m.linux.org.uk_cgit_xf86-2Dvideo-2Darmada.git_log_-3Fh-
> > > > 3Dunstable-2Ddevel=DwIBaQ&;
> > > > c=DPL6_X_6JkXFx7AXWqB0tg=lqdeeSSEes0GFDDl656eViXO7breS55ytWkh
> > > > pk5R81I=oEAlP64L9vkuUs_k3kGwwwlN1WJbDMJbCo0uDhwKwwk=3ZHj-
> > > > 6JXZBLSTWg_4KMnL0VNi7z8c0RxHzj2U5ywVIw=).
> > > > 
> > > > Is it something missing in Xserver or in UDL driver?
> > > 
> > > Use the -modesetting driverr for UDL, that one works correctly.
> > 
> > If you're talking about "modesetting" driver of Xserver [1] then
> > indeed
> > picture is displayed on the screen. But there I guess won't be any
> > 3D acceleration.
> > 
> > At least that's what was suggested to me earlier here [2] by Lucas:
> > >8---
> > For 3D acceleration to work under X you need the etnaviv specific
> > DDX
> > driver, which can be found here:
> > 
> > http://git.arm.linux.org.uk/cgit/xf86-video-armada.git/log/?h=unsta
> > ble-devel
> 
> You definitely want to use -modesetting for UDL. And I thought with
> glamour and the corresponding mesa work you should also get
> accelaration. Insisting that you must use a driver-specific ddx is
> broken, the world doesn't work like that anymore.

On etnaviv the world definitely still works like this. The etnaviv DDX
uses the dedicated 2D hardware of the Vivante GPUs, which is much
faster and efficient than going through Glamor.
Especially since almost all X accel operations are done on linear
buffers, while the 3D GPU can only ever do tiled on both sampler and
render, where some multi-pipe 3D cores can't even read the tiling they
write out. So Glamor is an endless copy fest using the resolve engine
on those.

If using etnaviv with UDL is a use-case that need to be supported, one
would need to port the UDL specifics from -modesetting to the -armada
DDX.

> Lucas, can you pls clarify? Also, why does -armada bind against all
> kms drivers, that's probaly too much.

I think that's a local modification done by Alexey. The armada driver
only binds to armada and imx-drm by default.

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

Re: Fwd: DRM MIPI DSI device and I2C device?

[Laurent Pinchart]

Hi Carsten,

On Wednesday, 4 April 2018 11:41:05 EEST Carsten Behling wrote:
> > Hi,
> > 
> > I would like to write a DRM bridge driver that is an I2C device and a DRM
> > MIPI DSI device.
> > 
> > It looks like that both - 'i2c-core.c: of_i2c_register_devices' and
> > 'drm_mipi_dsi.c: mipi_dsi_host_register'  are registering their devices by
> > iterating over devicetree child nodes with
> > for_each_available_child_of_node.
> > 
> > Since I can't make the bridge a child node of both, I don't know how to
> > resolve it.
> 
> Found the answer myself. adv7533 driver is a good example. Devicetree
> exists for qcom apq8016-sbc. No need to make the bridge a MIPI DSI child
> node.

This is an issue that has largely been ignored so far in Linux. Both DT and 
the Linux kernel device model organize devices in a tree structure based on 
the control buses. Devices that are connected to multiple control buses 
haven't been taken into account in the design and are thus hard to support.

As you may know, DSI can work in command mode or data mode. In data mode the 
DSI bus is only use to transfer video data, while in command mode it is also 
used to control the device (reading and writing registers).

A DSI device operating in data mode and controlled through I2C isn't a 
problem, as there's a single control bus in that case. The device should be a 
child of the I2C controller in DT, and will be instantiated through 
of_i2c_register_devices(). A DSI device operating in command mode without any 
other control bus isn't a problem either, it will be a child of the DSI master 
in DT, and will bee instantiated through mipi_dsi_host_register().

A DSI device operating in command mode that also require configuration through 
a separate control bus (such as I2C, but also SPI) is much more problematic to 
support. Fortunately those devices are rare. Hopefully your device won't fall 
in this category. If it does, we can discuss how to best support it.

-- 
Regards,

Laurent Pinchart



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

Re: [Patch v2 1/6] drm/omap: Add ability to filter out modes which can't be supported

[Tomi Valkeinen]

On 04/04/18 17:23, Laurent Pinchart wrote:

 +  WARN(out_width > dispc.feat->ovl_width_max,
 +   "Requested OVL width (%d) is larger than can be supported
 (%d).\n",
 +   out_width, dispc.feat->ovl_width_max);
>>>
>>> Why don't you return an error here? I don't see a need for WARN here.
>>
>> So here you mean replace the WARN with something like this:
>>
>>  if (out_width > dispc.feat->ovl_width_max) {
>>  DSSERR("Requested OVL width (%d) is larger than can be 
>> supported (%d).
> \n",
>> out_width, dispc.feat->ovl_width_max);
>> return -EINVAL;
>>  }
> 
> Can this happen ? If we reject invalid settings in omapdrm we should never 
> get 
> them here.

That's true. And we should check them in the plane atomic check (but do
we?).

In that case I don't mind a warn there, but you should still return an
error if it happens, instead of continuing with bad config.

 +  /* Check if the advertised width exceed what the pipeline can do */
 +  if (!r) {
 +  struct omap_drm_private *priv = dev->dev_private;
 +  u16 width, height;
 +
 +  priv->dispc_ops->ovl_get_max_size(, );
 +  if (mode->hdisplay > width)
 +  r = -EINVAL;
>>>
>>> You should check the height also.
>>
>> Yeah, I'll fix that.
> 
> Unless I'm mistaken the restriction doesn't come from the output side of the 
> display controller but from the overlays (planes), right ? Shouldn't it then 
> be implemented in the drm_plane_helper_funcs.atomic_check operation ?

Yes, but I don't so. If our planes can support up to, say, 1000. Then we
plug in a monitor with native width of 1100, which omapdrm would accept
happily and try to use it by default. But we can't show fbdev or any
normal setup there, because the planes won't support it. How would we
manage that?

While not strictly correct, I think it's fine to reject videomodes which
can't be shown with a normal full-screen plane.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: DRM_UDL and GPU under Xserver

[Jose Abreu]

Hi Alexey, Daniel,

On 05-04-2018 10:32, Daniel Vetter wrote:
> On Thu, Apr 5, 2018 at 9:16 AM, Alexey Brodkin
>  wrote:
>> Hi Daniel,
>>
>> On Thu, 2018-04-05 at 08:18 +0200, Daniel Vetter wrote:
>>> On Wed, Apr 4, 2018 at 10:06 PM, Alexey Brodkin
>>>  wrote:
 Hello,

 We're trying to use DisplayLink USB2-to-HDMI adapter to render 
 GPU-accelerated graphics.
 Hardware setup is as simple as a devboard + DisplayLink adapter.
 Devboards we use for this experiment are:
  * Wandboard Quad (based on IMX6 SoC with Vivante GPU) or
  * HSDK (based on Synopsys ARC HS38 SoC with Vivante GPU as well)

 I'm sure any other board with DRM supported GPU will work, those we just 
 used
 as the very recent Linux kernels could be easily run on them both.

 Basically the problem is UDL needs to be explicitly notified about new data
 to be rendered on the screen compared to typical bit-streamers that 
 infinitely
 scan a dedicated buffer in memory.

 In case of UDL there're just 2 ways for this notification:
  1) DRM_IOCTL_MODE_PAGE_FLIP that calls drm_crtc_funcs->page_flip()
  2) DRM_IOCTL_MODE_DIRTYFB that calls drm_framebuffer_funcs->dirty()

 But neither of IOCTLs happen when we run Xserver with xf86-video-armada 
 driver
 (see 
 https://urldefense.proofpoint.com/v2/url?u=http-3A__git.arm.linux.org.uk_cgit_xf86-2Dvideo-2Darmada.git_log_-3Fh-3Dunstable-2Ddevel=DwIBaQ;
 c=DPL6_X_6JkXFx7AXWqB0tg=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I=oEAlP64L9vkuUs_k3kGwwwlN1WJbDMJbCo0uDhwKwwk=3ZHj-
 6JXZBLSTWg_4KMnL0VNi7z8c0RxHzj2U5ywVIw=).

 Is it something missing in Xserver or in UDL driver?
>>> Use the -modesetting driverr for UDL, that one works correctly.
>> If you're talking about "modesetting" driver of Xserver [1] then indeed
>> picture is displayed on the screen. But there I guess won't be any 3D 
>> acceleration.
>>
>> At least that's what was suggested to me earlier here [2] by Lucas:
>> >8---
>> For 3D acceleration to work under X you need the etnaviv specific DDX
>> driver, which can be found here:
>>
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__git.arm.linux.org.uk_cgit_xf86-2Dvideo-2Darmada.git_log_-3Fh-3Dunstable-2Ddevel=DwIGaQ=DPL6_X_6JkXFx7AXWqB0tg=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY=NxAlhKaLZI6JlMs1pUBPHr79zFQ8ytECx0wtkVRrkeQ=9XSI0qYPrADUy1eUuKDexVOT98l9APph-ArYowGWwow=
> You definitely want to use -modesetting for UDL. And I thought with
> glamour and the corresponding mesa work you should also get
> accelaration. Insisting that you must use a driver-specific ddx is
> broken, the world doesn't work like that anymore.

I think what Alexey wants to do is not supported by -modesetting
driver. He wants to offload rendering to a Vivante GPU and then
display the result in *another* output ... For this I think full
PRIME support is needed, right? I see -modesetting has
drmPrimeFDToHandle but no drmPrimeHandleToFD support. In other
words -modesetting can not export buffers to another -modesetting
driver, it can only import them (?)

Thanks and Best Regards,
Jose Miguel Abreu

>
> Lucas, can you pls clarify? Also, why does -armada bind against all
> kms drivers, that's probaly too much.
> -Daniel
>
>> >8---
>>
>> [1] 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__cgit.freedesktop.org_xorg_xserver_tree_hw_xfree86_drivers_modesetting=DwIGaQ=DPL6_X_6JkXFx7AXWqB0tg=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY=NxAlhKaLZI6JlMs1pUBPHr79zFQ8ytECx0wtkVRrkeQ=yvnVItPaOgvVT8aJFwTO5XXLCCmlSiD89JwhcGeo7MI=
>> [2] 
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.infradead.org_pipermail_linux-2Dsnps-2Darc_2017-2DNovember_003031.html=DwIGaQ=DPL6_X_6JkXFx7AXWqB0tg=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY=NxAlhKaLZI6JlMs1pUBPHr79zFQ8ytECx0wtkVRrkeQ=8gdiQaxAN7AT_2vwaIpXpXfVPSPKPM275rlmcQZKu28=
>>
>> -Alexey
>
>

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

Re: [PATCH libdrm v2 14/23] meson: use simple option handling for omap

[Eric Engestrom]

On Wednesday, 2018-04-04 19:55:18 +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Wed, Apr 04, 2018 at 04:38:09PM +0100, Eric Engestrom wrote:
> > [...]
> >
> > -with_omap = false
> > -_omap = get_option('omap')
> > -if _omap == 'true'
> > -  if not with_atomics
> > -error('libdrm_omap requires atomics.')
> > -  endif
> > -  with_omap = true
> > +with_exynos = false
> > +_exynos = get_option('exynos')
> > +if _exynos == 'auto'
> > +  with_exynos = true
> > +else
> > +  with_exynos = _exynos == 'true'
> >  endif
> 
> Looks like some patch rebasing went wrong with this one (it
> simplifies omap, but also adds some exynos stuff)?

Indeed, that's a complete rebase fail...
Thanks for not letting me push this unaware :)

I'll send a v3 of 8-19 next week, after landing the first bits (1-7).
I think I'll defer 20-23 to after everything else has landed.

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

Re: [RFC 2/3] drm: Add helper iterator functions to iterate over plane damage.

[Daniel Vetter]

On Thu, Apr 05, 2018 at 10:49:09AM +0200, Thomas Hellstrom wrote:
> On 04/05/2018 09:52 AM, Daniel Vetter wrote:
> > On Wed, Apr 04, 2018 at 04:49:07PM -0700, Deepak Rawat wrote:
> > > With damage property in drm_plane_state, this patch adds helper iterator
> > > to traverse the damage clips. Iterator will return the damage rectangles
> > > in framebuffer, plane or crtc coordinates as need by driver
> > > implementation.
> > > 
> > > Signed-off-by: Deepak Rawat 
> > I'd really like selftest/unittests for this stuff. There's an awful lot of
> > cornercases in this here (for any of the transformed iterators at least),
> > and unit tests is the best way to make sure we handle them all correctly.
> > 
> > Bonus points if you integrate the new selftests into igt so intel CI can
> > run them, seel igt/tests/drm_mm.c for an example. drm_mm selftest is also
> > the framework I'd copy for this stuff.
> > 
> > > ---
> > >   drivers/gpu/drm/drm_atomic_helper.c | 122 
> > > 
> > >   include/drm/drm_atomic_helper.h |  39 
> > >   2 files changed, 161 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > index 55b44e3..355b514 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -3865,3 +3865,125 @@ void 
> > > __drm_atomic_helper_private_obj_duplicate_state(struct drm_private_obj 
> > > *obj
> > >   memcpy(state, obj->state, sizeof(*state));
> > >   }
> > >   EXPORT_SYMBOL(__drm_atomic_helper_private_obj_duplicate_state);
> > > +
> > > +/**
> > > + * drm_atomic_helper_damage_iter_init - initialize the damage iterator
> > > + * @iter: The iterator to initialize.
> > > + * @type: Coordinate type caller is interested in.
> > > + * @state: plane_state from which to iterate the damage clips.
> > > + * @hdisplay: Width of crtc on which plane is scanned out.
> > > + * @vdisplay: Height of crtc on which plane is scanned out.
> > > + *
> > > + * Initialize an iterator that is used to translate and clip a set of 
> > > damage
> > > + * rectangles in framebuffer coordinates to plane and crtc coordinates. 
> > > The type
> > > + * argument specify which type of coordinate to iterate in.
> > > + *
> > > + * Returns: 0 on success and negative error code on error. If an error 
> > > code is
> > > + * returned then it means the plane state should not update.
> > > + */
> > > +int
> > > +drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter 
> > > *iter,
> > > +enum drm_atomic_helper_damage_clip_type type,
> > > +const struct drm_plane_state *state,
> > > +uint32_t hdisplay, uint32_t vdisplay)
> > > +{
> > > + if (!state || !state->crtc || !state->fb)
> > > + return -EINVAL;
> > > +
> > > + memset(iter, 0, sizeof(*iter));
> > > + iter->clips = (struct drm_rect *)state->damage_clips->data;
> > > + iter->num_clips = state->num_clips;
> > > + iter->type = type;
> > > +
> > > + /*
> > > +  * Full update in case of scaling or rotation. In future support for
> > > +  * scaling/rotating damage clips can be added
> > > +  */
> > > + if (state->crtc_w != (state->src_w >> 16) ||
> > > + state->crtc_h != state->src_h >> 16 || state->rotation != 0) {
> > > + iter->curr_clip = iter->num_clips;
> > > + return 0;
> > Given there's no user of this I have no idea how this manages to provoke a
> > full clip rect. selftest code would be perfect for stuff like this.
> > 
> > Also, I think we should provide a full clip for the case of num_clips ==
> > 0, so that driver code can simply iterate over all clips and doesn't ever
> > have to handle the "no clip rects provided" case as a special case itself.
> > 
> > > + }
> > > +
> > > + iter->fb_src.x1 = 0;
> > > + iter->fb_src.y1 = 0;
> > > + iter->fb_src.x2 = state->fb->width;
> > > + iter->fb_src.y2 = state->fb->height;
> > > +
> > > + iter->plane_src.x1 = state->src_x >> 16;
> > > + iter->plane_src.y1 = state->src_y >> 16;
> > > + iter->plane_src.x2 = iter->plane_src.x1 + (state->src_w >> 16);
> > > + iter->plane_src.y2 = iter->plane_src.y1 + (state->src_h >> 16);
> > > + iter->translate_plane_x = -iter->plane_src.x1;
> > > + iter->translate_plane_y = -iter->plane_src.y1;
> > > +
> > > + /* Clip plane src rect to fb dimensions */
> > > + drm_rect_intersect(>plane_src, >fb_src);
> > This smells like driver bug. Also, see Ville's recent efforts to improve
> > the atomic plane clipping, I think drm_plane_state already has all the
> > clip rects you want.
> > 
> > > +
> > > + iter->crtc_src.x1 = 0;
> > > + iter->crtc_src.y1 = 0;
> > > + iter->crtc_src.x2 = hdisplay;
> > > + iter->crtc_src.y2 = vdisplay;
> > > + iter->translate_crtc_x = -(iter->plane_src.x1 - state->crtc_x);
> > > + iter->translate_crtc_x = -(iter->plane_src.y1 - state->crtc_y);
> > > +
> > > + /* Clip crtc 

Re: [RFC 1/3] drm: Add DAMAGE_CLIPS property to plane

[Daniel Vetter]

On Thu, Apr 05, 2018 at 11:00:15AM +0200, Thomas Hellstrom wrote:
> On 04/05/2018 09:35 AM, Daniel Vetter wrote:
> > On Wed, Apr 04, 2018 at 04:49:06PM -0700, Deepak Rawat wrote:
> > > From: Lukasz Spintzyk 
> > > 
> > > Optional plane property to mark damaged regions on the plane in
> > > framebuffer coordinates of the framebuffer attached to the plane.
> > > 
> > > The layout of blob data is simply an array of drm_mode_rect with maximum
> > > array size limited by DRM_MODE_FB_DIRTY_MAX_CLIPS. Unlike plane src
> > > coordinates, damage clips are not in 16.16 fixed point.
> > > 
> > > Damage clips are a hint to kernel as which area of framebuffer has
> > > changed since last page-flip. This should be helpful for some drivers
> > > especially for virtual devices where each framebuffer change needs to
> > > be transmitted over network, usb, etc.
> > > 
> > > Driver which are interested in enabling DAMAGE_CLIPS property for a
> > > plane should enable this property using drm_plane_enable_damage_clips.
> > > 
> > > Signed-off-by: Lukasz Spintzyk 
> > > Signed-off-by: Deepak Rawat 
> > The property uapi section is missing, see:
> > 
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__dri.freedesktop.org_docs_drm_gpu_drm-2Dkms.html-23plane-2Dcomposition-2Dproperties=DwIBAg=uilaK90D4TOVoH58JNXRgQ=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ=J31YNV1uz3IVRaorM-eo48msi9__sQurqRZrig2UE1s=vzsuquF1agbOw54HSC_18Kk2k9j83m8RusmDCtPAFWQ=
> > 
> > Plane composition feels like the best place to put this. Please use that
> > section to tie all the various bits together, including the helpers you're
> > adding in the following patches for drivers to use.
> > 
> > Bunch of nitpicks below, but overall I'm agreeing now with just going with
> > fb coordinate damage rects.
> > 
> > Like you say, the thing needed here now is userspace + driver actually
> > implementing this. I'd also say the compat helper to map the legacy
> > fb->dirty to this new atomic way of doing things should be included here
> > (gives us a lot more testing for these new paths).
> > 
> > Icing on the cake would be an igt to make sure kernel rejects invalid clip
> > rects correctly.
> > 
> > > ---
> > >   drivers/gpu/drm/drm_atomic.c| 42 
> > > +
> > >   drivers/gpu/drm/drm_atomic_helper.c |  4 
> > >   drivers/gpu/drm/drm_mode_config.c   |  5 +
> > >   drivers/gpu/drm/drm_plane.c | 12 +++
> > >   include/drm/drm_mode_config.h   | 15 +
> > >   include/drm/drm_plane.h | 16 ++
> > >   include/uapi/drm/drm_mode.h | 15 +
> > >   7 files changed, 109 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > index 7d25c42..9226d24 100644
> > > --- a/drivers/gpu/drm/drm_atomic.c
> > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > @@ -669,6 +669,40 @@ static void drm_atomic_crtc_print_state(struct 
> > > drm_printer *p,
> > >   }
> > >   /**
> > > + * drm_atomic_set_damage_for_plane - sets the damage clips property to 
> > > plane
> > > + * @state: plane state
> > > + * @blob: damage clips in framebuffer coordinates
> > > + *
> > > + * Returns:
> > > + *
> > > + * Zero on success, error code on failure.
> > > + */
> > > +static int drm_atomic_set_damage_for_plane(struct drm_plane_state *state,
> > > +struct drm_property_blob *blob)
> > > +{
> > > + if (blob == state->damage_clips)
> > > + return 0;
> > > +
> > > + drm_property_blob_put(state->damage_clips);
> > > + state->damage_clips = NULL;
> > > +
> > > + if (blob) {
> > > + uint32_t count = blob->length/sizeof(struct drm_rect);
> > > +
> > > + if (count > DRM_MODE_FB_DIRTY_MAX_CLIPS)
> > > + return -EINVAL;
> > > +
> > > + state->damage_clips = drm_property_blob_get(blob);
> > > + state->num_clips = count;
> > > + } else {
> > > + state->damage_clips = NULL;
> > > + state->num_clips = 0;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > >* drm_atomic_get_plane_state - get plane state
> > >* @state: global atomic state object
> > >* @plane: plane to get state object for
> > > @@ -793,6 +827,12 @@ static int drm_atomic_plane_set_property(struct 
> > > drm_plane *plane,
> > >   state->color_encoding = val;
> > >   } else if (property == plane->color_range_property) {
> > >   state->color_range = val;
> > > + } else if (property == config->prop_damage_clips) {
> > > + struct drm_property_blob *blob =
> > > + drm_property_lookup_blob(dev, val);
> > > + int ret = drm_atomic_set_damage_for_plane(state, blob);
> > There's already a helper with size-checking built-in, see
> > drm_atomic_replace_property_blob_from_id(). Wrt computing num_clips I'd
> > just 

Re: [Intel-gfx] [PATCH v3 09/10] drm/i915/psr: Set DPCD PSR2 enable bit when needed

[Chris Wilson]

Quoting Rodrigo Vivi (2018-03-30 18:41:08)
> On Wed, Mar 28, 2018 at 03:30:45PM -0700, José Roberto de Souza wrote:
> > In the 2 eDP1.4a pannels tested set or not set bit have no effect
> > but is better set it and comply with specification.
> > 
> > Signed-off-by: José Roberto de Souza 
> > Cc: Dhinakaran Pandiyan 
> > Reviewed-by: Rodrigo Vivi 
> 
> patches 1-9 pushed to dinq. Thanks for patches and reviews.

Might be coincidental, but this GPF hasn't occurred before:

https://intel-gfx-ci.01.org/tree/drm-tip/kasan_26/fi-cfl-s3/dmesg15.log
<7>[   40.159658] [drm:intel_enable_sagv [i915]] Enabling the SAGV
<0>[   40.162715] kasan: CONFIG_KASAN_INLINE enabled
<0>[   40.162884] kasan: GPF could be caused by NULL-ptr deref or user memory 
access
<4>[   40.162910] general protection fault:  [#1] PREEMPT SMP KASAN PTI
<4>[   40.179985] Modules linked in: i915 cdc_ether usbnet x86_pkg_temp_thermal 
r8152 intel_powerclamp mii coretemp crct10dif_pclmul crc32_pclmul 
ghash_clmulni_intel e1000e mei_me mei prime_numbers
<4>[   40.18] CPU: 9 PID: 1395 Comm: kms_color Tainted: G U   
4.16.0-rc7-g29940f138482-kasan_26+ #1
<4>[   40.180002] Hardware name: Intel Corporation CoffeeLake Client 
Platform/CoffeeLake S UDIMM RVP, BIOS CNLSFWR1.R00.X118.B19.1802080131 
02/08/2018
<4>[   40.180055] RIP: 0010:intel_psr_flush+0x140/0x4b0 [i915]
<4>[   40.180057] RSP: 0018:880404fd7818 EFLAGS: 00010202
<4>[   40.180060] RAX: dc00 RBX: 8803b482 RCX: 

<4>[   40.180062] RDX: 00bc RSI:  RDI: 
05e0
<4>[   40.180076] RBP: 8803b482a5d8 R08: 8b02b360 R09: 
8a32df20
<4>[   40.180078] R10: 880404fd7818 R11:  R12: 
0100
<4>[   40.180079] R13:  R14: 0100 R15: 

<4>[   40.180081] FS:  7ff255bfe980() GS:88041dc4() 
knlGS:
<4>[   40.180083] CS:  0010 DS:  ES:  CR0: 80050033
<4>[   40.180085] CR2: 7fdf372522f8 CR3: 000419d9a005 CR4: 
003606e0
<4>[   40.180086] DR0:  DR1:  DR2: 

<4>[   40.180088] DR3:  DR6: fffe0ff0 DR7: 
0400
<4>[   40.180089] Call Trace:
<4>[   40.180125]  intel_frontbuffer_flush+0x8e/0xb0 [i915]
<4>[   40.180161]  intel_prepare_plane_fb+0x699/0xb20 [i915]
<4>[   40.180167]  drm_atomic_helper_prepare_planes+0x118/0x480
<4>[   40.180202]  ? haswell_crtc_compute_clock+0xc4/0xc4 [i915]
<4>[   40.180237]  intel_atomic_commit+0x215/0xcd0 [i915]
<4>[   40.180242]  set_property_atomic+0x1d5/0x250
<4>[   40.180245]  ? drm_object_property_get_value+0xe0/0xe0
<4>[   40.180251]  drm_mode_obj_set_property_ioctl+0x334/0x5b0
<4>[   40.180254]  ? drm_mode_obj_find_prop_id+0x180/0x180
<4>[   40.180257]  ? drm_is_current_master+0x5a/0x100
<4>[   40.180260]  ? drm_mode_obj_find_prop_id+0x180/0x180
<4>[   40.180262]  drm_ioctl_kernel+0x189/0x200
<4>[   40.180265]  ? drm_ioctl_permit+0x2b0/0x2b0
<4>[   40.180269]  drm_ioctl+0x662/0x880
<4>[   40.180272]  ? drm_mode_obj_find_prop_id+0x180/0x180
<4>[   40.180274]  ? drm_getstats+0x20/0x20
<4>[   40.180277]  ? lock_acquire+0x390/0x390
<4>[   40.180281]  ? debug_check_no_locks_freed+0x270/0x270
<4>[   40.180284]  ? __might_fault+0xea/0x1a0
<4>[   40.180288]  do_vfs_ioctl+0x171/0xe50
<4>[   40.180292]  ? ioctl_preallocate+0x170/0x170
<4>[   40.180295]  ? __task_pid_nr_ns+0x17b/0x3d0
<4>[   40.180298]  ? lock_acquire+0x390/0x390
<4>[   40.180302]  SyS_ioctl+0x36/0x70
<4>[   40.180304]  ? do_vfs_ioctl+0xe50/0xe50
<4>[   40.180307]  do_syscall_64+0x18a/0x5c0
<4>[   40.180311]  entry_SYSCALL_64_after_hwframe+0x42/0xb7
<4>[   40.180313] RIP: 0033:0x7ff254cf65d7
<4>[   40.180315] RSP: 002b:7ffc2f0fca58 EFLAGS: 0246 ORIG_RAX: 
0010
<4>[   40.180317] RAX: ffda RBX:  RCX: 
7ff254cf65d7
<4>[   40.180319] RDX: 7ffc2f0fca90 RSI: c01864ba RDI: 
0003
<4>[   40.180321] RBP: 7ffc2f0fca90 R08: 0001 R09: 

<4>[   40.180322] R10: 55be9798e010 R11: 0246 R12: 
c01864ba
<4>[   40.180324] R13: 0003 R14:  R15: 

<4>[   40.180328] Code: ea 03 80 3c 02 00 0f 85 4c 03 00 00 4d 8b ad 48 ff ff 
ff 48 b8 00 00 00 00 00 fc ff df 49 8d bd e0 05 00 00 48 89 fa 48 c1 ea 03 <0f> 
b6 04 02 84 c0 74 08 3c 03 0f 8e ee 01 00 00 4d 63 ad e0 05 
<1>[   40.180402] RIP: intel_psr_flush+0x140/0x4b0 [i915] RSP: 880404fd7818
<4>[   40.180416] ---[ end trace 53653c4618c4b0b4 ]---

A null pointer in intel_psr_flush, probably crtc?

The code where we case the psr.enabled in the intel_psr_work preamble is
unlocked, and we do race there with intel_psr_disable() setting it to NULL
before cancelling the work. E.g.

diff --git 

[Bug 199229] choppy cursor on ryzen 5 2400g

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=199229

Michel Dänzer (mic...@daenzer.net) changed:

   What|Removed |Added

 CC||harry.wentl...@amd.com

--- Comment #1 from Michel Dänzer (mic...@daenzer.net) ---
Can you bisect?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm v2 21/23] meson: build exynos by default

[Eric Engestrom]

On Wednesday, 2018-04-04 14:10:33 -0700, Dylan Baker wrote:
> For exynos and omap, are they in active use anymore? I'm pretty sure that
> development of omap (the hardware) stopped, and others have told me exynos has
> stopped too. I also don't think there's any open source consumer, since there 
> is
> no mesa driver for either of these.

Happy to drop these enablement patches; I just like to have everything
possible built by default, but if these are dead I'm fine with leaving
them disabled by default.

> 
> Dylan
> 
> Quoting Eric Engestrom (2018-04-04 08:38:16)
> > Signed-off-by: Eric Engestrom 
> > ---
> >  meson.build | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meson.build b/meson.build
> > index 24688535a329ac530c10..7b26977a9e84290fdd37 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -76,7 +76,7 @@ foreach d : [
> >['nouveau', true, true],
> >['vmwgfx', false, true],
> >['omap', true, true],
> > -  ['exynos', false, false],
> > +  ['exynos', false, true],
> >['freedreno', true, ['arm', 
> > 'aarch64'].contains(host_machine.cpu_family())],
> >['tegra', true, false],
> >['vc4', false, ['arm', 'aarch64'].contains(host_machine.cpu_family())],
> > -- 
> > Cheers,
> >   Eric
> > 


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

[Bug 105729] AMD Radeon flickering on HiDPI display with Gnome Wayland after wake from suspend

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105729

--- Comment #4 from Michel Dänzer  ---
Please attach the corresponding output of dmesg and glxinfo.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 01/11] symbols-check: add new meta-script

[Eric Engestrom]

On Wednesday, 2018-04-04 18:39:48 +0100, Emil Velikov wrote:
> On 4 April 2018 at 17:28, Eric Engestrom  wrote:
> > On Wednesday, 2018-04-04 16:41:35 +0100, Eric Engestrom wrote:
> >> Note: copied from Mesa
> >>
> >> Signed-off-by: Eric Engestrom 
> >> ---
> >>  meson.build   |  1 +
> >>  symbols-check | 79 
> >> +++
> >>  2 files changed, 80 insertions(+)
> >>  create mode 100755 symbols-check
> >>
> >> diff --git a/meson.build b/meson.build
> >> index a725f05d342bbec4df18..c035a00c6747b8d46a9b 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -272,6 +272,7 @@ pkg.generate(
> >>
> >>  env_test = environment()
> >>  env_test.set('NM', find_program('nm').path())
> >> +env_test.set('top_srcdir', meson.source_root())
> >>
> >>  if with_libkms
> >>subdir('libkms')
> >> diff --git a/symbols-check b/symbols-check
> >> new file mode 100755
> >> index ..bac466d93dcb45cee0bb
> >> --- /dev/null
> >> +++ b/symbols-check
> >> @@ -0,0 +1,79 @@
> >> +#!/bin/sh
> >> +set -eu
> >> +set -o pipefail
> >> +
> >> +# Platform specific symbols
> >> +# These will simply be ignored
> >> +PLAT_FUNCS="
> >> +__bss_start
> >> +_init
> >> +_fini
> >> +_end
> >> +_edata
> >> +
> >> +# From tegra-symbol-check
> >> +__bss_end__
> >> +__bss_start__
> >> +__bss_start
> >> +__end__
> >> +_bss_end__
> >> +_edata
> >> +_end
> >> +_fini
> >> +_init
> >
> > Haha, oops... I had noticed that one of the old scripts had more platform
> > symbols than the rest, and I wanted to check if/when those were needed
> > so I just stuffed them here in the mean time, but then I forgot :]
> >
> > I'll check this when I have the time (not this week), or if anyone knows..?
> >
> > In the mean time, please review the rest and ignore these 10 lines :)
> >
> The extra __ suffix/prefixed ones are ARM specific. They seems to be
> introduced from day 1 in glibc, without any obvious reason.
> They're just aliases - __end__, _bss_end__, etc are identical to _end
> 
> If you want to cater for GNU/Hurd - the following three should be
> listed as well.
> 
> _fbss
> _fdata
> _ftext

Thanks for the explanation!
I think I'll just add all these to the platform-specific list on both
mesa and libdrm, they have specific enough names (beginning with
underscore, for starters) that no entrypoint of ours should ever match.

> 
> I haven't had time to read through the patches, although +1 on the overall 
> idea.
> Will try to get to it sometime tomorrow.

Thanks, but I won't have time to push anything until next week anyway,
so there's no rush ^^

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

Re: DRM_UDL and GPU under Xserver

[Daniel Vetter]

On Thu, Apr 5, 2018 at 9:16 AM, Alexey Brodkin
 wrote:
> Hi Daniel,
>
> On Thu, 2018-04-05 at 08:18 +0200, Daniel Vetter wrote:
>> On Wed, Apr 4, 2018 at 10:06 PM, Alexey Brodkin
>>  wrote:
>> > Hello,
>> >
>> > We're trying to use DisplayLink USB2-to-HDMI adapter to render 
>> > GPU-accelerated graphics.
>> > Hardware setup is as simple as a devboard + DisplayLink adapter.
>> > Devboards we use for this experiment are:
>> >  * Wandboard Quad (based on IMX6 SoC with Vivante GPU) or
>> >  * HSDK (based on Synopsys ARC HS38 SoC with Vivante GPU as well)
>> >
>> > I'm sure any other board with DRM supported GPU will work, those we just 
>> > used
>> > as the very recent Linux kernels could be easily run on them both.
>> >
>> > Basically the problem is UDL needs to be explicitly notified about new data
>> > to be rendered on the screen compared to typical bit-streamers that 
>> > infinitely
>> > scan a dedicated buffer in memory.
>> >
>> > In case of UDL there're just 2 ways for this notification:
>> >  1) DRM_IOCTL_MODE_PAGE_FLIP that calls drm_crtc_funcs->page_flip()
>> >  2) DRM_IOCTL_MODE_DIRTYFB that calls drm_framebuffer_funcs->dirty()
>> >
>> > But neither of IOCTLs happen when we run Xserver with xf86-video-armada 
>> > driver
>> > (see 
>> > https://urldefense.proofpoint.com/v2/url?u=http-3A__git.arm.linux.org.uk_cgit_xf86-2Dvideo-2Darmada.git_log_-3Fh-3Dunstable-2Ddevel=DwIBaQ;
>> > c=DPL6_X_6JkXFx7AXWqB0tg=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I=oEAlP64L9vkuUs_k3kGwwwlN1WJbDMJbCo0uDhwKwwk=3ZHj-
>> > 6JXZBLSTWg_4KMnL0VNi7z8c0RxHzj2U5ywVIw=).
>> >
>> > Is it something missing in Xserver or in UDL driver?
>>
>> Use the -modesetting driverr for UDL, that one works correctly.
>
> If you're talking about "modesetting" driver of Xserver [1] then indeed
> picture is displayed on the screen. But there I guess won't be any 3D 
> acceleration.
>
> At least that's what was suggested to me earlier here [2] by Lucas:
> >8---
> For 3D acceleration to work under X you need the etnaviv specific DDX
> driver, which can be found here:
>
> http://git.arm.linux.org.uk/cgit/xf86-video-armada.git/log/?h=unstable-devel

You definitely want to use -modesetting for UDL. And I thought with
glamour and the corresponding mesa work you should also get
accelaration. Insisting that you must use a driver-specific ddx is
broken, the world doesn't work like that anymore.

Lucas, can you pls clarify? Also, why does -armada bind against all
kms drivers, that's probaly too much.
-Daniel

> >8---
>
> [1] 
> https://cgit.freedesktop.org/xorg/xserver/tree/hw/xfree86/drivers/modesetting
> [2] 
> http://lists.infradead.org/pipermail/linux-snps-arc/2017-November/003031.html
>
> -Alexey



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 04/15] v4l: vsp1: Use vsp1_entity.pipe to check if entity belongs to a pipeline

[Laurent Pinchart]

The DRM pipeline handling code uses the entity's pipe list head to check
whether the entity is already included in a pipeline. This method is a
bit fragile in the sense that it uses list_empty() on a list_head that
is a list member. Replace it by a simpler check for the entity pipe
pointer.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a7ad85ab0b08..e210917fdc3f 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -119,9 +119,9 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
 * Remove the RPF from the pipe and the list of BRU
 * inputs.
 */
-   WARN_ON(list_empty(>entity.list_pipe));
+   WARN_ON(!rpf->entity.pipe);
rpf->entity.pipe = NULL;
-   list_del_init(>entity.list_pipe);
+   list_del(>entity.list_pipe);
pipe->inputs[i] = NULL;
 
bru->inputs[rpf->bru_input].rpf = NULL;
@@ -537,7 +537,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
continue;
}
 
-   if (list_empty(>entity.list_pipe)) {
+   if (!rpf->entity.pipe) {
rpf->entity.pipe = pipe;
list_add_tail(>entity.list_pipe, >entities);
}
@@ -566,7 +566,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
   VI6_DPR_NODE_UNUSED);
 
entity->pipe = NULL;
-   list_del_init(>list_pipe);
+   list_del(>list_pipe);
 
continue;
}
-- 
Regards,

Laurent Pinchart

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

[PATCH v2 15/15] v4l: vsp1: Rename BRU to BRx

[Laurent Pinchart]

Some VSP instances have two blending units named BRU (Blend/ROP Unit)
and BRS (Blend/ROP Sub unit). The BRS is a smaller version of the BRU
with only two inputs, but otherwise offers similar features and offers
the same register interface. The BRU and BRS can be used exchangeably in
VSP pipelines (provided no more than two inputs are needed).

Due to historical reasons, the VSP1 driver implements support for both
the BRU and BRS through objects named vsp1_bru. The code uses the name
BRU to refer to either the BRU or the BRS, except in a few places where
noted explicitly. This creates confusion.

In an effort to avoid confusion, rename the vsp1_bru object and the
corresponding API to vsp1_brx, and use BRx to refer to blend unit
instances regardless of their type. The names BRU and BRS are retained
where reference to a particular blend unit type is needed, as well as in
hardware registers to stay close to the datasheet.

Signed-off-by: Laurent Pinchart 
Acked-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/Makefile   |   2 +-
 drivers/media/platform/vsp1/vsp1.h |   6 +-
 .../media/platform/vsp1/{vsp1_bru.c => vsp1_brx.c} | 202 ++---
 .../media/platform/vsp1/{vsp1_bru.h => vsp1_brx.h} |  18 +-
 drivers/media/platform/vsp1/vsp1_drm.c | 174 +-
 drivers/media/platform/vsp1/vsp1_drm.h |   6 +-
 drivers/media/platform/vsp1/vsp1_drv.c |   6 +-
 drivers/media/platform/vsp1/vsp1_pipe.c|  12 +-
 drivers/media/platform/vsp1/vsp1_pipe.h|   4 +-
 drivers/media/platform/vsp1/vsp1_rpf.c |  12 +-
 drivers/media/platform/vsp1/vsp1_rwpf.h|   2 +-
 drivers/media/platform/vsp1/vsp1_video.c   |  16 +-
 drivers/media/platform/vsp1/vsp1_wpf.c |   8 +-
 13 files changed, 234 insertions(+), 234 deletions(-)
 rename drivers/media/platform/vsp1/{vsp1_bru.c => vsp1_brx.c} (63%)
 rename drivers/media/platform/vsp1/{vsp1_bru.h => vsp1_brx.h} (66%)

diff --git a/drivers/media/platform/vsp1/Makefile 
b/drivers/media/platform/vsp1/Makefile
index f5cd6f0491cb..596775f932c0 100644
--- a/drivers/media/platform/vsp1/Makefile
+++ b/drivers/media/platform/vsp1/Makefile
@@ -3,7 +3,7 @@ vsp1-y  := vsp1_drv.o 
vsp1_entity.o vsp1_pipe.o
 vsp1-y += vsp1_dl.o vsp1_drm.o vsp1_video.o
 vsp1-y += vsp1_rpf.o vsp1_rwpf.o vsp1_wpf.o
 vsp1-y += vsp1_clu.o vsp1_hsit.o vsp1_lut.o
-vsp1-y += vsp1_bru.o vsp1_sru.o vsp1_uds.o
+vsp1-y += vsp1_brx.o vsp1_sru.o vsp1_uds.o
 vsp1-y += vsp1_hgo.o vsp1_hgt.o vsp1_histo.o
 vsp1-y += vsp1_lif.o
 
diff --git a/drivers/media/platform/vsp1/vsp1.h 
b/drivers/media/platform/vsp1/vsp1.h
index 78ef838416b3..894cc725c2d4 100644
--- a/drivers/media/platform/vsp1/vsp1.h
+++ b/drivers/media/platform/vsp1/vsp1.h
@@ -30,7 +30,7 @@ struct rcar_fcp_device;
 struct vsp1_drm;
 struct vsp1_entity;
 struct vsp1_platform_data;
-struct vsp1_bru;
+struct vsp1_brx;
 struct vsp1_clu;
 struct vsp1_hgo;
 struct vsp1_hgt;
@@ -78,8 +78,8 @@ struct vsp1_device {
struct rcar_fcp_device *fcp;
struct device *bus_master;
 
-   struct vsp1_bru *brs;
-   struct vsp1_bru *bru;
+   struct vsp1_brx *brs;
+   struct vsp1_brx *bru;
struct vsp1_clu *clu;
struct vsp1_hgo *hgo;
struct vsp1_hgt *hgt;
diff --git a/drivers/media/platform/vsp1/vsp1_bru.c 
b/drivers/media/platform/vsp1/vsp1_brx.c
similarity index 63%
rename from drivers/media/platform/vsp1/vsp1_bru.c
rename to drivers/media/platform/vsp1/vsp1_brx.c
index e8fd2ae3b3eb..b4af1d546022 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_brx.c
@@ -1,5 +1,5 @@
 /*
- * vsp1_bru.c  --  R-Car VSP1 Blend ROP Unit
+ * vsp1_brx.c  --  R-Car VSP1 Blend ROP Unit (BRU and BRS)
  *
  * Copyright (C) 2013 Renesas Corporation
  *
@@ -17,45 +17,45 @@
 #include 
 
 #include "vsp1.h"
-#include "vsp1_bru.h"
+#include "vsp1_brx.h"
 #include "vsp1_dl.h"
 #include "vsp1_pipe.h"
 #include "vsp1_rwpf.h"
 #include "vsp1_video.h"
 
-#define BRU_MIN_SIZE   1U
-#define BRU_MAX_SIZE   8190U
+#define BRX_MIN_SIZE   1U
+#define BRX_MAX_SIZE   8190U
 
 /* 
-
  * Device Access
  */
 
-static inline void vsp1_bru_write(struct vsp1_bru *bru, struct vsp1_dl_list 
*dl,
+static inline void vsp1_brx_write(struct vsp1_brx *brx, struct vsp1_dl_list 
*dl,
  u32 reg, u32 data)
 {
-   vsp1_dl_list_write(dl, bru->base + reg, data);
+   

[PATCH v2 08/15] v4l: vsp1: Replace manual DRM pipeline input setup in vsp1_du_setup_lif

[Laurent Pinchart]

The vsp1_du_setup_lif() function sets up the DRM pipeline input
manually. This duplicates the code from the
vsp1_du_pipeline_setup_input() function. Replace the manual
implementation by a call to the function.

As the pipeline has no enabled input in vsp1_du_setup_lif(), the
vsp1_du_pipeline_setup_input() function will not setup any RPF, and will
thus not setup formats on the BRU sink pads. This isn't a problem as all
inputs are disabled, and the BRU sink pads will be reconfigured from the
atomic commit handler when inputs will be enabled.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
--
Changes since v1:

- Fixed spelling in commit message
---
 drivers/media/platform/vsp1/vsp1_drm.c | 40 +-
 1 file changed, 6 insertions(+), 34 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 6ad8aa6c8138..00ce99bd1605 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -412,47 +412,19 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
dev_dbg(vsp1->dev, "%s: configuring LIF%u with format %ux%u\n",
__func__, pipe_index, cfg->width, cfg->height);
 
-   /*
-* Configure the format at the BRU sinks and propagate it through the
-* pipeline.
-*/
+   /* Setup formats through the pipeline. */
+   ret = vsp1_du_pipeline_setup_input(vsp1, pipe);
+   if (ret < 0)
+   return ret;
+
memset(, 0, sizeof(format));
format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-
-   for (i = 0; i < pipe->bru->source_pad; ++i) {
-   format.pad = i;
-
-   format.format.width = cfg->width;
-   format.format.height = cfg->height;
-   format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
-   format.format.field = V4L2_FIELD_NONE;
-
-   ret = v4l2_subdev_call(>bru->subdev, pad,
-  set_fmt, NULL, );
-   if (ret < 0)
-   return ret;
-
-   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
-   __func__, format.format.width, format.format.height,
-   format.format.code, BRU_NAME(pipe->bru), i);
-   }
-
-   format.pad = pipe->bru->source_pad;
+   format.pad = RWPF_PAD_SINK;
format.format.width = cfg->width;
format.format.height = cfg->height;
format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
format.format.field = V4L2_FIELD_NONE;
 
-   ret = v4l2_subdev_call(>bru->subdev, pad, set_fmt, NULL,
-  );
-   if (ret < 0)
-   return ret;
-
-   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
-   __func__, format.format.width, format.format.height,
-   format.format.code, BRU_NAME(pipe->bru), i);
-
-   format.pad = RWPF_PAD_SINK;
ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL,
   );
if (ret < 0)
-- 
Regards,

Laurent Pinchart

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

[PATCH v2 07/15] v4l: vsp1: Setup BRU at atomic commit time

[Laurent Pinchart]

To implement fully dynamic plane assignment to pipelines, we need to
reassign the BRU and BRS to the DRM pipelines in the atomic commit
handler. In preparation for this setup factor out the BRU source pad
code and call it both at LIF setup and atomic commit time.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 56 +-
 drivers/media/platform/vsp1/vsp1_drm.h |  5 +++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 7bf697ba7969..6ad8aa6c8138 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -148,12 +148,51 @@ static int vsp1_du_pipeline_setup_rpf(struct vsp1_device 
*vsp1,
return 0;
 }
 
+/* Setup the BRU source pad. */
+static int vsp1_du_pipeline_setup_bru(struct vsp1_device *vsp1,
+ struct vsp1_pipeline *pipe)
+{
+   struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
+   struct v4l2_subdev_format format = {
+   .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+   };
+   int ret;
+
+   /*
+* Configure the format on the BRU source and verify that it matches the
+* requested format. We don't set the media bus code as it is configured
+* on the BRU sink pad 0 and propagated inside the entity, not on the
+* source pad.
+*/
+   format.pad = pipe->bru->source_pad;
+   format.format.width = drm_pipe->width;
+   format.format.height = drm_pipe->height;
+   format.format.field = V4L2_FIELD_NONE;
+
+   ret = v4l2_subdev_call(>bru->subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, BRU_NAME(pipe->bru), pipe->bru->source_pad);
+
+   if (format.format.width != drm_pipe->width ||
+   format.format.height != drm_pipe->height) {
+   dev_dbg(vsp1->dev, "%s: format mismatch\n", __func__);
+   return -EPIPE;
+   }
+
+   return 0;
+}
+
 static unsigned int rpf_zpos(struct vsp1_device *vsp1, struct vsp1_rwpf *rpf)
 {
return vsp1->drm->inputs[rpf->entity.index].zpos;
 }
 
-/* Setup the input side of the pipeline (RPFs and BRU sink pads). */
+/* Setup the input side of the pipeline (RPFs and BRU). */
 static int vsp1_du_pipeline_setup_input(struct vsp1_device *vsp1,
struct vsp1_pipeline *pipe)
 {
@@ -191,6 +230,18 @@ static int vsp1_du_pipeline_setup_input(struct vsp1_device 
*vsp1,
inputs[j] = rpf;
}
 
+   /*
+* Setup the BRU. This must be done before setting up the RPF input
+* pipelines as the BRU sink compose rectangles depend on the BRU source
+* format.
+*/
+   ret = vsp1_du_pipeline_setup_bru(vsp1, pipe);
+   if (ret < 0) {
+   dev_err(vsp1->dev, "%s: failed to setup %s source\n", __func__,
+   BRU_NAME(pipe->bru));
+   return ret;
+   }
+
/* Setup the RPF input pipeline for every enabled input. */
for (i = 0; i < pipe->bru->source_pad; ++i) {
struct vsp1_rwpf *rpf = inputs[i];
@@ -355,6 +406,9 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
return 0;
}
 
+   drm_pipe->width = cfg->width;
+   drm_pipe->height = cfg->height;
+
dev_dbg(vsp1->dev, "%s: configuring LIF%u with format %ux%u\n",
__func__, pipe_index, cfg->width, cfg->height);
 
diff --git a/drivers/media/platform/vsp1/vsp1_drm.h 
b/drivers/media/platform/vsp1/vsp1_drm.h
index 9aa19325cbe9..c8dd75ba01f6 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.h
+++ b/drivers/media/platform/vsp1/vsp1_drm.h
@@ -20,12 +20,17 @@
 /**
  * vsp1_drm_pipeline - State for the API exposed to the DRM driver
  * @pipe: the VSP1 pipeline used for display
+ * @width: output display width
+ * @height: output display height
  * @du_complete: frame completion callback for the DU driver (optional)
  * @du_private: data to be passed to the du_complete callback
  */
 struct vsp1_drm_pipeline {
struct vsp1_pipeline pipe;
 
+   unsigned int width;
+   unsigned int height;
+
/* Frame synchronisation */
void (*du_complete)(void *, bool);
void *du_private;
-- 
Regards,

Laurent Pinchart

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

[PATCH v2 11/15] v4l: vsp1: Add per-display list internal completion notification support

[Laurent Pinchart]

Display list completion is already reported to the frame end handler,
but that mechanism is global to all display lists. In order to implement
BRU and BRS reassignment in DRM pipelines we will need to commit a
display list and wait for its completion internally, without reporting
it to the DRM driver. Extend the display list API to support such an
internal use of the display list.

Signed-off-by: Laurent Pinchart 
---
Changes since v1:

- Use the frame end status flags to report notification
- Rename the notify flag to internal
---
 drivers/media/platform/vsp1/vsp1_dl.c| 23 ++-
 drivers/media/platform/vsp1/vsp1_dl.h|  3 ++-
 drivers/media/platform/vsp1/vsp1_drm.c   |  2 +-
 drivers/media/platform/vsp1/vsp1_video.c |  2 +-
 4 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 662fa2a347c9..30ad491605ff 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -72,6 +72,7 @@ struct vsp1_dl_body {
  * @fragments: list of extra display list bodies
  * @has_chain: if true, indicates that there's a partition chain
  * @chain: entry in the display list partition chain
+ * @internal: whether the display list is used for internal purpose
  */
 struct vsp1_dl_list {
struct list_head list;
@@ -85,6 +86,8 @@ struct vsp1_dl_list {
 
bool has_chain;
struct list_head chain;
+
+   bool internal;
 };
 
 enum vsp1_dl_mode {
@@ -550,8 +553,16 @@ static void vsp1_dl_list_commit_continuous(struct 
vsp1_dl_list *dl)
 * case we can't replace the queued list by the new one, as we could
 * race with the hardware. We thus mark the update as pending, it will
 * be queued up to the hardware by the frame end interrupt handler.
+*
+* If a display list is already pending we simply drop it as the new
+* display list is assumed to contain a more recent configuration. It is
+* an error if the already pending list has the internal flag set, as
+* there is then a process waiting for that list to complete. This
+* shouldn't happen as the waiting process should perform proper
+* locking, but warn just in case.
 */
if (vsp1_dl_list_hw_update_pending(dlm)) {
+   WARN_ON(dlm->pending && dlm->pending->internal);
__vsp1_dl_list_put(dlm->pending);
dlm->pending = dl;
return;
@@ -581,7 +592,7 @@ static void vsp1_dl_list_commit_singleshot(struct 
vsp1_dl_list *dl)
dlm->active = dl;
 }
 
-void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
+void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal)
 {
struct vsp1_dl_manager *dlm = dl->dlm;
struct vsp1_dl_list *dl_child;
@@ -598,6 +609,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
}
}
 
+   dl->internal = internal;
+
spin_lock_irqsave(>lock, flags);
 
if (dlm->singleshot)
@@ -624,6 +637,10 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
  * raced with the frame end interrupt. The function always returns with the 
flag
  * set in header mode as display list processing is then not continuous and
  * races never occur.
+ *
+ * The VSP1_DL_FRAME_END_INTERNAL flag indicates that the previous display list
+ * has completed and had been queued with the internal notification flag.
+ * Internal notification is only supported for continuous mode.
  */
 unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
 {
@@ -656,6 +673,10 @@ unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager 
*dlm)
 * frame end interrupt. The display list thus becomes active.
 */
if (dlm->queued) {
+   if (dlm->queued->internal)
+   flags |= VSP1_DL_FRAME_END_INTERNAL;
+   dlm->queued->internal = false;
+
__vsp1_dl_list_put(dlm->active);
dlm->active = dlm->queued;
dlm->queued = NULL;
diff --git a/drivers/media/platform/vsp1/vsp1_dl.h 
b/drivers/media/platform/vsp1/vsp1_dl.h
index cbc2fc53e10b..1a5bbd5ddb7b 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.h
+++ b/drivers/media/platform/vsp1/vsp1_dl.h
@@ -21,6 +21,7 @@ struct vsp1_dl_list;
 struct vsp1_dl_manager;
 
 #define VSP1_DL_FRAME_END_COMPLETEDBIT(0)
+#define VSP1_DL_FRAME_END_INTERNAL BIT(1)
 
 void vsp1_dlm_setup(struct vsp1_device *vsp1);
 
@@ -34,7 +35,7 @@ unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager 
*dlm);
 struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm);
 void vsp1_dl_list_put(struct vsp1_dl_list *dl);
 void vsp1_dl_list_write(struct vsp1_dl_list *dl, u32 reg, u32 data);
-void vsp1_dl_list_commit(struct vsp1_dl_list *dl);
+void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal);
 
 struct vsp1_dl_body 

[PATCH v2 12/15] v4l: vsp1: Generalize detection of entity removal from DRM pipeline

[Laurent Pinchart]

When disabling a DRM plane, the corresponding RPF is only marked as
removed from the pipeline in the atomic update handler, with the actual
removal happening when configuring the pipeline at atomic commit time.
This is required as the RPF has to be disabled in the hardware, which
can't be done from the atomic update handler.

The current implementation is RPF-specific. Make it independent of the
entity type by using the entity's pipe pointer to mark removal from the
pipeline. This will allow using the mechanism to remove BRU instances.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 68b126044ea1..a9c53892a9ea 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -346,13 +346,12 @@ static void vsp1_du_pipeline_configure(struct 
vsp1_pipeline *pipe)
dl = vsp1_dl_list_get(pipe->output->dlm);
 
list_for_each_entry_safe(entity, next, >entities, list_pipe) {
-   /* Disconnect unused RPFs from the pipeline. */
-   if (entity->type == VSP1_ENTITY_RPF &&
-   !pipe->inputs[entity->index]) {
+   /* Disconnect unused entities from the pipeline. */
+   if (!entity->pipe) {
vsp1_dl_list_write(dl, entity->route->reg,
   VI6_DPR_NODE_UNUSED);
 
-   entity->pipe = NULL;
+   entity->sink = NULL;
list_del(>list_pipe);
 
continue;
@@ -569,10 +568,11 @@ int vsp1_du_atomic_update(struct device *dev, unsigned 
int pipe_index,
rpf_index);
 
/*
-* Remove the RPF from the pipe's inputs. The atomic flush
-* handler will disable the input and remove the entity from the
-* pipe's entities list.
+* Remove the RPF from the pipeline's inputs. Keep it in the
+* pipeline's entity list to let vsp1_du_pipeline_configure()
+* remove it from the hardware pipeline.
 */
+   rpf->entity.pipe = NULL;
drm_pipe->pipe.inputs[rpf_index] = NULL;
return 0;
}
-- 
Regards,

Laurent Pinchart

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

[PATCH v2 10/15] v4l: vsp1: Turn frame end completion status into a bitfield

[Laurent Pinchart]

We will soon need to return more than a boolean completion status from
the vsp1_dlm_irq_frame_end() IRQ handler. Turn the return value into a
bitfield to prepare for that. No functional change is introduced here.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_dl.c| 22 +-
 drivers/media/platform/vsp1/vsp1_dl.h|  4 +++-
 drivers/media/platform/vsp1/vsp1_drm.c   |  5 +++--
 drivers/media/platform/vsp1/vsp1_pipe.c  |  8 
 drivers/media/platform/vsp1/vsp1_pipe.h  |  2 +-
 drivers/media/platform/vsp1/vsp1_video.c |  4 ++--
 6 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 0b86ed01e85d..662fa2a347c9 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -616,14 +616,18 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
  * vsp1_dlm_irq_frame_end - Display list handler for the frame end interrupt
  * @dlm: the display list manager
  *
- * Return true if the previous display list has completed at frame end, or 
false
- * if it has been delayed by one frame because the display list commit raced
- * with the frame end interrupt. The function always returns true in header 
mode
- * as display list processing is then not continuous and races never occur.
+ * Return a set of flags that indicates display list completion status.
+ *
+ * The VSP1_DL_FRAME_END_COMPLETED flag indicates that the previous display 
list
+ * has completed at frame end. If the flag is not returned display list
+ * completion has been delayed by one frame because the display list commit
+ * raced with the frame end interrupt. The function always returns with the 
flag
+ * set in header mode as display list processing is then not continuous and
+ * races never occur.
  */
-bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
+unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
 {
-   bool completed = false;
+   unsigned int flags = 0;
 
spin_lock(>lock);
 
@@ -634,7 +638,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
if (dlm->singleshot) {
__vsp1_dl_list_put(dlm->active);
dlm->active = NULL;
-   completed = true;
+   flags |= VSP1_DL_FRAME_END_COMPLETED;
goto done;
}
 
@@ -655,7 +659,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
__vsp1_dl_list_put(dlm->active);
dlm->active = dlm->queued;
dlm->queued = NULL;
-   completed = true;
+   flags |= VSP1_DL_FRAME_END_COMPLETED;
}
 
/*
@@ -672,7 +676,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
 done:
spin_unlock(>lock);
 
-   return completed;
+   return flags;
 }
 
 /* Hardware Setup */
diff --git a/drivers/media/platform/vsp1/vsp1_dl.h 
b/drivers/media/platform/vsp1/vsp1_dl.h
index ee3508172f0a..cbc2fc53e10b 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.h
+++ b/drivers/media/platform/vsp1/vsp1_dl.h
@@ -20,6 +20,8 @@ struct vsp1_dl_fragment;
 struct vsp1_dl_list;
 struct vsp1_dl_manager;
 
+#define VSP1_DL_FRAME_END_COMPLETEDBIT(0)
+
 void vsp1_dlm_setup(struct vsp1_device *vsp1);
 
 struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
@@ -27,7 +29,7 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device 
*vsp1,
unsigned int prealloc);
 void vsp1_dlm_destroy(struct vsp1_dl_manager *dlm);
 void vsp1_dlm_reset(struct vsp1_dl_manager *dlm);
-bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm);
+unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm);
 
 struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm);
 void vsp1_dl_list_put(struct vsp1_dl_list *dl);
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a79b05ef..541473b1df67 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -34,12 +34,13 @@
  */
 
 static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe,
-  bool completed)
+  unsigned int completion)
 {
struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
 
if (drm_pipe->du_complete)
-   drm_pipe->du_complete(drm_pipe->du_private, completed);
+   drm_pipe->du_complete(drm_pipe->du_private,
+ completion & VSP1_DL_FRAME_END_COMPLETED);
 }
 
 /* 
-
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c 
b/drivers/media/platform/vsp1/vsp1_pipe.c
index 99ccbac3256a..1134f14ed4aa 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.c
+++ 

[PATCH v2 13/15] v4l: vsp1: Assign BRU and BRS to pipelines dynamically

[Laurent Pinchart]

The VSPDL variant drives two DU channels through two LIF and two
blenders, BRU and BRS. The DU channels thus share the five available
VSPDL inputs and expose them as five KMS planes.

The current implementation assigns the BRS to the second LIF and thus
artificially limits the number of planes for the second display channel
to two at most.

Lift this artificial limitation by assigning the BRU and BRS to the
display pipelines on demand based on the number of planes used by each
pipeline. When a display pipeline needs more than two inputs and the BRU
is already in use by the other pipeline, this requires reconfiguring the
other pipeline to free the BRU before processing, which can result in
frame drop on both pipelines.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 161 +++--
 drivers/media/platform/vsp1/vsp1_drm.h |   9 ++
 2 files changed, 144 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a9c53892a9ea..f82f83b6d4ff 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -37,10 +37,15 @@ static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline 
*pipe,
   unsigned int completion)
 {
struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
+   bool complete = completion == VSP1_DL_FRAME_END_COMPLETED;
 
if (drm_pipe->du_complete)
-   drm_pipe->du_complete(drm_pipe->du_private,
- completion & VSP1_DL_FRAME_END_COMPLETED);
+   drm_pipe->du_complete(drm_pipe->du_private, complete);
+
+   if (completion & VSP1_DL_FRAME_END_INTERNAL) {
+   drm_pipe->force_bru_release = false;
+   wake_up(_pipe->wait_queue);
+   }
 }
 
 /* 
-
@@ -150,6 +155,10 @@ static int vsp1_du_pipeline_setup_rpf(struct vsp1_device 
*vsp1,
 }
 
 /* Setup the BRU source pad. */
+static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1,
+struct vsp1_pipeline *pipe);
+static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe);
+
 static int vsp1_du_pipeline_setup_bru(struct vsp1_device *vsp1,
  struct vsp1_pipeline *pipe)
 {
@@ -157,8 +166,93 @@ static int vsp1_du_pipeline_setup_bru(struct vsp1_device 
*vsp1,
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
+   struct vsp1_entity *bru;
int ret;
 
+   /*
+* Pick a BRU:
+* - If we need more than two inputs, use the main BRU.
+* - Otherwise, if we are not forced to release our BRU, keep it.
+* - Else, use any free BRU (randomly starting with the main BRU).
+*/
+   if (pipe->num_inputs > 2)
+   bru = >bru->entity;
+   else if (pipe->bru && !drm_pipe->force_bru_release)
+   bru = pipe->bru;
+   else if (!vsp1->bru->entity.pipe)
+   bru = >bru->entity;
+   else
+   bru = >brs->entity;
+
+   /* Switch BRU if needed. */
+   if (bru != pipe->bru) {
+   struct vsp1_entity *released_bru = NULL;
+
+   /* Release our BRU if we have one. */
+   if (pipe->bru) {
+   /*
+* The BRU might be acquired by the other pipeline in
+* the next step. We must thus remove it from the list
+* of entities for this pipeline. The other pipeline's
+* hardware configuration will reconfigure the BRU
+* routing.
+*
+* However, if the other pipeline doesn't acquire our
+* BRU, we need to keep it in the list, otherwise the
+* hardware configuration step won't disconnect it from
+* the pipeline. To solve this, store the released BRU
+* pointer to add it back to the list of entities later
+* if it isn't acquired by the other pipeline.
+*/
+   released_bru = pipe->bru;
+
+   list_del(>bru->list_pipe);
+   pipe->bru->sink = NULL;
+   pipe->bru->pipe = NULL;
+   pipe->bru = NULL;
+   }
+
+   /*
+* If the BRU we need is in use, force the owner pipeline to
+* switch to the other BRU and wait until the switch completes.
+*/
+   if (bru->pipe) {
+   struct vsp1_drm_pipeline 

[PATCH v2 14/15] v4l: vsp1: Add BRx dynamic assignment debugging messages

[Laurent Pinchart]

Dynamic assignment of the BRU and BRS to pipelines is prone to
regressions, add messages to make debugging easier. Keep it as a
separate commit to ease removal of those messages once the code will
deem to be completely stable.

Signed-off-by: Laurent Pinchart 
Acked-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index f82f83b6d4ff..b43d6dc0d5f5 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -190,6 +190,10 @@ static int vsp1_du_pipeline_setup_bru(struct vsp1_device 
*vsp1,
 
/* Release our BRU if we have one. */
if (pipe->bru) {
+   dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
+   __func__, pipe->lif->index,
+   BRU_NAME(pipe->bru));
+
/*
 * The BRU might be acquired by the other pipeline in
 * the next step. We must thus remove it from the list
@@ -219,6 +223,9 @@ static int vsp1_du_pipeline_setup_bru(struct vsp1_device 
*vsp1,
if (bru->pipe) {
struct vsp1_drm_pipeline *owner_pipe;
 
+   dev_dbg(vsp1->dev, "%s: pipe %u: waiting for %s\n",
+   __func__, pipe->lif->index, BRU_NAME(bru));
+
owner_pipe = to_vsp1_drm_pipeline(bru->pipe);
owner_pipe->force_bru_release = true;
 
@@ -245,6 +252,9 @@ static int vsp1_du_pipeline_setup_bru(struct vsp1_device 
*vsp1,
  >entities);
 
/* Add the BRU to the pipeline. */
+   dev_dbg(vsp1->dev, "%s: pipe %u: acquired %s\n",
+   __func__, pipe->lif->index, BRU_NAME(bru));
+
pipe->bru = bru;
pipe->bru->pipe = pipe;
pipe->bru->sink = >output->entity;
@@ -548,6 +558,10 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
drm_pipe->du_complete = NULL;
pipe->num_inputs = 0;
 
+   dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
+   __func__, pipe->lif->index,
+   BRU_NAME(pipe->bru));
+
list_del(>bru->list_pipe);
pipe->bru->pipe = NULL;
pipe->bru = NULL;
-- 
Regards,

Laurent Pinchart

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

[PATCH v2 09/15] v4l: vsp1: Move DRM pipeline output setup code to a function

[Laurent Pinchart]

In order to make the vsp1_du_setup_lif() easier to read, and for
symmetry with the DRM pipeline input setup, move the pipeline output
setup code to a separate function.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
--
Changes since v1:

- Rename vsp1_du_pipeline_setup_input() to
  vsp1_du_pipeline_setup_inputs()
- Initialize format local variable to 0 in
  vsp1_du_pipeline_setup_output()
---
 drivers/media/platform/vsp1/vsp1_drm.c | 114 ++---
 1 file changed, 64 insertions(+), 50 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 00ce99bd1605..a79b05ef 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -193,8 +193,8 @@ static unsigned int rpf_zpos(struct vsp1_device *vsp1, 
struct vsp1_rwpf *rpf)
 }
 
 /* Setup the input side of the pipeline (RPFs and BRU). */
-static int vsp1_du_pipeline_setup_input(struct vsp1_device *vsp1,
-   struct vsp1_pipeline *pipe)
+static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1,
+struct vsp1_pipeline *pipe)
 {
struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, };
struct vsp1_bru *bru = to_bru(>bru->subdev);
@@ -276,6 +276,65 @@ static int vsp1_du_pipeline_setup_input(struct vsp1_device 
*vsp1,
return 0;
 }
 
+/* Setup the output side of the pipeline (WPF and LIF). */
+static int vsp1_du_pipeline_setup_output(struct vsp1_device *vsp1,
+struct vsp1_pipeline *pipe)
+{
+   struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
+   struct v4l2_subdev_format format = { 0, };
+   int ret;
+
+   format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+   format.pad = RWPF_PAD_SINK;
+   format.format.width = drm_pipe->width;
+   format.format.height = drm_pipe->height;
+   format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
+   format.format.field = V4L2_FIELD_NONE;
+
+   ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on WPF%u sink\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, pipe->output->entity.index);
+
+   format.pad = RWPF_PAD_SOURCE;
+   ret = v4l2_subdev_call(>output->entity.subdev, pad, get_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: got format %ux%u (%x) on WPF%u source\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, pipe->output->entity.index);
+
+   format.pad = LIF_PAD_SINK;
+   ret = v4l2_subdev_call(>lif->subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on LIF%u sink\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, pipe->lif->index);
+
+   /*
+* Verify that the format at the output of the pipeline matches the
+* requested frame size and media bus code.
+*/
+   if (format.format.width != drm_pipe->width ||
+   format.format.height != drm_pipe->height ||
+   format.format.code != MEDIA_BUS_FMT_ARGB_1X32) {
+   dev_dbg(vsp1->dev, "%s: format mismatch on LIF%u\n", __func__,
+   pipe->lif->index);
+   return -EPIPE;
+   }
+
+   return 0;
+}
+
 /* Configure all entities in the pipeline. */
 static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
 {
@@ -356,7 +415,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
struct vsp1_drm_pipeline *drm_pipe;
struct vsp1_pipeline *pipe;
struct vsp1_bru *bru;
-   struct v4l2_subdev_format format;
unsigned long flags;
unsigned int i;
int ret;
@@ -413,58 +471,14 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
__func__, pipe_index, cfg->width, cfg->height);
 
/* Setup formats through the pipeline. */
-   ret = vsp1_du_pipeline_setup_input(vsp1, pipe);
-   if (ret < 0)
-   return ret;
-
-   memset(, 0, sizeof(format));
-   format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-   format.pad = RWPF_PAD_SINK;
-   format.format.width = cfg->width;
-   format.format.height = cfg->height;
-   format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
-   format.format.field = V4L2_FIELD_NONE;
-
-   ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL,
-  );
-   if 

[PATCH v2 03/15] v4l: vsp1: Store pipeline pointer in vsp1_entity

[Laurent Pinchart]

Various types of objects subclassing vsp1_entity currently store a
pointer to the pipeline. Move the pointer to vsp1_entity to simplify the
code and avoid storing the pipeline in more entity subclasses later.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c| 20 +--
 drivers/media/platform/vsp1/vsp1_drv.c|  2 +-
 drivers/media/platform/vsp1/vsp1_entity.h |  2 ++
 drivers/media/platform/vsp1/vsp1_histo.c  |  2 +-
 drivers/media/platform/vsp1/vsp1_histo.h  |  3 ---
 drivers/media/platform/vsp1/vsp1_pipe.c   | 33 +--
 drivers/media/platform/vsp1/vsp1_rwpf.h   |  2 --
 drivers/media/platform/vsp1/vsp1_video.c  | 17 +++-
 8 files changed, 34 insertions(+), 47 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a267f12f0cc8..a7ad85ab0b08 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -120,6 +120,7 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
 * inputs.
 */
WARN_ON(list_empty(>entity.list_pipe));
+   rpf->entity.pipe = NULL;
list_del_init(>entity.list_pipe);
pipe->inputs[i] = NULL;
 
@@ -536,8 +537,10 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
continue;
}
 
-   if (list_empty(>entity.list_pipe))
+   if (list_empty(>entity.list_pipe)) {
+   rpf->entity.pipe = pipe;
list_add_tail(>entity.list_pipe, >entities);
+   }
 
bru->inputs[i].rpf = rpf;
rpf->bru_input = i;
@@ -562,6 +565,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
vsp1_dl_list_write(dl, entity->route->reg,
   VI6_DPR_NODE_UNUSED);
 
+   entity->pipe = NULL;
list_del_init(>list_pipe);
 
continue;
@@ -625,24 +629,28 @@ int vsp1_drm_init(struct vsp1_device *vsp1)
 
vsp1_pipeline_init(pipe);
 
+   pipe->frame_end = vsp1_du_pipeline_frame_end;
+
/*
 * The DRM pipeline is static, add entities manually. The first
 * pipeline uses the BRU and the second pipeline the BRS.
 */
pipe->bru = i == 0 ? >bru->entity : >brs->entity;
-   pipe->lif = >lif[i]->entity;
pipe->output = vsp1->wpf[i];
-   pipe->output->pipe = pipe;
-   pipe->frame_end = vsp1_du_pipeline_frame_end;
+   pipe->lif = >lif[i]->entity;
 
+   pipe->bru->pipe = pipe;
pipe->bru->sink = >output->entity;
pipe->bru->sink_pad = 0;
+   list_add_tail(>bru->list_pipe, >entities);
+
+   pipe->output->entity.pipe = pipe;
pipe->output->entity.sink = pipe->lif;
pipe->output->entity.sink_pad = 0;
+   list_add_tail(>output->entity.list_pipe, >entities);
 
-   list_add_tail(>bru->list_pipe, >entities);
+   pipe->lif->pipe = pipe;
list_add_tail(>lif->list_pipe, >entities);
-   list_add_tail(>output->entity.list_pipe, >entities);
}
 
/* Disable all RPFs initially. */
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c 
b/drivers/media/platform/vsp1/vsp1_drv.c
index eed9516e25e1..58a7993f2306 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -63,7 +63,7 @@ static irqreturn_t vsp1_irq_handler(int irq, void *data)
vsp1_write(vsp1, VI6_WPF_IRQ_STA(i), ~status & mask);
 
if (status & VI6_WFP_IRQ_STA_DFE) {
-   vsp1_pipeline_frame_end(wpf->pipe);
+   vsp1_pipeline_frame_end(wpf->entity.pipe);
ret = IRQ_HANDLED;
}
}
diff --git a/drivers/media/platform/vsp1/vsp1_entity.h 
b/drivers/media/platform/vsp1/vsp1_entity.h
index 408602ebeb97..c26523c56c05 100644
--- a/drivers/media/platform/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/vsp1/vsp1_entity.h
@@ -106,6 +106,8 @@ struct vsp1_entity {
unsigned int index;
const struct vsp1_route *route;
 
+   struct vsp1_pipeline *pipe;
+
struct list_head list_dev;
struct list_head list_pipe;
 
diff --git a/drivers/media/platform/vsp1/vsp1_histo.c 
b/drivers/media/platform/vsp1/vsp1_histo.c
index afab77cf4fa5..8638ebc514b4 100644
--- a/drivers/media/platform/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/vsp1/vsp1_histo.c
@@ 

[PATCH v2 06/15] v4l: vsp1: Move DRM atomic commit pipeline setup to separate function

[Laurent Pinchart]

The DRM pipeline setup code used at atomic commit time is similar to the
setup code used when enabling the pipeline. Move it to a separate
function in order to share it.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 347 +
 1 file changed, 180 insertions(+), 167 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 9a043a915c0b..7bf697ba7969 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -46,6 +46,185 @@ static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline 
*pipe,
  * Pipeline Configuration
  */
 
+/* Setup one RPF and the connected BRU sink pad. */
+static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1,
+ struct vsp1_pipeline *pipe,
+ struct vsp1_rwpf *rpf,
+ unsigned int bru_input)
+{
+   struct v4l2_subdev_selection sel;
+   struct v4l2_subdev_format format;
+   const struct v4l2_rect *crop;
+   int ret;
+
+   /*
+* Configure the format on the RPF sink pad and propagate it up to the
+* BRU sink pad.
+*/
+   crop = >drm->inputs[rpf->entity.index].crop;
+
+   memset(, 0, sizeof(format));
+   format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+   format.pad = RWPF_PAD_SINK;
+   format.format.width = crop->width + crop->left;
+   format.format.height = crop->height + crop->top;
+   format.format.code = rpf->fmtinfo->mbus;
+   format.format.field = V4L2_FIELD_NONE;
+
+   ret = v4l2_subdev_call(>entity.subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev,
+   "%s: set format %ux%u (%x) on RPF%u sink\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, rpf->entity.index);
+
+   memset(, 0, sizeof(sel));
+   sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+   sel.pad = RWPF_PAD_SINK;
+   sel.target = V4L2_SEL_TGT_CROP;
+   sel.r = *crop;
+
+   ret = v4l2_subdev_call(>entity.subdev, pad, set_selection, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev,
+   "%s: set selection (%u,%u)/%ux%u on RPF%u sink\n",
+   __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
+   rpf->entity.index);
+
+   /*
+* RPF source, hardcode the format to ARGB to turn on format
+* conversion if needed.
+*/
+   format.pad = RWPF_PAD_SOURCE;
+
+   ret = v4l2_subdev_call(>entity.subdev, pad, get_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev,
+   "%s: got format %ux%u (%x) on RPF%u source\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, rpf->entity.index);
+
+   format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
+
+   ret = v4l2_subdev_call(>entity.subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   /* BRU sink, propagate the format from the RPF source. */
+   format.pad = bru_input;
+
+   ret = v4l2_subdev_call(>bru->subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, BRU_NAME(pipe->bru), format.pad);
+
+   sel.pad = bru_input;
+   sel.target = V4L2_SEL_TGT_COMPOSE;
+   sel.r = vsp1->drm->inputs[rpf->entity.index].compose;
+
+   ret = v4l2_subdev_call(>bru->subdev, pad, set_selection, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: set selection (%u,%u)/%ux%u on %s pad %u\n",
+   __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
+   BRU_NAME(pipe->bru), sel.pad);
+
+   return 0;
+}
+
+static unsigned int rpf_zpos(struct vsp1_device *vsp1, struct vsp1_rwpf *rpf)
+{
+   return vsp1->drm->inputs[rpf->entity.index].zpos;
+}
+
+/* Setup the input side of the pipeline (RPFs and BRU sink pads). */
+static int vsp1_du_pipeline_setup_input(struct vsp1_device *vsp1,
+   struct vsp1_pipeline *pipe)
+{
+   struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, };
+   struct vsp1_bru *bru = to_bru(>bru->subdev);
+   unsigned int i;
+   int ret;
+
+   /* Count the number of enabled inputs and sort them by Z-order. */
+ 

[PATCH v2 02/15] v4l: vsp1: Remove unused field from vsp1_drm_pipeline structure

[Laurent Pinchart]

The vsp1_drm_pipeline enabled field is set but never used. Remove it.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 4 
 drivers/media/platform/vsp1/vsp1_drm.h | 2 --
 2 files changed, 6 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a1f2ba044092..a267f12f0cc8 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -273,10 +273,6 @@ EXPORT_SYMBOL_GPL(vsp1_du_setup_lif);
  */
 void vsp1_du_atomic_begin(struct device *dev, unsigned int pipe_index)
 {
-   struct vsp1_device *vsp1 = dev_get_drvdata(dev);
-   struct vsp1_drm_pipeline *drm_pipe = >drm->pipe[pipe_index];
-
-   drm_pipe->enabled = drm_pipe->pipe.num_inputs != 0;
 }
 EXPORT_SYMBOL_GPL(vsp1_du_atomic_begin);
 
diff --git a/drivers/media/platform/vsp1/vsp1_drm.h 
b/drivers/media/platform/vsp1/vsp1_drm.h
index 1cd9db785bf7..9aa19325cbe9 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.h
+++ b/drivers/media/platform/vsp1/vsp1_drm.h
@@ -20,13 +20,11 @@
 /**
  * vsp1_drm_pipeline - State for the API exposed to the DRM driver
  * @pipe: the VSP1 pipeline used for display
- * @enabled: pipeline state at the beginning of an update
  * @du_complete: frame completion callback for the DU driver (optional)
  * @du_private: data to be passed to the du_complete callback
  */
 struct vsp1_drm_pipeline {
struct vsp1_pipeline pipe;
-   bool enabled;
 
/* Frame synchronisation */
void (*du_complete)(void *, bool);
-- 
Regards,

Laurent Pinchart

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

[PATCH v2 01/15] v4l: vsp1: Don't start/stop media pipeline for DRM

[Laurent Pinchart]

The DRM support code manages a pipeline of VSP entities, each backed by
a media entity. When starting or stopping the pipeline, it starts and
stops the media pipeline through the media API in order to store the
pipeline pointer in every entity.

The driver doesn't use the pipe pointer in media entities, neither does
it rely on the other effects of the media_pipeline_start() and
media_pipeline_stop() functions. Furthermore, as the media links for the
DRM pipeline are never set up correctly, and as the pipeline can be
modified dynamically when enabling or disabling planes, the current
implementation is not correct. Remove the incorrect and unneeded code.

While at it remove the outdated comment that states that entities are
not started when the LIF is setup, as they now are.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
--
Changes since v1:

- Remove outdated comment
---
 drivers/media/platform/vsp1/vsp1_drm.c | 18 +-
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index b8fee1834253..a1f2ba044092 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -109,8 +109,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
if (ret == -ETIMEDOUT)
dev_err(vsp1->dev, "DRM pipeline stop timeout\n");
 
-   media_pipeline_stop(>output->entity.subdev.entity);
-
for (i = 0; i < ARRAY_SIZE(pipe->inputs); ++i) {
struct vsp1_rwpf *rpf = pipe->inputs[i];
 
@@ -223,13 +221,7 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
return -EPIPE;
}
 
-   /*
-* Mark the pipeline as streaming and enable the VSP1. This will store
-* the pipeline pointer in all entities, which the s_stream handlers
-* will need. We don't start the entities themselves right at this point
-* as there's no plane configured yet, so we can't start processing
-* buffers.
-*/
+   /* Enable the VSP1. */
ret = vsp1_device_get(vsp1);
if (ret < 0)
return ret;
@@ -241,14 +233,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
drm_pipe->du_complete = cfg->callback;
drm_pipe->du_private = cfg->callback_data;
 
-   ret = media_pipeline_start(>output->entity.subdev.entity,
- >pipe);
-   if (ret < 0) {
-   dev_dbg(vsp1->dev, "%s: pipeline start failed\n", __func__);
-   vsp1_device_put(vsp1);
-   return ret;
-   }
-
/* Disable the display interrupts. */
vsp1_write(vsp1, VI6_DISP_IRQ_STA, 0);
vsp1_write(vsp1, VI6_DISP_IRQ_ENB, 0);
-- 
Regards,

Laurent Pinchart

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

[PATCH v2 05/15] v4l: vsp1: Share duplicated DRM pipeline configuration code

[Laurent Pinchart]

Move the duplicated DRM pipeline configuration code to a function and
call it from vsp1_du_setup_lif() and vsp1_du_atomic_flush().

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 95 +++---
 1 file changed, 43 insertions(+), 52 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index e210917fdc3f..9a043a915c0b 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -42,6 +42,47 @@ static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline 
*pipe,
drm_pipe->du_complete(drm_pipe->du_private, completed);
 }
 
+/* 
-
+ * Pipeline Configuration
+ */
+
+/* Configure all entities in the pipeline. */
+static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
+{
+   struct vsp1_entity *entity;
+   struct vsp1_entity *next;
+   struct vsp1_dl_list *dl;
+
+   dl = vsp1_dl_list_get(pipe->output->dlm);
+
+   list_for_each_entry_safe(entity, next, >entities, list_pipe) {
+   /* Disconnect unused RPFs from the pipeline. */
+   if (entity->type == VSP1_ENTITY_RPF &&
+   !pipe->inputs[entity->index]) {
+   vsp1_dl_list_write(dl, entity->route->reg,
+  VI6_DPR_NODE_UNUSED);
+
+   entity->pipe = NULL;
+   list_del(>list_pipe);
+
+   continue;
+   }
+
+   vsp1_entity_route_setup(entity, pipe, dl);
+
+   if (entity->ops->configure) {
+   entity->ops->configure(entity, pipe, dl,
+  VSP1_ENTITY_PARAMS_INIT);
+   entity->ops->configure(entity, pipe, dl,
+  VSP1_ENTITY_PARAMS_RUNTIME);
+   entity->ops->configure(entity, pipe, dl,
+  VSP1_ENTITY_PARAMS_PARTITION);
+   }
+   }
+
+   vsp1_dl_list_commit(dl);
+}
+
 /* 
-
  * DU Driver API
  */
@@ -85,9 +126,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
struct vsp1_drm_pipeline *drm_pipe;
struct vsp1_pipeline *pipe;
struct vsp1_bru *bru;
-   struct vsp1_entity *entity;
-   struct vsp1_entity *next;
-   struct vsp1_dl_list *dl;
struct v4l2_subdev_format format;
unsigned long flags;
unsigned int i;
@@ -239,22 +277,7 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
vsp1_write(vsp1, VI6_DISP_IRQ_ENB, 0);
 
/* Configure all entities in the pipeline. */
-   dl = vsp1_dl_list_get(pipe->output->dlm);
-
-   list_for_each_entry_safe(entity, next, >entities, list_pipe) {
-   vsp1_entity_route_setup(entity, pipe, dl);
-
-   if (entity->ops->configure) {
-   entity->ops->configure(entity, pipe, dl,
-  VSP1_ENTITY_PARAMS_INIT);
-   entity->ops->configure(entity, pipe, dl,
-  VSP1_ENTITY_PARAMS_RUNTIME);
-   entity->ops->configure(entity, pipe, dl,
-  VSP1_ENTITY_PARAMS_PARTITION);
-   }
-   }
-
-   vsp1_dl_list_commit(dl);
+   vsp1_du_pipeline_configure(pipe);
 
/* Start the pipeline. */
spin_lock_irqsave(>irqlock, flags);
@@ -490,15 +513,9 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
struct vsp1_pipeline *pipe = _pipe->pipe;
struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, };
struct vsp1_bru *bru = to_bru(>bru->subdev);
-   struct vsp1_entity *entity;
-   struct vsp1_entity *next;
-   struct vsp1_dl_list *dl;
unsigned int i;
int ret;
 
-   /* Prepare the display list. */
-   dl = vsp1_dl_list_get(pipe->output->dlm);
-
/* Count the number of enabled inputs and sort them by Z-order. */
pipe->num_inputs = 0;
 
@@ -557,33 +574,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
__func__, rpf->entity.index);
}
 
-   /* Configure all entities in the pipeline. */
-   list_for_each_entry_safe(entity, next, >entities, list_pipe) {
-   /* Disconnect unused RPFs from the pipeline. */
-   if (entity->type == VSP1_ENTITY_RPF &&
-   !pipe->inputs[entity->index]) {
-   vsp1_dl_list_write(dl, entity->route->reg,
-   

[PATCH v2 00/15] R-Car VSP1: Dynamically assign blend units to display pipelines

[Laurent Pinchart]

Hello,

On R-Car H3 ES2.0+ and M3-N SoCs, two display pipelines are served by the same
VSP instance (named VSPDL). The VSPDL includes two blending units named BRU
and BRS, unlike the other display-related VSPD that serves a single display
channel with a single blending unit.

The VSPDL has five inputs and can freely assign them at runtime to two display
pipelines, using the BRU and BRS to blend multiple inputs. The BRU supports
blending up to five inputs, while the BRS is limited to two inputs.

Each display pipeline requires a blending unit, and the BRU and BRS are
currently assigned statically to the first and second pipeline respectively.
This artificially limits the number of inputs for the second pipeline to two.
To overcome that limitation, the BRU and BRS need to be dynamically assigned
to the display pipelines, which is what this patch series does.

Patches 01/15 to 10/15 perform small cleanups and refactoring to prepare for
the rest of the series. Patches 11/15 and 12/15 implement new internal
features for the same purpose, and patch 13/15 performs the bulk of the work
by implementing dynamic BRU and BRS assignment to pipelines.

Reassigning the BRU and BRS when two pipelines are running results in flicker
as one pipeline has to first release its blending unit. Synchronization
between the two pipelines also require locking that effectively serializes
page flips for the two pipelines, even when not interacting with each other.
This is currently believed to be unavoidable due to the hardware design, but
please feel free to prove me wrong (ideally with a patch - one can always
dream).

Patch 14/15 then adds messages useful for debugging this new feature. I have
kept it separate from 13/15 to make it easier to remove those messages once
dynamic assignment of blending units will be deemed perfectly stable, but I
won't oppose squashing it with 13/15 if that is preferred.

Patch 15/15 finally rename BRU to BRx to avoid confusion between the BRU terms
that refer to the BRU in particular, and the ones that refer to any of the BRU
or BRS. As this might be a bit controversial I've put the patch last in the
series in case it needs to be dropped.

I have decided to CC the dri-devel mailing list even though the code doesn't
touch the R-Car DU driver and will be merged through the Linux media tree as
the display is involved and the series could benefit from the expertise of the
DRM/KMS community from that point of view.

The patches are based on top of the latest Linux media master branch. For
convenience their are available from

git://linuxtv.org/pinchartl/media.git v4l2-vsp1-bru-brs-v2-20180405

The series passes the DU test suite with the new BRx dynamic assignment
test available from

git://git.ideasonboard.com/renesas/kms-tests.git bru-brs

The VSP test suite also runs without any noticed regression.

Since v1, patch 10/15 has been added an the v1 02/15 patch merged with
01/15. Other small changes are documented in individual changelogs.

Laurent Pinchart (15):
  v4l: vsp1: Don't start/stop media pipeline for DRM
  v4l: vsp1: Remove unused field from vsp1_drm_pipeline structure
  v4l: vsp1: Store pipeline pointer in vsp1_entity
  v4l: vsp1: Use vsp1_entity.pipe to check if entity belongs to a
pipeline
  v4l: vsp1: Share duplicated DRM pipeline configuration code
  v4l: vsp1: Move DRM atomic commit pipeline setup to separate function
  v4l: vsp1: Setup BRU at atomic commit time
  v4l: vsp1: Replace manual DRM pipeline input setup in
vsp1_du_setup_lif
  v4l: vsp1: Move DRM pipeline output setup code to a function
  v4l: vsp1: Turn frame end completion status into a bitfield
  v4l: vsp1: Add per-display list internal completion notification
support
  v4l: vsp1: Generalize detection of entity removal from DRM pipeline
  v4l: vsp1: Assign BRU and BRS to pipelines dynamically
  v4l: vsp1: Add BRx dynamic assignment debugging messages
  v4l: vsp1: Rename BRU to BRx

 drivers/media/platform/vsp1/Makefile   |   2 +-
 drivers/media/platform/vsp1/vsp1.h |   6 +-
 .../media/platform/vsp1/{vsp1_bru.c => vsp1_brx.c} | 202 ++---
 .../media/platform/vsp1/{vsp1_bru.h => vsp1_brx.h} |  18 +-
 drivers/media/platform/vsp1/vsp1_dl.c  |  45 +-
 drivers/media/platform/vsp1/vsp1_dl.h  |   7 +-
 drivers/media/platform/vsp1/vsp1_drm.c | 828 -
 drivers/media/platform/vsp1/vsp1_drm.h |  16 +-
 drivers/media/platform/vsp1/vsp1_drv.c |   8 +-
 drivers/media/platform/vsp1/vsp1_entity.h  |   2 +
 drivers/media/platform/vsp1/vsp1_histo.c   |   2 +-
 drivers/media/platform/vsp1/vsp1_histo.h   |   3 -
 drivers/media/platform/vsp1/vsp1_pipe.c|  53 +-
 drivers/media/platform/vsp1/vsp1_pipe.h|   6 +-
 drivers/media/platform/vsp1/vsp1_rpf.c |  12 +-
 drivers/media/platform/vsp1/vsp1_rwpf.h|   4 +-
 drivers/media/platform/vsp1/vsp1_v

Re: [PATCH v3 36/40] drm/i915: Implement gmbus burst read

[Jani Nikula]

On Tue, 03 Apr 2018, Daniel Vetter  wrote:
> On Tue, Apr 03, 2018 at 07:27:49PM +0530, Ramalingam C wrote:
>> Implements a interface for single burst read of data that is larger
>> than 512 Bytes through gmbus.

Where does 512 come from? Current code chunks at GMBUS_BYTE_COUNT_MAX
i.e. 256 bytes. Should GMBUS_BYTE_COUNT_MAX be platform specific?

>> 
>> HDCP2.2 spec expects HDCP2.2 transmitter to read 522Bytes of HDCP
>> receiver certificates in single burst read. On gmbus, to read more
>> than 511Bytes, HW provides a workaround for burst read.
>> 
>> This patch passes the burst read request through gmbus read functions.
>> And implements the sequence of enabling and disabling the burst read.
>> 
>> v2:
>>   No Changes.
>> v3:
>>   No Changes.
>> 
>> Signed-off-by: Ramalingam C 
>
> Why only enable this burst_read mode for hdcp, and not for all i2c
> transactions? Seems to unecessarily complicate the code, since it requires
> that you pass burst_read through the entire call chain. For other changes
> we've done for hdcp (like enabling the read/write mode and other stuff)
> we've enabled it for all i2c transactions. That also means more testing,
> since it will be used even when HDCP is not in use.

Agreed.

Shouldn't the decision to use burst read be based on the length to be
read? More than 256 bytes, use burst, otherwise not. Or are there
functional differences or benefits to using burst mode for shorter
reads?

I guess I'd still prepare to add some burst field to struct intel_gmbus,
not unlike force_bit.

Side note, I would prefer significant changes to basic plumbing like
gmbus be highlighted better. Please at least prefix the hdcp patches
with "drm/i915/hdcp" and gmbus with "drm/i915/gmbus". The patch at hand
is kind of hidden in the middle of a large topical series, and would
deserve to be looked at by people who aren't necessarily all that into
hdcp.


BR,
Jani.


> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |   2 +
>>  drivers/gpu/drm/i915/i915_reg.h  |   3 +
>>  drivers/gpu/drm/i915/intel_i2c.c | 124 
>> +--
>>  3 files changed, 112 insertions(+), 17 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 6e740f6fe33f..72534a1e544b 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3688,6 +3688,8 @@ extern void intel_teardown_gmbus(struct 
>> drm_i915_private *dev_priv);
>>  extern bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
>>   unsigned int pin);
>>  extern int intel_gmbus_output_aksv(struct i2c_adapter *adapter);
>> +extern int intel_gmbus_burst_read(struct i2c_adapter *adapter,
>> +  unsigned int offset, void *buf, size_t size);
>>  
>>  extern struct i2c_adapter *
>>  intel_gmbus_get_adapter(struct drm_i915_private *dev_priv, unsigned int 
>> pin);
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index f04ad3c15abd..56979bc4e9d8 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3123,6 +3123,7 @@ enum i915_power_well_id {
>>  #define   GMBUS_RATE_400KHZ (2<<8) /* reserved on Pineview */
>>  #define   GMBUS_RATE_1MHZ   (3<<8) /* reserved on Pineview */
>>  #define   GMBUS_HOLD_EXT(1<<7) /* 300ns hold time, rsvd on Pineview */
>> +#define   GMBUS_BYTE_CNT_OVERRIDE (1<<6)
>>  #define   GMBUS_PIN_DISABLED0
>>  #define   GMBUS_PIN_SSC 1
>>  #define   GMBUS_PIN_VGADDC  2
>> @@ -3150,8 +3151,10 @@ enum i915_power_well_id {
>>  #define   GMBUS_CYCLE_WAIT  (1<<25)
>>  #define   GMBUS_CYCLE_INDEX (2<<25)
>>  #define   GMBUS_CYCLE_STOP  (4<<25)
>> +#define   GMBUS_CYCLE_MASK  (7<<25)
>>  #define   GMBUS_BYTE_COUNT_SHIFT 16
>>  #define   GMBUS_BYTE_COUNT_MAX   256U
>> +#define   GMBUS_BYTE_COUNT_HW_MAX 511U
>>  #define   GMBUS_SLAVE_INDEX_SHIFT 8
>>  #define   GMBUS_SLAVE_ADDR_SHIFT 1
>>  #define   GMBUS_SLAVE_READ  (1<<0)
>> diff --git a/drivers/gpu/drm/i915/intel_i2c.c 
>> b/drivers/gpu/drm/i915/intel_i2c.c
>> index e6875509bcd9..dcb2be0d54ee 100644
>> --- a/drivers/gpu/drm/i915/intel_i2c.c
>> +++ b/drivers/gpu/drm/i915/intel_i2c.c
>> @@ -364,21 +364,30 @@ gmbus_wait_idle(struct drm_i915_private *dev_priv)
>>  static int
>>  gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv,
>>unsigned short addr, u8 *buf, unsigned int len,
>> -  u32 gmbus1_index)
>> +  u32 gmbus1_index, bool burst_read)
>>  {
>> +unsigned int size = len;
>> +int ret;
>> +
>> +if (burst_read) {
>> +/* Seq to enable Burst Read */
>> +I915_WRITE_FW(GMBUS0, (I915_READ_FW(GMBUS0) |
>> +  GMBUS_BYTE_CNT_OVERRIDE));
>> +size = GMBUS_BYTE_COUNT_HW_MAX;
>> +}
>> +
>>  I915_WRITE_FW(GMBUS1,
>>gmbus1_index |
>>

Re: [RFC 1/3] drm: Add DAMAGE_CLIPS property to plane

[Thomas Hellstrom]

On 04/05/2018 09:35 AM, Daniel Vetter wrote:

On Wed, Apr 04, 2018 at 04:49:06PM -0700, Deepak Rawat wrote:

From: Lukasz Spintzyk 

Optional plane property to mark damaged regions on the plane in
framebuffer coordinates of the framebuffer attached to the plane.

The layout of blob data is simply an array of drm_mode_rect with maximum
array size limited by DRM_MODE_FB_DIRTY_MAX_CLIPS. Unlike plane src
coordinates, damage clips are not in 16.16 fixed point.

Damage clips are a hint to kernel as which area of framebuffer has
changed since last page-flip. This should be helpful for some drivers
especially for virtual devices where each framebuffer change needs to
be transmitted over network, usb, etc.

Driver which are interested in enabling DAMAGE_CLIPS property for a
plane should enable this property using drm_plane_enable_damage_clips.

Signed-off-by: Lukasz Spintzyk 
Signed-off-by: Deepak Rawat 

The property uapi section is missing, see:

https://urldefense.proofpoint.com/v2/url?u=https-3A__dri.freedesktop.org_docs_drm_gpu_drm-2Dkms.html-23plane-2Dcomposition-2Dproperties=DwIBAg=uilaK90D4TOVoH58JNXRgQ=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ=J31YNV1uz3IVRaorM-eo48msi9__sQurqRZrig2UE1s=vzsuquF1agbOw54HSC_18Kk2k9j83m8RusmDCtPAFWQ=

Plane composition feels like the best place to put this. Please use that
section to tie all the various bits together, including the helpers you're
adding in the following patches for drivers to use.

Bunch of nitpicks below, but overall I'm agreeing now with just going with
fb coordinate damage rects.

Like you say, the thing needed here now is userspace + driver actually
implementing this. I'd also say the compat helper to map the legacy
fb->dirty to this new atomic way of doing things should be included here
(gives us a lot more testing for these new paths).

Icing on the cake would be an igt to make sure kernel rejects invalid clip
rects correctly.


---
  drivers/gpu/drm/drm_atomic.c| 42 +
  drivers/gpu/drm/drm_atomic_helper.c |  4 
  drivers/gpu/drm/drm_mode_config.c   |  5 +
  drivers/gpu/drm/drm_plane.c | 12 +++
  include/drm/drm_mode_config.h   | 15 +
  include/drm/drm_plane.h | 16 ++
  include/uapi/drm/drm_mode.h | 15 +
  7 files changed, 109 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7d25c42..9226d24 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -669,6 +669,40 @@ static void drm_atomic_crtc_print_state(struct drm_printer 
*p,
  }
  
  /**

+ * drm_atomic_set_damage_for_plane - sets the damage clips property to plane
+ * @state: plane state
+ * @blob: damage clips in framebuffer coordinates
+ *
+ * Returns:
+ *
+ * Zero on success, error code on failure.
+ */
+static int drm_atomic_set_damage_for_plane(struct drm_plane_state *state,
+  struct drm_property_blob *blob)
+{
+   if (blob == state->damage_clips)
+   return 0;
+
+   drm_property_blob_put(state->damage_clips);
+   state->damage_clips = NULL;
+
+   if (blob) {
+   uint32_t count = blob->length/sizeof(struct drm_rect);
+
+   if (count > DRM_MODE_FB_DIRTY_MAX_CLIPS)
+   return -EINVAL;
+
+   state->damage_clips = drm_property_blob_get(blob);
+   state->num_clips = count;
+   } else {
+   state->damage_clips = NULL;
+   state->num_clips = 0;
+   }
+
+   return 0;
+}
+
+/**
   * drm_atomic_get_plane_state - get plane state
   * @state: global atomic state object
   * @plane: plane to get state object for
@@ -793,6 +827,12 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
state->color_encoding = val;
} else if (property == plane->color_range_property) {
state->color_range = val;
+   } else if (property == config->prop_damage_clips) {
+   struct drm_property_blob *blob =
+   drm_property_lookup_blob(dev, val);
+   int ret = drm_atomic_set_damage_for_plane(state, blob);

There's already a helper with size-checking built-in, see
drm_atomic_replace_property_blob_from_id(). Wrt computing num_clips I'd
just provide a little inline helper that does the
blob->length/sizeof(drm_rect) conversion (it's just a shift, so fast).


+   drm_property_blob_put(blob);
+   return ret;
} else if (plane->funcs->atomic_set_property) {
return plane->funcs->atomic_set_property(plane, state,
property, val);
@@ -856,6 +896,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->color_encoding;
} else if (property == plane->color_range_property) {
 

Re: [RFC 2/3] drm: Add helper iterator functions to iterate over plane damage.

[Thomas Hellstrom]

On 04/05/2018 09:52 AM, Daniel Vetter wrote:


TYPE_PLANE I have no idea who needs that. I suggest we just drop it.


I'm assuming CRTC plane coordinates here. They are used for uploading 
contents of hardware planes. Like, in the simplest case, cursor images.


/Thomas

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

Re: [RFC 2/3] drm: Add helper iterator functions to iterate over plane damage.

[Thomas Hellstrom]

On 04/05/2018 09:52 AM, Daniel Vetter wrote:

On Wed, Apr 04, 2018 at 04:49:07PM -0700, Deepak Rawat wrote:

With damage property in drm_plane_state, this patch adds helper iterator
to traverse the damage clips. Iterator will return the damage rectangles
in framebuffer, plane or crtc coordinates as need by driver
implementation.

Signed-off-by: Deepak Rawat 

I'd really like selftest/unittests for this stuff. There's an awful lot of
cornercases in this here (for any of the transformed iterators at least),
and unit tests is the best way to make sure we handle them all correctly.

Bonus points if you integrate the new selftests into igt so intel CI can
run them, seel igt/tests/drm_mm.c for an example. drm_mm selftest is also
the framework I'd copy for this stuff.


---
  drivers/gpu/drm/drm_atomic_helper.c | 122 
  include/drm/drm_atomic_helper.h |  39 
  2 files changed, 161 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 55b44e3..355b514 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3865,3 +3865,125 @@ void 
__drm_atomic_helper_private_obj_duplicate_state(struct drm_private_obj *obj
memcpy(state, obj->state, sizeof(*state));
  }
  EXPORT_SYMBOL(__drm_atomic_helper_private_obj_duplicate_state);
+
+/**
+ * drm_atomic_helper_damage_iter_init - initialize the damage iterator
+ * @iter: The iterator to initialize.
+ * @type: Coordinate type caller is interested in.
+ * @state: plane_state from which to iterate the damage clips.
+ * @hdisplay: Width of crtc on which plane is scanned out.
+ * @vdisplay: Height of crtc on which plane is scanned out.
+ *
+ * Initialize an iterator that is used to translate and clip a set of damage
+ * rectangles in framebuffer coordinates to plane and crtc coordinates. The 
type
+ * argument specify which type of coordinate to iterate in.
+ *
+ * Returns: 0 on success and negative error code on error. If an error code is
+ * returned then it means the plane state should not update.
+ */
+int
+drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
+  enum drm_atomic_helper_damage_clip_type type,
+  const struct drm_plane_state *state,
+  uint32_t hdisplay, uint32_t vdisplay)
+{
+   if (!state || !state->crtc || !state->fb)
+   return -EINVAL;
+
+   memset(iter, 0, sizeof(*iter));
+   iter->clips = (struct drm_rect *)state->damage_clips->data;
+   iter->num_clips = state->num_clips;
+   iter->type = type;
+
+   /*
+* Full update in case of scaling or rotation. In future support for
+* scaling/rotating damage clips can be added
+*/
+   if (state->crtc_w != (state->src_w >> 16) ||
+   state->crtc_h != state->src_h >> 16 || state->rotation != 0) {
+   iter->curr_clip = iter->num_clips;
+   return 0;

Given there's no user of this I have no idea how this manages to provoke a
full clip rect. selftest code would be perfect for stuff like this.

Also, I think we should provide a full clip for the case of num_clips ==
0, so that driver code can simply iterate over all clips and doesn't ever
have to handle the "no clip rects provided" case as a special case itself.


+   }
+
+   iter->fb_src.x1 = 0;
+   iter->fb_src.y1 = 0;
+   iter->fb_src.x2 = state->fb->width;
+   iter->fb_src.y2 = state->fb->height;
+
+   iter->plane_src.x1 = state->src_x >> 16;
+   iter->plane_src.y1 = state->src_y >> 16;
+   iter->plane_src.x2 = iter->plane_src.x1 + (state->src_w >> 16);
+   iter->plane_src.y2 = iter->plane_src.y1 + (state->src_h >> 16);
+   iter->translate_plane_x = -iter->plane_src.x1;
+   iter->translate_plane_y = -iter->plane_src.y1;
+
+   /* Clip plane src rect to fb dimensions */
+   drm_rect_intersect(>plane_src, >fb_src);

This smells like driver bug. Also, see Ville's recent efforts to improve
the atomic plane clipping, I think drm_plane_state already has all the
clip rects you want.


+
+   iter->crtc_src.x1 = 0;
+   iter->crtc_src.y1 = 0;
+   iter->crtc_src.x2 = hdisplay;
+   iter->crtc_src.y2 = vdisplay;
+   iter->translate_crtc_x = -(iter->plane_src.x1 - state->crtc_x);
+   iter->translate_crtc_x = -(iter->plane_src.y1 - state->crtc_y);
+
+   /* Clip crtc src rect to plane dimensions */
+   drm_rect_translate(>crtc_src, -iter->translate_crtc_x,
+  -iter->translate_crtc_x);

We can also scale.


I suggest we leave out scaling for now until someone actually needs it.




+   drm_rect_intersect(>crtc_src, >plane_src);
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_damage_iter_init);
+
+/**
+ * drm_atomic_helper_damage_iter_next - advance the damage iterator
+ * @iter: The 

Re: [PATCH 11/15] v4l: vsp1: Add per-display list completion notification support

[Kieran Bingham]

Hi Laurent,

On 04/04/18 22:43, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Wednesday, 4 April 2018 19:16:46 EEST Kieran Bingham wrote:
>> On 26/02/18 21:45, Laurent Pinchart wrote:



>>>
>>> -void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
>>> +void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool notify)
>>
>> Rather than changing the vsp1_dl_list_commit() function - would it be nicer
>> to have an API to request or set the notify property? :
>>
>> @@..@@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
>> ...
>> +/* The BRx will be released, without sending an update to DRM */
>> +if (drm_pipe->force_bru_release)
>> +vsp1_dl_list_request_internal_notify(dl);
>>
>>  vsp1_dl_list_commit(dl);
>> ...
> 
> That's not a bad idea, but I wonder if it's worth it as we'll have to call an 
> extra function for what is essentially an internal API. On the other hand 
> this 
> isn't a common case, so it's not a hot code path. We could also argue equally 
> that it is the commit that is internal or that it is the display list that is 

Aha, yes - it is more so that the commit is internal ...

> for internal purpose. Do you think an extra function call is better ? If you 
> do I'll change it.

so it could also instead be just a separate commit() function:


void vsp1_dl_list_commit_internal(struct vsp1_dl_list *dl)
{
dl->internal = true;
vsp1_dl_list_commit(dl);
}


...


{
/* The BRx will be released, without sending an update to DRM */
if (drm_pipe->force_bru_release)
vsp1_dl_list_commit_internal(dl);
else
vsp1_dl_list_commit(dl);
}

I'll leave the final implementation decision with you - I just thought that
extending the commit call with a notify flag seemed odd.

Of course - that could also have been due to the naming of the 'notify' - if it
was 'internal' as discussed in the other patches, then perhaps a flag on the
function call is still a sensible way. It just affects the other commit usages,
but there's only a total of three calls - so it's really not a big deal. If
there were 12 call locations perhaps the function wrapper would have more merit
- but probably not so much at 3 :D

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

Re: Closing and repoening laptop lid causes scanout corruption (regression since 4.15.12)

[Jani Nikula]

On Wed, 04 Apr 2018, Wolfgang Draxinger  wrote:
> ever since I upgraded my system to kernel version 4.15.12 I'm
> experiencing a kind of interesting problem with my laptop's graphics
> output.

Please file a bug at [1], including the description below. Add
drm.debug=14 module parameter, attach dmesg from boot.


BR,
Jani.

[1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI=DRM/Intel



-- 
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] drm/omap: fix crash if there's no video PLL

[Laurent Pinchart]

Hi Tomi,

On Thursday, 5 April 2018 09:55:37 EEST Tomi Valkeinen wrote:
> Commit 8a7eda7686675b73d74c22c0d5b83059f9d783f6 ("drm: omapdrm: dispc:
> Pass DISPC pointer to remaining dispc API functions") made dpi.c use
> ctx->pll even when there's no PLL, causing a crash at modeset on AM4
> EVM, and presumably all OMAP2/3 boards.
> 
> Fix this by having struct dpi_data pointer in the ctx instead, giving
> access to dispc without going through the pll.
> 
> Signed-off-by: Tomi Valkeinen 
> Reported-by: Keerthy 
> Cc: Laurent Pinchart 

Oops, sorry about this :-/

You should add a

Fixes: 8a7eda768667 ("drm: omapdrm: dispc: Pass DISPC pointer to remaining 
dispc API functions")

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/omapdrm/dss/dpi.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c
> b/drivers/gpu/drm/omapdrm/dss/dpi.c index fb1c27f69e3a..3d662e6805eb 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
> @@ -142,7 +142,7 @@ static enum dss_clk_source dpi_get_clk_src(struct
> dpi_data *dpi) }
> 
>  struct dpi_clk_calc_ctx {
> - struct dss_pll *pll;
> + struct dpi_data *dpi;
>   unsigned int clkout_idx;
> 
>   /* inputs */
> @@ -191,7 +191,7 @@ static bool dpi_calc_hsdiv_cb(int m_dispc, unsigned long
> dispc, ctx->pll_cinfo.mX[ctx->clkout_idx] = m_dispc;
>   ctx->pll_cinfo.clkout[ctx->clkout_idx] = dispc;
> 
> - return dispc_div_calc(ctx->pll->dss->dispc, dispc,
> + return dispc_div_calc(ctx->dpi->dss->dispc, dispc,
> ctx->pck_min, ctx->pck_max,
> dpi_calc_dispc_cb, ctx);
>  }
> @@ -208,8 +208,8 @@ static bool dpi_calc_pll_cb(int n, int m, unsigned long
> fint, ctx->pll_cinfo.fint = fint;
>   ctx->pll_cinfo.clkdco = clkdco;
> 
> - return dss_pll_hsdiv_calc_a(ctx->pll, clkdco,
> - ctx->pck_min, dss_get_max_fck_rate(ctx->pll->dss),
> + return dss_pll_hsdiv_calc_a(ctx->dpi->pll, clkdco,
> + ctx->pck_min, dss_get_max_fck_rate(ctx->dpi->dss),
>   dpi_calc_hsdiv_cb, ctx);
>  }
> 
> @@ -219,7 +219,7 @@ static bool dpi_calc_dss_cb(unsigned long fck, void
> *data)
> 
>   ctx->fck = fck;
> 
> - return dispc_div_calc(ctx->pll->dss->dispc, fck,
> + return dispc_div_calc(ctx->dpi->dss->dispc, fck,
> ctx->pck_min, ctx->pck_max,
> dpi_calc_dispc_cb, ctx);
>  }
> @@ -230,7 +230,7 @@ static bool dpi_pll_clk_calc(struct dpi_data *dpi,
> unsigned long pck, unsigned long clkin;
> 
>   memset(ctx, 0, sizeof(*ctx));
> - ctx->pll = dpi->pll;
> + ctx->dpi = dpi;
>   ctx->clkout_idx = dss_pll_get_clkout_idx_for_src(dpi->clk_src);
> 
>   clkin = clk_get_rate(dpi->pll->clkin);
> @@ -244,7 +244,7 @@ static bool dpi_pll_clk_calc(struct dpi_data *dpi,
> unsigned long pck, pll_min = 0;
>   pll_max = 0;
> 
> - return dss_pll_calc_a(ctx->pll, clkin,
> + return dss_pll_calc_a(ctx->dpi->pll, clkin,
>   pll_min, pll_max,
>   dpi_calc_pll_cb, ctx);
>   } else { /* DSS_PLL_TYPE_B */
> @@ -275,6 +275,7 @@ static bool dpi_dss_clk_calc(struct dpi_data *dpi,
> unsigned long pck, bool ok;
> 
>   memset(ctx, 0, sizeof(*ctx));
> + ctx->dpi = dpi;
>   if (pck > 1000 * i * i * i)
>   ctx->pck_min = max(pck - 1000 * i * i * i, 0lu);
>   else

-- 
Regards,

Laurent Pinchart



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

[Bug 105883] booting with kernel using amd-staging-drm-next on 2400G hangs

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105883

--- Comment #5 from Joshua Lee  ---
(In reply to Edward Kigwana from comment #4)
> Try 
> 
> options amdgpu dpm=0 dc=1 and seee if it still locks up.

That's in /etc/modconf.d or the like, right?

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 3/3] drm: Add helper to validate damage during modeset_check

[Daniel Vetter]

On Wed, Apr 04, 2018 at 04:49:08PM -0700, Deepak Rawat wrote:
> This patch adds a helper which should be called by driver which enable
> damage (by calling drm_plane_enable_damage_clips) from atomic_check
> hook. This helper for now set the damage to NULL for the planes on crtc
> which need full modeset.
> 
> The driver also need to check for other crtc properties which can
> affect damage in atomic_check hook, like gamma, ctm, etc. Plane related
> properties which affect damage can be handled in damage iterator.
> 
> Signed-off-by: Deepak Rawat 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 47 
> +
>  include/drm/drm_atomic_helper.h |  2 ++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 355b514..23f44ab 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3987,3 +3987,50 @@ drm_atomic_helper_damage_iter_next(struct 
> drm_atomic_helper_damage_iter *iter,
>   return true;
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_damage_iter_next);
> +
> +/**
> + * drm_atomic_helper_check_damage - validate state object for damage changes
> + * @dev: DRM device
> + * @state: the driver state object
> + *
> + * Check the state object if damage is required or not. In case damage is not
> + * required e.g. need modeset, the damage blob is set to NULL.

Why is that needed?

I can imagine that drivers unconditionally upload everything for a
modeset, and hence don't need special damage tracking. But for that it's
imo better to have a clear_damage() helper.

But even for modesets (e.g. resolution changes) I'd expect that virtual
drivers don't want to upload unecessary amounts of data. Manual upload
hw drivers probably need to upload everything, because the panel will have
forgotten all the old data once power cycled.

And if you think this is really the right thing, then we need to rename
the function to tell what it does, e.g.

drm_damage_helper_clear_damage_on_modeset()

drm_damage_helper because I think we should stuff this all into
drm_damage_helper.c, see previous patch.

But I think a

drm_damage_helper_clear_damage(crtc_state)

which you can use in your crtc->atomic_check function like

crtc_atomic_check(state)
{
if (drm_atomic_crtc_needs_modeset(state))
drm_damage_helper_clear_damage(state);
}

is more flexible and useful for drivers. There might be other cases where
clearing damage is the right thing to do. Also, there's the question of
whether no damage clips == full damage or not, so maybe we should call
this helper full_damage() instead.
-Daniel

> + *
> + * NOTE: This helper is not called by core. Those driver which enable damage
> + * using drm_plane_enable_damage_clips should call this from their 
> @atomic_check
> + * hook.
> + */
> +int drm_atomic_helper_check_damage(struct drm_device *dev,
> +struct drm_atomic_state *state)
> +{
> + struct drm_crtc *crtc;
> + struct drm_plane *plane;
> + struct drm_crtc_state *crtc_state;
> + unsigned plane_mask;
> + int i;
> +
> + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> + if (!drm_atomic_crtc_needs_modeset(crtc_state))
> + continue;
> +
> + plane_mask = crtc_state->plane_mask;
> + plane_mask |= crtc->state->plane_mask;
> +
> + drm_for_each_plane_mask(plane, dev, plane_mask) {
> + struct drm_plane_state *plane_state =
> + drm_atomic_get_plane_state(state, plane);
> +
> + if (IS_ERR(plane_state))
> + return PTR_ERR(plane_state);
> +
> + if (plane_state->damage_clips) {
> + 
> drm_property_blob_put(plane_state->damage_clips);
> + plane_state->damage_clips = NULL;
> + plane_state->num_clips = 0;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_damage);
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index ebd4b66..b12335c 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -224,6 +224,8 @@ drm_atomic_helper_damage_iter_init(struct 
> drm_atomic_helper_damage_iter *iter,
>  bool
>  drm_atomic_helper_damage_iter_next(struct drm_atomic_helper_damage_iter 
> *iter,
>  struct drm_rect *rect);
> +int drm_atomic_helper_check_damage(struct drm_device *dev,
> +struct drm_atomic_state *state);
>  
>  /**
>   * drm_atomic_crtc_for_each_plane - iterate over planes currently attached 
> to CRTC
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC 2/3] drm: Add helper iterator functions to iterate over plane damage.

[Daniel Vetter]

On Wed, Apr 04, 2018 at 04:49:07PM -0700, Deepak Rawat wrote:
> With damage property in drm_plane_state, this patch adds helper iterator
> to traverse the damage clips. Iterator will return the damage rectangles
> in framebuffer, plane or crtc coordinates as need by driver
> implementation.
> 
> Signed-off-by: Deepak Rawat 

I'd really like selftest/unittests for this stuff. There's an awful lot of
cornercases in this here (for any of the transformed iterators at least),
and unit tests is the best way to make sure we handle them all correctly.

Bonus points if you integrate the new selftests into igt so intel CI can
run them, seel igt/tests/drm_mm.c for an example. drm_mm selftest is also
the framework I'd copy for this stuff.

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 122 
> 
>  include/drm/drm_atomic_helper.h |  39 
>  2 files changed, 161 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 55b44e3..355b514 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3865,3 +3865,125 @@ void 
> __drm_atomic_helper_private_obj_duplicate_state(struct drm_private_obj *obj
>   memcpy(state, obj->state, sizeof(*state));
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_private_obj_duplicate_state);
> +
> +/**
> + * drm_atomic_helper_damage_iter_init - initialize the damage iterator
> + * @iter: The iterator to initialize.
> + * @type: Coordinate type caller is interested in.
> + * @state: plane_state from which to iterate the damage clips.
> + * @hdisplay: Width of crtc on which plane is scanned out.
> + * @vdisplay: Height of crtc on which plane is scanned out.
> + *
> + * Initialize an iterator that is used to translate and clip a set of damage
> + * rectangles in framebuffer coordinates to plane and crtc coordinates. The 
> type
> + * argument specify which type of coordinate to iterate in.
> + *
> + * Returns: 0 on success and negative error code on error. If an error code 
> is
> + * returned then it means the plane state should not update.
> + */
> +int
> +drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter 
> *iter,
> +enum drm_atomic_helper_damage_clip_type type,
> +const struct drm_plane_state *state,
> +uint32_t hdisplay, uint32_t vdisplay)
> +{
> + if (!state || !state->crtc || !state->fb)
> + return -EINVAL;
> +
> + memset(iter, 0, sizeof(*iter));
> + iter->clips = (struct drm_rect *)state->damage_clips->data;
> + iter->num_clips = state->num_clips;
> + iter->type = type;
> +
> + /*
> +  * Full update in case of scaling or rotation. In future support for
> +  * scaling/rotating damage clips can be added
> +  */
> + if (state->crtc_w != (state->src_w >> 16) ||
> + state->crtc_h != state->src_h >> 16 || state->rotation != 0) {
> + iter->curr_clip = iter->num_clips;
> + return 0;

Given there's no user of this I have no idea how this manages to provoke a
full clip rect. selftest code would be perfect for stuff like this.

Also, I think we should provide a full clip for the case of num_clips ==
0, so that driver code can simply iterate over all clips and doesn't ever
have to handle the "no clip rects provided" case as a special case itself.

> + }
> +
> + iter->fb_src.x1 = 0;
> + iter->fb_src.y1 = 0;
> + iter->fb_src.x2 = state->fb->width;
> + iter->fb_src.y2 = state->fb->height;
> +
> + iter->plane_src.x1 = state->src_x >> 16;
> + iter->plane_src.y1 = state->src_y >> 16;
> + iter->plane_src.x2 = iter->plane_src.x1 + (state->src_w >> 16);
> + iter->plane_src.y2 = iter->plane_src.y1 + (state->src_h >> 16);
> + iter->translate_plane_x = -iter->plane_src.x1;
> + iter->translate_plane_y = -iter->plane_src.y1;
> +
> + /* Clip plane src rect to fb dimensions */
> + drm_rect_intersect(>plane_src, >fb_src);

This smells like driver bug. Also, see Ville's recent efforts to improve
the atomic plane clipping, I think drm_plane_state already has all the
clip rects you want.

> +
> + iter->crtc_src.x1 = 0;
> + iter->crtc_src.y1 = 0;
> + iter->crtc_src.x2 = hdisplay;
> + iter->crtc_src.y2 = vdisplay;
> + iter->translate_crtc_x = -(iter->plane_src.x1 - state->crtc_x);
> + iter->translate_crtc_x = -(iter->plane_src.y1 - state->crtc_y);
> +
> + /* Clip crtc src rect to plane dimensions */
> + drm_rect_translate(>crtc_src, -iter->translate_crtc_x,
> +-iter->translate_crtc_x);

We can also scale.

> + drm_rect_intersect(>crtc_src, >plane_src);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_damage_iter_init);
> +
> +/**
> + * drm_atomic_helper_damage_iter_next - advance the damage iterator
> + * @iter: The iterator to 

Re: [RFC 1/3] drm: Add DAMAGE_CLIPS property to plane

[Daniel Vetter]

On Wed, Apr 04, 2018 at 04:49:06PM -0700, Deepak Rawat wrote:
> From: Lukasz Spintzyk 
> 
> Optional plane property to mark damaged regions on the plane in
> framebuffer coordinates of the framebuffer attached to the plane.
> 
> The layout of blob data is simply an array of drm_mode_rect with maximum
> array size limited by DRM_MODE_FB_DIRTY_MAX_CLIPS. Unlike plane src
> coordinates, damage clips are not in 16.16 fixed point.
> 
> Damage clips are a hint to kernel as which area of framebuffer has
> changed since last page-flip. This should be helpful for some drivers
> especially for virtual devices where each framebuffer change needs to
> be transmitted over network, usb, etc.
> 
> Driver which are interested in enabling DAMAGE_CLIPS property for a
> plane should enable this property using drm_plane_enable_damage_clips.
> 
> Signed-off-by: Lukasz Spintzyk 
> Signed-off-by: Deepak Rawat 

The property uapi section is missing, see:

https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-composition-properties

Plane composition feels like the best place to put this. Please use that
section to tie all the various bits together, including the helpers you're
adding in the following patches for drivers to use.

Bunch of nitpicks below, but overall I'm agreeing now with just going with
fb coordinate damage rects.

Like you say, the thing needed here now is userspace + driver actually
implementing this. I'd also say the compat helper to map the legacy
fb->dirty to this new atomic way of doing things should be included here
(gives us a lot more testing for these new paths).

Icing on the cake would be an igt to make sure kernel rejects invalid clip
rects correctly.

> ---
>  drivers/gpu/drm/drm_atomic.c| 42 
> +
>  drivers/gpu/drm/drm_atomic_helper.c |  4 
>  drivers/gpu/drm/drm_mode_config.c   |  5 +
>  drivers/gpu/drm/drm_plane.c | 12 +++
>  include/drm/drm_mode_config.h   | 15 +
>  include/drm/drm_plane.h | 16 ++
>  include/uapi/drm/drm_mode.h | 15 +
>  7 files changed, 109 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7d25c42..9226d24 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -669,6 +669,40 @@ static void drm_atomic_crtc_print_state(struct 
> drm_printer *p,
>  }
>  
>  /**
> + * drm_atomic_set_damage_for_plane - sets the damage clips property to plane
> + * @state: plane state
> + * @blob: damage clips in framebuffer coordinates
> + *
> + * Returns:
> + *
> + * Zero on success, error code on failure.
> + */
> +static int drm_atomic_set_damage_for_plane(struct drm_plane_state *state,
> +struct drm_property_blob *blob)
> +{
> + if (blob == state->damage_clips)
> + return 0;
> +
> + drm_property_blob_put(state->damage_clips);
> + state->damage_clips = NULL;
> +
> + if (blob) {
> + uint32_t count = blob->length/sizeof(struct drm_rect);
> +
> + if (count > DRM_MODE_FB_DIRTY_MAX_CLIPS)
> + return -EINVAL;
> +
> + state->damage_clips = drm_property_blob_get(blob);
> + state->num_clips = count;
> + } else {
> + state->damage_clips = NULL;
> + state->num_clips = 0;
> + }
> +
> + return 0;
> +}
> +
> +/**
>   * drm_atomic_get_plane_state - get plane state
>   * @state: global atomic state object
>   * @plane: plane to get state object for
> @@ -793,6 +827,12 @@ static int drm_atomic_plane_set_property(struct 
> drm_plane *plane,
>   state->color_encoding = val;
>   } else if (property == plane->color_range_property) {
>   state->color_range = val;
> + } else if (property == config->prop_damage_clips) {
> + struct drm_property_blob *blob =
> + drm_property_lookup_blob(dev, val);
> + int ret = drm_atomic_set_damage_for_plane(state, blob);

There's already a helper with size-checking built-in, see
drm_atomic_replace_property_blob_from_id(). Wrt computing num_clips I'd
just provide a little inline helper that does the
blob->length/sizeof(drm_rect) conversion (it's just a shift, so fast).

> + drm_property_blob_put(blob);
> + return ret;
>   } else if (plane->funcs->atomic_set_property) {
>   return plane->funcs->atomic_set_property(plane, state,
>   property, val);
> @@ -856,6 +896,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>   *val = state->color_encoding;
>   } else if (property == plane->color_range_property) {
>   *val = state->color_range;
> + } else if (property == config->prop_damage_clips) {
> + *val = (state->damage_clips) ? 

Re: [RFC 0/3] drm: page-flip with damage

[Daniel Vetter]

On Wed, Apr 04, 2018 at 04:49:05PM -0700, Deepak Rawat wrote:
> Hi All,
> 
> This is extension to Lukasz Spintzyk earlier draft of damage interface for 
> drm.
> Bascially a new plane property is added called "DAMAGE_CLIPS" which is simply
> an array of drm_rect (exported to userspace as drm_mode_rect). The clips
> represents damage in framebuffer coordinate of attached fb to the plane.
> 
> Helper iterator is added to traverse the damage rectangles and get the damage
> clips in framebuffer, plane or crtc coordinates as need by driver
> implementation. Finally a helper to reset damage in case need full update is
> required. Drivers interested in page-flip with damage should call this from
> atomic_check hook.
> 
> With the RFC for atomic implementation of dirtyfb ioctl I was thinking
> should we need to consider dirty_fb flags, especially
> DRM_MODE_FB_DIRTY_ANNOTATE_COPY to be passed with atomic
> DAMAGE_CLIPS property blob? I didn't considered that untill now. If no driver
> uses that in my opinion for simplicity this can be ignored?

Last time I've checked no driver nor userspace really used this. I'd drop
it for the new uapi like you've done.

> About overlaping of damage rectangles is also not finalized. This really
> depends on driver specific implementation and can be left open-ended?

Overlapping is userspace being stupid imo :-) I guess we should say that
drivers should at least not fall over overlapping rects, and if they can't
handle them, either coalesce into one big rect, or split them on their own
somehow.

> My knowledge is limited to vmwgfx so would like to hear about other driver use
> cases and this can be modified in keeping other drivers need.
> 
> Going forward driver implementation for vmwgfx and user-space implementation
> of kmscube/weston will be next step to test the changes.

I think an implementation for -modesetting and weston would be perfect.
Kmscube has very littel value for damage tracking purposes (it's not a
full compositor, just renders new frame each scene). And for kms I'm not a
fan of using vendor-specific drivers to demonstrate new generic uapi - way
too big chances you're making some vendor driver specific hardcoded
assumption that doesn't hold anywhere else. Also makes it harder for
others to test-implement your uapi in their drivers.
-Daniel

> 
> Thanks,
> Deepak
> 
> Deepak Rawat (2):
>   drm: Add helper iterator functions to iterate over plane damage.
>   drm: Add helper to validate damage during modeset_check
> 
> Lukasz Spintzyk (1):
>   drm: Add DAMAGE_CLIPS property to plane
> 
>  drivers/gpu/drm/drm_atomic.c|  42 +
>  drivers/gpu/drm/drm_atomic_helper.c | 173 
> 
>  drivers/gpu/drm/drm_mode_config.c   |   5 ++
>  drivers/gpu/drm/drm_plane.c |  12 +++
>  include/drm/drm_atomic_helper.h |  41 +
>  include/drm/drm_mode_config.h   |  15 
>  include/drm/drm_plane.h |  16 
>  include/uapi/drm/drm_mode.h |  15 
>  8 files changed, 319 insertions(+)
> 
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm/msm/A6xx: Add support for using system cache(llc)

[Vivek Gautam]

Hi Sharat,


On 3/23/2018 12:49 PM, Sharat Masetty wrote:

The last level system cache can be partitioned to 32
different slices of which GPU has two slices preallocated.
The "gpu" slice is used for caching GPU buffers and
the "gpuhtw" slice is used for caching the GPU SMMU
pagetables.  This patch talks to the core system cache
driver to acquire the slice handles, configure the SCID's
to those slices and activates and deactivates the slices
upon GPU power collapse and restore.

Some support from the IOMMU driver is also needed to
make use of the system cache. IOMMU_UPSTREAM_HINT is
a buffer protection flag which enables caching GPU data
buffers in the system cache with memory attributes such
as outer cacheable, read-allocate, write-allocate for buffers.
The GPU then has the ability to override a few cacheability
parameters which it does to override write-allocate to
write-no-allocate as the GPU hardware does not benefit much
from it.
Similarly DOMAIN_ATTR_USE_UPSTREAM_HINT is another domain level
attribute used by the IOMMU driver to set the right attributes
to cache the hardware pagetables into the system cache.

Signed-off-by: Sharat Masetty 
---


Couple of minor nits. Please see comments inline below.


  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 162 +-
  drivers/gpu/drm/msm/adreno/a6xx_gpu.h |   9 ++
  drivers/gpu/drm/msm/msm_iommu.c   |  13 +++
  drivers/gpu/drm/msm/msm_mmu.h |   3 +
  4 files changed, 186 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index bd50674..e4554eb 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -13,6 +13,7 @@
  
  #include 

  #include 
+#include 
  
  #include "msm_gem.h"

  #include "msm_mmu.h"
@@ -913,6 +914,154 @@ static irqreturn_t a6xx_irq(struct msm_gpu *gpu)
~0
  };
  
+#define A6XX_LLC_NUM_GPU_SCIDS		5

+#define A6XX_GPU_LLC_SCID_NUM_BITS 5
+
+#define A6XX_GPU_LLC_SCID_MASK \
+   ((1 << (A6XX_LLC_NUM_GPU_SCIDS * A6XX_GPU_LLC_SCID_NUM_BITS)) - 1)
+
+#define A6XX_GPUHTW_LLC_SCID_SHIFT 25
+#define A6XX_GPUHTW_LLC_SCID_MASK \
+   (((1 << A6XX_GPU_LLC_SCID_NUM_BITS) - 1) << A6XX_GPUHTW_LLC_SCID_SHIFT)
+
+static inline void a6xx_gpu_cx_rmw(struct a6xx_llc *llc,
+   u32 reg, u32 mask, u32 or)
+{
+   msm_rmw(llc->mmio + (reg << 2), mask, or);
+}
+
+static void a6xx_llc_deactivate(struct msm_gpu *gpu)
+{
+   struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+   struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+   struct a6xx_llc *llc = _gpu->llc;
+
+   llcc_slice_deactivate(llc->gpu_llc_slice);
+   llcc_slice_deactivate(llc->gpuhtw_llc_slice);
+}
+
+static void a6xx_llc_activate(struct msm_gpu *gpu)
+{
+   struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+   struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+   struct a6xx_llc *llc = _gpu->llc;
+
+   if (!llc->mmio)
+   return;
+
+   if (llc->gpu_llc_slice)
+   if (!llcc_slice_activate(llc->gpu_llc_slice))
+   /* Program the sub-cache ID for all GPU blocks */
+   a6xx_gpu_cx_rmw(llc,
+   REG_A6XX_GPU_CX_MISC_SYSTEM_CACHE_CNTL_1,
+   A6XX_GPU_LLC_SCID_MASK,
+   (llc->cntl1_regval &
+   A6XX_GPU_LLC_SCID_MASK));
+
+   if (llc->gpuhtw_llc_slice)
+   if (!llcc_slice_activate(llc->gpuhtw_llc_slice))
+   /* Program the sub-cache ID for GPU pagetables */
+   a6xx_gpu_cx_rmw(llc,
+   REG_A6XX_GPU_CX_MISC_SYSTEM_CACHE_CNTL_1,
+   A6XX_GPUHTW_LLC_SCID_MASK,
+   (llc->cntl1_regval &
+   A6XX_GPUHTW_LLC_SCID_MASK));
+
+   /* Program cacheability overrides */
+   a6xx_gpu_cx_rmw(llc, REG_A6XX_GPU_CX_MISC_SYSTEM_CACHE_CNTL_0, 0xF,
+   llc->cntl0_regval);
+}
+
+void a6xx_llc_slices_destroy(struct a6xx_llc *llc)


static?


+{
+   if (llc->mmio) {
+   iounmap(llc->mmio);
+   llc->mmio = NULL;
+   }
+
+   llcc_slice_putd(llc->gpu_llc_slice);
+   llc->gpu_llc_slice = NULL;
+
+   llcc_slice_putd(llc->gpuhtw_llc_slice);
+   llc->gpuhtw_llc_slice = NULL;
+}
+
+static int a6xx_llc_slices_init(struct platform_device *pdev,
+   struct a6xx_llc *llc)
+{
+   int i;
+
+   /* Get the system cache slice descriptor for GPU and GPUHTWs */
+   llc->gpu_llc_slice = llcc_slice_getd(>dev, "gpu");
+   if (IS_ERR(llc->gpu_llc_slice))
+   llc->gpu_llc_slice = NULL;
+
+   llc->gpuhtw_llc_slice = llcc_slice_getd(>dev, "gpuhtw");
+   if (IS_ERR(llc->gpuhtw_llc_slice))
+   llc->gpuhtw_llc_slice = NULL;
+
+   

Re: [Intel-gfx] [PATCH] drm/i915: Do NOT skip the first 4k of stolen memory for pre-allocated buffers

[Daniel Vetter]

On Fri, Mar 30, 2018 at 02:27:15PM +0200, Hans de Goede wrote:
> Before this commit the WaSkipStolenMemoryFirstPage workaround code was
> skipping the first 4k by passing 4096 as start of the address range passed
> to drm_mm_init(). This means that calling drm_mm_reserve_node() to try and
> reserve the firmware framebuffer so that we can inherit it would always
> fail, as the firmware framebuffer starts at address 0.
> 
> Commit d43537610470 ("drm/i915: skip the first 4k of stolen memory on
> everything >= gen8") says in its commit message: "This is confirmed to fix
> Skylake screen flickering issues (probably caused by the fact that we
> initialized a ring in the first page of stolen, but I didn't 100% confirm
> this theory)."
> 
> Which suggests that it is safe to use the first page for a linear
> framebuffer as the firmware is doing.
> 
> This commit always passes 0 as start to drm_mm_init() and works around
> WaSkipStolenMemoryFirstPage in i915_gem_stolen_insert_node_in_range()
> by insuring the start address passed by to drm_mm_insert_node_in_range()
> is always 4k or more. All entry points to i915_gem_stolen.c go through
> i915_gem_stolen_insert_node_in_range(), so that any newly allocated
> objects such as ring-buffers will not be allocated in the first 4k.
> 
> The one exception is i915_gem_object_create_stolen_for_preallocated()
> which directly calls drm_mm_reserve_node() which now will be able to
> use the first 4k.
> 
> This fixes the i915 driver no longer being able to inherit the firmware
> framebuffer on gen8+, which fixes the video output changing from the
> vendor logo to a black screen as soon as the i915 driver is loaded
> (on systems without fbcon).
> 
> Signed-off-by: Hans de Goede 

I think this is worth a shot. The only explanation I can think of why the
GOP could get away with this and still follow the w/a is if it doesn't
have a 1:1 mapping between GGTT and stolen. Atm we hardcode that
assumption in intel_alloc_initial_plane_obj by passing the base_aligned as
both the stolen_offset and the gtt_offset (but it's only the gtt_offset
really). And since we're not re-writing the ptes it's not noticeable.

I think to decide whether this is the right approach we should
double-check whether that 1:1 assumption really holds true: Either read
back the ggtt ptes and check their addresses (but iirc on some platforms
their write-only, readback doesn't work), or we also rewrite the ptes
again for preallocated stuff, like when binding a normal object into the
gtt. If either of these approaches confirms that those affected gen8+
machines still use the 1:1 mapping, then I'm happy to put my r-b on this
patch. If not, well then we at least know what to fix: We need to read out
the real stolen_offset, instead of making assumptions.

Cheers, Daniel
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index af915d041281..ad949cc30928 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -51,6 +51,10 @@ int i915_gem_stolen_insert_node_in_range(struct 
> drm_i915_private *dev_priv,
>   if (!drm_mm_initialized(_priv->mm.stolen))
>   return -ENODEV;
>  
> + /* WaSkipStolenMemoryFirstPage:bdw+ */
> + if (INTEL_GEN(dev_priv) >= 8 && start < 4096)
> + start = 4096;
> +
>   mutex_lock(_priv->mm.stolen_lock);
>   ret = drm_mm_insert_node_in_range(_priv->mm.stolen, node,
> size, alignment, 0,
> @@ -343,7 +347,6 @@ int i915_gem_init_stolen(struct drm_i915_private 
> *dev_priv)
>  {
>   resource_size_t reserved_base, stolen_top;
>   resource_size_t reserved_total, reserved_size;
> - resource_size_t stolen_usable_start;
>  
>   mutex_init(_priv->mm.stolen_lock);
>  
> @@ -435,17 +438,11 @@ int i915_gem_init_stolen(struct drm_i915_private 
> *dev_priv)
>(u64)resource_size(_priv->dsm) >> 10,
>((u64)resource_size(_priv->dsm) - reserved_total) 
> >> 10);
>  
> - stolen_usable_start = 0;
> - /* WaSkipStolenMemoryFirstPage:bdw+ */
> - if (INTEL_GEN(dev_priv) >= 8)
> - stolen_usable_start = 4096;
> -
>   dev_priv->stolen_usable_size =
> - resource_size(_priv->dsm) - reserved_total - 
> stolen_usable_start;
> + resource_size(_priv->dsm) - reserved_total;
>  
>   /* Basic memrange allocator for stolen space. */
> - drm_mm_init(_priv->mm.stolen, stolen_usable_start,
> - dev_priv->stolen_usable_size);
> + drm_mm_init(_priv->mm.stolen, 0, dev_priv->stolen_usable_size);
>  
>   return 0;
>  }
> -- 
> 2.17.0.rc2
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> 

Re: [PATCH libdrm v2 14/23] meson: use simple option handling for omap

[Sebastian Reichel]

Hi,

On Wed, Apr 04, 2018 at 04:38:09PM +0100, Eric Engestrom wrote:
> [...]
>
> -with_omap = false
> -_omap = get_option('omap')
> -if _omap == 'true'
> -  if not with_atomics
> -error('libdrm_omap requires atomics.')
> -  endif
> -  with_omap = true
> +with_exynos = false
> +_exynos = get_option('exynos')
> +if _exynos == 'auto'
> +  with_exynos = true
> +else
> +  with_exynos = _exynos == 'true'
>  endif

Looks like some patch rebasing went wrong with this one (it
simplifies omap, but also adds some exynos stuff)?

-- Sebastian


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH] drm: rcar-du: track dma-buf fences

[Ucan, Emre (ADITG/ESB)]

Hello Laurent,

Thank you for your review.

Actually I sent this email yesterday. But  I don't see it in mailing list 
archives because of some reason.

> -Original Message-
> From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
> Sent: Dienstag, 3. April 2018 20:53
> To: Ucan, Emre (ADITG/ESB)
> Cc: dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH] drm: rcar-du: track dma-buf fences
> 
> Hello Emre,
> 
> Thank you for the patch.
> 
> On Tuesday, 3 April 2018 12:14:33 EEST Emre Ucan wrote:
> > We have to check dma-buf reservation objects
> > of our framebuffers before we use them.
> > Otherwise, another driver might be writing
> > on the same buffer which we are using.
> > This would cause visible tearing effects
> > on display.
> >
> > We can use existing atomic helper functions
> > to solve this problem.
> >
> > Signed-off-by: Emre Ucan 
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_du_kms.c |  2 ++
> >  drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 20 
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 0329b35..f3da3d1 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > @@ -255,6 +255,8 @@ static void rcar_du_atomic_commit_tail(struct
> > drm_atomic_state *old_state) {
> > struct drm_device *dev = old_state->dev;
> >
> > +   drm_atomic_helper_wait_for_fences(dev, old_state, false);
> > +
> 
> The commit_tail() function in drm_atomic_helper.c, which calls our
> atomic_commit_tail() implementation, already calls
> drm_atomic_helper_wait_for_fences(). Why is there a need to duplicate the
> call
> here ?

You are right. I will remove it in second version.

> 
> > /* Apply the atomic update. */
> > drm_atomic_helper_commit_modeset_disables(dev, old_state);
> > drm_atomic_helper_commit_planes(dev, old_state,
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 2c260c3..482e23c 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> > @@ -18,12 +18,16 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include 
> >
> > @@ -203,6 +207,20 @@ static void rcar_du_vsp_plane_setup(struct
> > rcar_du_vsp_plane *plane) plane->index, );
> >  }
> >
> > +static void rcar_du_vsp_set_fence_for_plane(struct drm_plane_state
> *state)
> > +{
> > +   struct drm_gem_cma_object *gem;
> > +   struct dma_buf *dma_buf;
> > +   struct dma_fence *fence;
> > +
> > +   gem = drm_fb_cma_get_gem_obj(state->fb, 0);
> > +   dma_buf = gem->base.dma_buf;
> > +   if (dma_buf) {
> > +   fence = reservation_object_get_excl_rcu(dma_buf->resv);
> > +   drm_atomic_set_fence_for_plane(state, fence);
> 
> Unless I'm mistaken this is used for implicit fencing only. What is your use
> case, wouldn't it be better for userspace to use explicit fencing as that is
> the fence model that has been selected for display ?

We are using Weston on Renesas R-Car H3 SoC. I am using GPU rendered client 
buffers
directly as scanout buffer for the display. Weston is not using explicit 
fencing.

> 
> > +   }
> > +}
> 
> This looks very similar to drm_gem_fb_prepare_fb(), couldn't you use that
> function instead ?

Description of drm_gem_fb_prepare_fb() function states that it can be
used as the _plane_helper_funcs.prepare_fb hook. But we have
our own hook function which is called rcar_du_vsp_plane_prepare_fb().
Therefore, I was not sure if it is correct to use drm_gem_fb_prepare_fb()
inside our hook function.

I will use it in the second version nevertheless.

> 
> >  static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
> > struct drm_plane_state *state)
> >  {
> > @@ -237,6 +255,8 @@ static int rcar_du_vsp_plane_prepare_fb(struct
> drm_plane
> > *plane, }
> > }
> >
> > +   rcar_du_vsp_set_fence_for_plane(state);
> > +
> > return 0;
> >
> >  fail:
> 
> --
> Regards,
> 
> Laurent Pinchart
> 
> 

Best Regards,

Emre Ucan

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

Re: [PATCH 2/5] arm64:dts:sdm845: Add support for GPU LLCC

[Vivek Gautam]

Hi Sharat,


On 3/23/2018 12:49 PM, Sharat Masetty wrote:

Add client side bindings required for the GPU to use the last level
system cache. Also add a register range in the GPU CX domain.

Signed-off-by: Sharat Masetty 
---
  arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index eb0a1b2..7e2d938 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -887,8 +887,8 @@
compatible = "qcom,adreno-630.2", "qcom,adreno";
#stream-id-cells = <16>;
  
-		reg = <0x500 0x4>;

-   reg-names = "kgsl_3d0_reg_memory";
+   reg = <0x500 0x4>, <0x509e000 0x10>;
+   reg-names = "kgsl_3d0_reg_memory", "cx_mem";
  
  		/*

 * Look ma, no clocks! The GPU clocks and power are controlled
@@ -898,6 +898,10 @@
interrupts = <0 300 0>;
interrupt-names = "kgsl_3d0_irq";
  
+		/* GPU related llc slices */

+   cache-slice-names = "gpu", "gpuhtw";
+   cache-slices = < 12>, < 11>;


Please add corresponding binding doc changes.

Best regards
Vivek


+
iommus = <_smmu 0>;
  
  		operating-points-v2 = <_opp_table>;


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

DRM_UDL and GPU under Xserver

[Alexey Brodkin]

Hello,

We're trying to use DisplayLink USB2-to-HDMI adapter to render GPU-accelerated 
graphics.
Hardware setup is as simple as a devboard + DisplayLink adapter.
Devboards we use for this experiment are:
 * Wandboard Quad (based on IMX6 SoC with Vivante GPU) or
 * HSDK (based on Synopsys ARC HS38 SoC with Vivante GPU as well)

I'm sure any other board with DRM supported GPU will work, those we just used
as the very recent Linux kernels could be easily run on them both.

Basically the problem is UDL needs to be explicitly notified about new data
to be rendered on the screen compared to typical bit-streamers that infinitely
scan a dedicated buffer in memory.

In case of UDL there're just 2 ways for this notification:
 1) DRM_IOCTL_MODE_PAGE_FLIP that calls drm_crtc_funcs->page_flip()
 2) DRM_IOCTL_MODE_DIRTYFB that calls drm_framebuffer_funcs->dirty()

But neither of IOCTLs happen when we run Xserver with xf86-video-armada driver
(see 
http://git.arm.linux.org.uk/cgit/xf86-video-armada.git/log/?h=unstable-devel).

Is it something missing in Xserver or in UDL driver?

Regards,
Alexey





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

Re: Signal handling in a page fault handler

[Matthew Wilcox]

On Wed, Apr 04, 2018 at 11:32:54AM +0200, Daniel Vetter wrote:
> So we've done some experiments for the case where the fault originated
> from kernel context (copy_to|from_user and friends). The fixup code seems
> to retry the copy once after the fault (in copy_user_handle_tail), if that
> fails again we get a short read/write. This might result in an -EFAULT,
> short read()/write() or anything else really, depending upon the syscall
> api.
> 
> Except in some code paths in gpu drivers where we convert anything into
> -ERESTARTSYS/EINTR if there's a signal pending it won't ever result in the
> syscall getting restarted (well except maybe short read/writes if
> userspace bothers with that).
> 
> So I guess gpu fault handlers indeed break the kernel's expectations, but
> then I think we're getting away with that because the inner workings of
> gpu memory objects is all heavily abstracted away by opengl/vulkan and
> friends.
> 
> I guess what we could do is try to only do killable sleeps if it's a
> kernel fault, but that means wiring a flag through all the callchains. Not
> pretty. Except when there's a magic set of functions that would convert
> all interruptible sleeps to killable ones only for us.

I actually have plans to allow mutex_lock_{interruptible,killable} to
return -EWOULDBLOCK if a flag is set.  So this doesn't seem entirely
unrelated.  Something like this perhaps:

 struct task_struct {
+   unsigned int sleep_state;
 };

 static noinline int __sched
-__mutex_lock_interruptible_slowpath(struct mutex *lock)
+__mutex_lock_slowpath(struct mutex *lock, long state)
 {
-   return __mutex_lock(lock, TASK_INTERRUPTIBLE, 0, NULL, _RET_IP_);
+   if (state == TASK_NOBLOCK)
+   return -EWOULDBLOCK;
+   return __mutex_lock(lock, state, 0, NULL, _RET_IP_);
 }

+int __sched mutex_lock_state(struct mutex *lock, long state)
+{
+   might_sleep();
+
+   if (__mutex_trylock_fast(lock))
+   return 0;
+
+   return __mutex_lock_slowpath(lock, state);
+}
+EXPORT_SYMBOL(mutex_lock_state);

Then the page fault handler can do something like:

old_state = current->sleep_state;
current->sleep_state = TASK_INTERRUPTIBLE;
...
current->sleep_state = old_state;


This has the page-fault-in-a-signal-handler problem.  I don't know if
there's a way to determine if we're already in a signal handler and use
a different sleep_state ...?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Signal handling in a page fault handler

[Matthew Wilcox]

On Wed, Apr 04, 2018 at 05:15:46PM +0200, Daniel Vetter wrote:
> On Wed, Apr 4, 2018 at 4:39 PM, Matthew Wilcox  wrote:
> > I actually have plans to allow mutex_lock_{interruptible,killable} to
> > return -EWOULDBLOCK if a flag is set.  So this doesn't seem entirely
> > unrelated.  Something like this perhaps:
> >
> >  struct task_struct {
> > +   unsigned int sleep_state;
> >  };
> >
> >  static noinline int __sched
> > -__mutex_lock_interruptible_slowpath(struct mutex *lock)
> > +__mutex_lock_slowpath(struct mutex *lock, long state)
> >  {
> > -   return __mutex_lock(lock, TASK_INTERRUPTIBLE, 0, NULL, _RET_IP_);
> > +   if (state == TASK_NOBLOCK)
> > +   return -EWOULDBLOCK;
> > +   return __mutex_lock(lock, state, 0, NULL, _RET_IP_);
> >  }
> >
> > +int __sched mutex_lock_state(struct mutex *lock, long state)
> > +{
> > +   might_sleep();
> > +
> > +   if (__mutex_trylock_fast(lock))
> > +   return 0;
> > +
> > +   return __mutex_lock_slowpath(lock, state);
> > +}
> > +EXPORT_SYMBOL(mutex_lock_state);
> >
> > Then the page fault handler can do something like:
> >
> > old_state = current->sleep_state;
> > current->sleep_state = TASK_INTERRUPTIBLE;
> > ...
> > current->sleep_state = old_state;
> >
> >
> > This has the page-fault-in-a-signal-handler problem.  I don't know if
> > there's a way to determine if we're already in a signal handler and use
> > a different sleep_state ...?
> 
> Not sure what problem you're trying to solve, but I don't think that's
> the one we have. The only way what we do goes wrong is if the fault
> originates from kernel context. For faults from the signal handler I
> think you just get to keep the pieces. Faults form kernel we can
> detect through FAULT_FLAG_USER.

Gah, I didn't explain well enough ;-(

From the get_user_pages (and similar) handlers, we'd do

 old_state = current->sleep_state;
 current->sleep_state = TASK_KILLABLE;
 ...
 current->sleep_state = old_state;

So you wouldn't need to discriminate on whether FAULT_FLAG_USER was set,
but could just use current->sleep_state.

> The issue I'm seeing is the following:
> 1. Some kernel code does copy_*_user, and it points at a gpu mmap region.
> 2. We fault and go into the gpu driver fault handler. That refuses to
> insert the pte because a signal is pending (because of all the
> interruptible waits and locks).
> 3. Fixup section runs, which afaict tries to do the copy once more
> using copy_user_handle_tail.
> 4. We fault again, because the pte is still not present.
> 5. GPU driver is still refusing to install the pte because signals are 
> pending.
> 6. Fixup section for copy_user_handle_tail just bails out.
> 7. copy_*_user returns and indicates that that not all bytes have been copied.
> 8. syscall (or whatever it is) bails out and returns to userspace,
> most likely with -EFAULT (but this ofc depends upon the syscall and
> what it should do when userspace access faults.
> 9. Signal finally gets handled, but the syscall already failed, and no
> one will restart it. If userspace is prudent, it might fail (or maybe
> hit an assert or something).

I think my patch above fixes this.  It makes the syscall killable rather
than interruptible, so it can never observe the short read / -EFAULT
return if it gets a fatal signal, and the non-fatal signal will be held
off until the syscall completes.

> Or maybe I'm confused by your diff, since nothing seems to use
> current->sleep_state. The problem is also that it's any sleep we do
> (they all tend to be interruptible, at least when waiting for the gpu
> or taking any locks that might be held while waiting for the gpu, or
> anything else that might be blocked waiting for the gpu really). So
> only patching mutex_lock won't fix this.

Sure, I was only patching mutex_lock_state in as an illustration.
I've also got a 'state' equivalent for wait_on_page_bit() (although
I'm not sure you care ...).

Looks like you'd need wait_for_completion_state() and
wait_event_state_timeout() as well.

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

[PATCH v2] drm: rcar-du: track dma-buf fences

[Emre Ucan]

We have to check dma-buf reservation objects
of our framebuffers before we use them.
Otherwise, another driver might be writing
on the same buffer which we are using.
This would cause visible tearing effects
on display.

We can use existing atomic helper functions
to solve this problem.

v2 changes:
- Remove drm_atomic_helper_wait_for_fences()
  call in rcar_du_kms.c. The commit_tail()
  function in drm_atomic_helper.c, which calls
  our atomic_commit_tail() implementation,
  already calls it.
- Remove proposed rcar_du_vsp_set_fence_for_plane()
  function. Call drm_gem_fb_prepare_fb(), which
  calls drm_atomic_set_fence_for_plane().

Signed-off-by: Emre Ucan 
---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c 
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 2c260c3..fbad616 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -237,7 +238,7 @@ static int rcar_du_vsp_plane_prepare_fb(struct drm_plane 
*plane,
}
}
 
-   return 0;
+   return drm_gem_fb_prepare_fb(plane, state);
 
 fail:
while (i--) {
-- 
2.7.4

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

Re: [PATCH 4/5] drm/msm: Pass mmu features to generic layers

[Vivek Gautam]

Hi Sharat,


On 3/23/2018 12:49 PM, Sharat Masetty wrote:

Allow different Adreno targets the ability to pass
specific mmu features to the generic layers. This will
help conditionally configure certain iommu features for
certain Adreno targets.

Also Add a few simple support functions to support a bitmask of
features that a specific MMU implementation supports.

Signed-off-by: Sharat Masetty 


Just a nit; please see my comment below.


---
  drivers/gpu/drm/msm/adreno/a3xx_gpu.c   |  2 +-
  drivers/gpu/drm/msm/adreno/a4xx_gpu.c   |  2 +-
  drivers/gpu/drm/msm/adreno/a5xx_gpu.c   |  2 +-
  drivers/gpu/drm/msm/adreno/a6xx_gpu.c   |  2 +-
  drivers/gpu/drm/msm/adreno/adreno_gpu.c |  4 +++-
  drivers/gpu/drm/msm/adreno/adreno_gpu.h |  2 +-
  drivers/gpu/drm/msm/msm_gpu.c   |  6 --
  drivers/gpu/drm/msm/msm_gpu.h   |  1 +
  drivers/gpu/drm/msm/msm_mmu.h   | 13 +
  9 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index 1dd84d3..a7a8573 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -492,7 +492,7 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev)
adreno_gpu->registers = a3xx_registers;
adreno_gpu->reg_offsets = a3xx_register_offsets;
  
-	ret = adreno_gpu_init(dev, pdev, adreno_gpu, , 1);

+   ret = adreno_gpu_init(dev, pdev, adreno_gpu, , 1, 0);
if (ret)
goto fail;
  
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c

index 2884b1b..5e7e15d6 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -574,7 +574,7 @@ struct msm_gpu *a4xx_gpu_init(struct drm_device *dev)
adreno_gpu->registers = a4xx_registers;
adreno_gpu->reg_offsets = a4xx_register_offsets;
  
-	ret = adreno_gpu_init(dev, pdev, adreno_gpu, , 1);

+   ret = adreno_gpu_init(dev, pdev, adreno_gpu, , 1, 0);
if (ret)
goto fail;
  
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c

index a4f68af..c9e06ff 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1295,7 +1295,7 @@ struct msm_gpu *a5xx_gpu_init(struct drm_device *dev)
  
  	check_speed_bin(>dev);
  
-	ret = adreno_gpu_init(dev, pdev, adreno_gpu, , 4);

+   ret = adreno_gpu_init(dev, pdev, adreno_gpu, , 4, 0);
if (ret) {
a5xx_destroy(&(a5xx_gpu->base.base));
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index e83b066..bd50674 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1040,7 +1040,7 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
adreno_gpu->registers = a6xx_registers;
adreno_gpu->reg_offsets = a6xx_register_offsets;
  
-	ret = adreno_gpu_init(dev, pdev, adreno_gpu, , 4);

+   ret = adreno_gpu_init(dev, pdev, adreno_gpu, , 4, 0);
if (ret) {
a6xx_destroy(&(a6xx_gpu->base.base));
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 6657461..a87ec6b 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -557,7 +557,8 @@ static int adreno_get_pwrlevels(struct device *dev,
  
  int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,

struct adreno_gpu *adreno_gpu,
-   const struct adreno_gpu_funcs *funcs, int nr_rings)
+   const struct adreno_gpu_funcs *funcs, int nr_rings,
+   unsigned long mmu_features)
  {
struct adreno_platform_config *config = pdev->dev.platform_data;
struct msm_gpu_config adreno_gpu_config  = { 0 };
@@ -576,6 +577,7 @@ int adreno_gpu_init(struct drm_device *drm, struct 
platform_device *pdev,
adreno_gpu_config.va_end = 0x;
  
  	adreno_gpu_config.nr_rings = nr_rings;

+   adreno_gpu_config.mmu_features = mmu_features;
  
  	adreno_get_pwrlevels(>dev, gpu);
  
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h

index bb9affd..19eda65 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -225,7 +225,7 @@ void adreno_submit(struct msm_gpu *gpu, struct 
msm_gem_submit *submit,
  
  int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,

struct adreno_gpu *gpu, const struct adreno_gpu_funcs *funcs,
-   int nr_rings);
+   int nr_rings, unsigned long mmu_features);
  void adreno_gpu_cleanup(struct adreno_gpu *gpu);
  int adreno_load_fw(struct adreno_gpu *adreno_gpu);
  
diff --git a/drivers/gpu/drm/msm/msm_gpu.c 

Re: [PATCH v2 0/5] allow override of bus format in bridges

[Peter Rosin]

On 2018-04-04 11:07, Laurent Pinchart wrote:
> Hi Daniel,
> 
> On Wednesday, 4 April 2018 09:34:41 EEST Daniel Vetter wrote:
>> On Wed, Apr 4, 2018 at 12:28 AM, Laurent Pinchart wrote:
>>> On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote:
 On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote:
> Hi!
>
> [I got to v2 sooner than expected]
>
> I have an Atmel sama5d31 hooked up to an lvds encoder and then
> on to an lvds panel. Which seems like something that has been
> done one or two times before...
>
> The problem is that the bus_format of the SoC and the panel do
> not agree. The SoC driver (atmel-hlcdc) can handle the
> rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is
> wired for the rgb565 case. The lvds encoder supports rgb888 on
> its input side with the LSB wires for each color simply pulled
> down internally in the encoder in my case which means that the
> rgb565 bus_format is the format that works best. And the panel
> is expecting lvds (vesa-24), which is what the encoder outputs.
>
> The reason I "blame" the bus_format of the drm_connector is that
> with the below DT snippet, things do not work *exactly* due to
> that. At least, it starts to work if I hack the panel-lvds driver
> to report the rgb565 bus_format instead of vesa-24.
>
> panel: panel {
> compatible = "panel-lvds";
> 
> width-mm = <304>;
> height-mm = <228;
> 
> data-mapping = "vesa-24";
> 
> panel-timing {
> // 1024x768 @ 60Hz (typical)
> clock-frequency = <5214 6500 7110>;
> hactive = <1024>;
> vactive = <768>;
> hfront-porch = <48 88 88>;
> hback-porch = <96 168 168>;
> hsync-len = <32 64 64>;
> vfront-porch = <8 13 14>;
> vback-porch = <8 13 14>;
> vsync-len = <8 12 14>;
> };
> 
> port {
> panel_input: endpoint {
> remote-endpoint = <_encoder_output>;
> };
> };
> };
> 
> lvds-encoder {
> compatible = "ti,ds90c185", "lvds-encoder";
> 
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
> 
> port@0 {
> reg = <0>;
> 
> lvds_encoder_input: endpoint {
> remote-endpoint = <_output>;
> };
> };
> 
> port@1 {
> reg = <1>;
> 
> lvds_encoder_output: endpoint {
> remote-endpoint = <_input>;
> };
> };
> };
> };
>
> But, instead of perverting the panel-lvds driver with support
> for a totally fake non-lvds bus_format, I intruduce an API that allows
> display controller drivers to query the required bus_format of any
> intermediate bridges, and match up with that instead of the formats
> given by the drm_connector. I trigger this with this addition to the
>
> lvds-encoder DT node:
> interface-pix-fmt = "rgb565";
>
> Naming is hard though, so I'm not sure if that's good?
>
> I threw in the first patch, since that is the actual lvds encoder
> I have in this case.
>
> Suggestions welcome.

 Took a quick look, feels rather un-atomic. And there's beend discussing
 for other bridge related state that we might want to track (like the full
 adjusted_mode that might need to be adjusted at each stage in the chain).
 So here's my suggestions:

 - Add an optional per-bridge internal state struct using the support in
   https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-driver-> 
 >>   private-state

   Yes it says "driver private", but since bridge is just helper stuff
   that's all included. "driver private" == "not exposed as uapi" here.
   Include all the usual convenience wrappers to get at the state for a
   bridge.

 - Then stuff your bus_format into that new drm_bridge_state struct.

 - Add a new bridge callback atomic_check, which gets that bridge state as
   parameter (similar to all the other atomic_check functions).

Re: [PATCH v2 2/5] drm: bridge: add API to query the expected input formats of bridges

[Peter Rosin]

On 2018-04-04 15:07, Laurent Pinchart wrote:
> First of all, thank you for the patch. This generates more discussion than I 
> had anticipated, which is both good and bad. I'll comment through the e-mail, 
> and try to explain both my initial idea, and also where it could lead us.

*snip*

Thank you for the long interesting mail! I will read it again, but my
immediate reaction is that I am not the man to attempt anything heavier
than static format information in drm bridges, and that I will hold off
any further work until some consensus has been reached.

> Finally, still regarding Peter's case, the decision to output RGB565 instead 
> of RGB888 (which the LVDS encoder expects) is due to PCB routing between the 
> display controller and the LVDS encoder. This isn't a property of the LVDS 
> encoder or the display controller, but of their hardware connection. This 
> patch series uses a DT property in the LVDS encoder DT node to convey that 
> information, but wouldn't it be equally correct (or incorrect :-)) to instead 
> use a DT property in the display controller DT node ?

Right, it might even be more correct to do it there? Thinking about it, I
have this in the DT

#include "sama5d3_lcd.dtsi"

 {
hlcdc-display-controller {
pinctrl-names = "default";
pinctrl-0 = <_lcd_base _lcd_rgb565>;

port@0 {
hlcdc_output: endpoint {
remote-endpoint = <_encoder_input>;
};
};
};
};

Where the _lcd_rgb565 handle tells the chip to not even bother
routing all of the rgb888 signals to pins and thus not waste pins that
are in fact used for other things. Maybe it is possible for the hlcdc
driver to look at that info and suggest rgb565 instead of rgb888 without
anything further in the DT? But maybe it's difficult to peek into pinctrl
bindings like that?

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

Re: [PATCH] drm/vc4: Fix memory leak during BO teardown

[Sasha Levin]

Hi Daniel J Blueman.

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has also determined it's probably a bug fixing patch. (score: 85.0720)

The bot has tested the following trees: v4.15.15, v4.14.32, v4.9.92, v4.4.126, 

v4.15.15: Build OK!
v4.14.32: Build OK!
v4.9.92: Build OK!
v4.4.126: Failed to apply! Possible dependencies:
463873d57014: ("drm/vc4: Add an API for creating GPU shaders in GEM BOs.")
c826a6e10644: ("drm/vc4: Add a BO cache.")
d5bc60f6ad05: ("drm/vc4: Add create and map BO ioctls.")
c826a6e10644: ("drm/vc4: Add a BO cache.")


Please let us know if you'd like to have this patch included in a stable tree.

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

RE: [PATCH] drm: rcar-du: track dma-buf fences

[Ucan, Emre (ADITG/ESB)]

Hello Laurent

Thank you for your review.

> -Original Message-
> From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
> Sent: Dienstag, 3. April 2018 20:53
> To: Ucan, Emre (ADITG/ESB)
> Cc: dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH] drm: rcar-du: track dma-buf fences
> 
> Hello Emre,
> 
> Thank you for the patch.
> 
> On Tuesday, 3 April 2018 12:14:33 EEST Emre Ucan wrote:
> > We have to check dma-buf reservation objects
> > of our framebuffers before we use them.
> > Otherwise, another driver might be writing
> > on the same buffer which we are using.
> > This would cause visible tearing effects
> > on display.
> >
> > We can use existing atomic helper functions
> > to solve this problem.
> >
> > Signed-off-by: Emre Ucan 
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_du_kms.c |  2 ++
> >  drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 20 
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 0329b35..f3da3d1 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > @@ -255,6 +255,8 @@ static void rcar_du_atomic_commit_tail(struct
> > drm_atomic_state *old_state) {
> > struct drm_device *dev = old_state->dev;
> >
> > +   drm_atomic_helper_wait_for_fences(dev, old_state, false);
> > +
> 
> The commit_tail() function in drm_atomic_helper.c, which calls our
> atomic_commit_tail() implementation, already calls
> drm_atomic_helper_wait_for_fences(). Why is there a need to duplicate the
> call
> here ?

You are right. I will remove it in second version.

> 
> > /* Apply the atomic update. */
> > drm_atomic_helper_commit_modeset_disables(dev, old_state);
> > drm_atomic_helper_commit_planes(dev, old_state,
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 2c260c3..482e23c 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> > @@ -18,12 +18,16 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include 
> >
> > @@ -203,6 +207,20 @@ static void rcar_du_vsp_plane_setup(struct
> > rcar_du_vsp_plane *plane) plane->index, );
> >  }
> >
> > +static void rcar_du_vsp_set_fence_for_plane(struct drm_plane_state
> *state)
> > +{
> > +   struct drm_gem_cma_object *gem;
> > +   struct dma_buf *dma_buf;
> > +   struct dma_fence *fence;
> > +
> > +   gem = drm_fb_cma_get_gem_obj(state->fb, 0);
> > +   dma_buf = gem->base.dma_buf;
> > +   if (dma_buf) {
> > +   fence = reservation_object_get_excl_rcu(dma_buf->resv);
> > +   drm_atomic_set_fence_for_plane(state, fence);
> 
> Unless I'm mistaken this is used for implicit fencing only. What is your use
> case, wouldn't it be better for userspace to use explicit fencing as that is
> the fence model that has been selected for display ?

We are using Weston on Renesas R-Car H3 SoC. I am using GPU rendered client 
buffers
directly as scanout buffer for the display. Weston is not using explicit 
fencing.

> 
> > +   }
> > +}
> 
> This looks very similar to drm_gem_fb_prepare_fb(), couldn't you use that
> function instead ?

Description of drm_gem_fb_prepare_fb() function states that it can be
used as the _plane_helper_funcs.prepare_fb hook. But we have
our own hook function which is called rcar_du_vsp_plane_prepare_fb().
Therefore, I was not sure if it is correct to use drm_gem_fb_prepare_fb()
inside our hook function.

I will use it in the second version nevertheless.

> 
> >  static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
> > struct drm_plane_state *state)
> >  {
> > @@ -237,6 +255,8 @@ static int rcar_du_vsp_plane_prepare_fb(struct
> drm_plane
> > *plane, }
> > }
> >
> > +   rcar_du_vsp_set_fence_for_plane(state);
> > +
> > return 0;
> >
> >  fail:
> 
> --
> Regards,
> 
> Laurent Pinchart
> 
> 

Best Regards,

Emre Ucan

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

Fwd: DRM MIPI DSI device and I2C device?

[Carsten Behling]

> Hi,
>
> I would like to write a DRM bridge driver that is an I2C device and a DRM
MIPI DSI device.
>
> It looks like that both - 'i2c-core.c: of_i2c_register_devices' and
'drm_mipi_dsi.c: mipi_dsi_host_register'  are registering their devices by
iterating over devicetree child nodes with for_each_available_child_of_node.
>
>Since I can't make the bridge a child node of both, I don't know how to
resolve it.
>
Found the answer myself. adv7533 driver is a good example. Devicetree
exists for qcom apq8016-sbc. No need to make the bridge a MIPI DSI child
node.

> Best regards
>-Carsten
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/1] drm/i915: Do not use kfree() to free kmem_cache_alloc() return value

[Xidong Wang]

In eb_lookup_vmas(), the return value of kmem_cache_alloc() is freed
with kfree(). I think the expected paired function is kmem_cahce_free().

Signed-off-by: Xidong Wang 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 8c170db..0414228 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -728,7 +728,7 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
 
err = radix_tree_insert(handles_vma, handle, vma);
if (unlikely(err)) {
-   kfree(lut);
+   kmem_cache_free(eb->i915->luts, lut);
goto err_obj;
}
 
-- 
2.7.4


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

[PATCH] uapi: fix linux/kfd_ioctl.h userspace compilation errors

[Dmitry V. Levin]

Consistently use types provided by  via 
to fix the following linux/kfd_ioctl.h userspace compilation errors:

/usr/include/linux/kfd_ioctl.h:266:2: error: unknown type name 'uint64_t'
  uint64_t tba_addr;  /* to KFD */
/usr/include/linux/kfd_ioctl.h:267:2: error: unknown type name 'uint64_t'
  uint64_t tma_addr;  /* to KFD */
/usr/include/linux/kfd_ioctl.h:268:2: error: unknown type name 'uint32_t'
  uint32_t gpu_id;  /* to KFD */
/usr/include/linux/kfd_ioctl.h:269:2: error: unknown type name 'uint32_t'
  uint32_t pad;

Fixes: d7b9bd2248d79 ("drm/amdkfd: Add support for user-mode trap handlers")
Cc:  # v4.16
Signed-off-by: Dmitry V. Levin 
---
 include/uapi/linux/kfd_ioctl.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index f4cab5b3ba9a..111d73ba2d96 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -263,10 +263,10 @@ struct kfd_ioctl_get_tile_config_args {
 };
 
 struct kfd_ioctl_set_trap_handler_args {
-   uint64_t tba_addr;  /* to KFD */
-   uint64_t tma_addr;  /* to KFD */
-   uint32_t gpu_id;/* to KFD */
-   uint32_t pad;
+   __u64 tba_addr; /* to KFD */
+   __u64 tma_addr; /* to KFD */
+   __u32 gpu_id;   /* to KFD */
+   __u32 pad;
 };
 
 #define AMDKFD_IOCTL_BASE 'K'
-- 
ldv
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/omap: fix crash if there's no video PLL

[Tomi Valkeinen]

Commit 8a7eda7686675b73d74c22c0d5b83059f9d783f6 ("drm: omapdrm: dispc:
Pass DISPC pointer to remaining dispc API functions") made dpi.c use
ctx->pll even when there's no PLL, causing a crash at modeset on AM4
EVM, and presumably all OMAP2/3 boards.

Fix this by having struct dpi_data pointer in the ctx instead, giving
access to dispc without going through the pll.

Signed-off-by: Tomi Valkeinen 
Reported-by: Keerthy 
Cc: Laurent Pinchart 
---
 drivers/gpu/drm/omapdrm/dss/dpi.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c 
b/drivers/gpu/drm/omapdrm/dss/dpi.c
index fb1c27f69e3a..3d662e6805eb 100644
--- a/drivers/gpu/drm/omapdrm/dss/dpi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
@@ -142,7 +142,7 @@ static enum dss_clk_source dpi_get_clk_src(struct dpi_data 
*dpi)
 }
 
 struct dpi_clk_calc_ctx {
-   struct dss_pll *pll;
+   struct dpi_data *dpi;
unsigned int clkout_idx;
 
/* inputs */
@@ -191,7 +191,7 @@ static bool dpi_calc_hsdiv_cb(int m_dispc, unsigned long 
dispc,
ctx->pll_cinfo.mX[ctx->clkout_idx] = m_dispc;
ctx->pll_cinfo.clkout[ctx->clkout_idx] = dispc;
 
-   return dispc_div_calc(ctx->pll->dss->dispc, dispc,
+   return dispc_div_calc(ctx->dpi->dss->dispc, dispc,
  ctx->pck_min, ctx->pck_max,
  dpi_calc_dispc_cb, ctx);
 }
@@ -208,8 +208,8 @@ static bool dpi_calc_pll_cb(int n, int m, unsigned long 
fint,
ctx->pll_cinfo.fint = fint;
ctx->pll_cinfo.clkdco = clkdco;
 
-   return dss_pll_hsdiv_calc_a(ctx->pll, clkdco,
-   ctx->pck_min, dss_get_max_fck_rate(ctx->pll->dss),
+   return dss_pll_hsdiv_calc_a(ctx->dpi->pll, clkdco,
+   ctx->pck_min, dss_get_max_fck_rate(ctx->dpi->dss),
dpi_calc_hsdiv_cb, ctx);
 }
 
@@ -219,7 +219,7 @@ static bool dpi_calc_dss_cb(unsigned long fck, void *data)
 
ctx->fck = fck;
 
-   return dispc_div_calc(ctx->pll->dss->dispc, fck,
+   return dispc_div_calc(ctx->dpi->dss->dispc, fck,
  ctx->pck_min, ctx->pck_max,
  dpi_calc_dispc_cb, ctx);
 }
@@ -230,7 +230,7 @@ static bool dpi_pll_clk_calc(struct dpi_data *dpi, unsigned 
long pck,
unsigned long clkin;
 
memset(ctx, 0, sizeof(*ctx));
-   ctx->pll = dpi->pll;
+   ctx->dpi = dpi;
ctx->clkout_idx = dss_pll_get_clkout_idx_for_src(dpi->clk_src);
 
clkin = clk_get_rate(dpi->pll->clkin);
@@ -244,7 +244,7 @@ static bool dpi_pll_clk_calc(struct dpi_data *dpi, unsigned 
long pck,
pll_min = 0;
pll_max = 0;
 
-   return dss_pll_calc_a(ctx->pll, clkin,
+   return dss_pll_calc_a(ctx->dpi->pll, clkin,
pll_min, pll_max,
dpi_calc_pll_cb, ctx);
} else { /* DSS_PLL_TYPE_B */
@@ -275,6 +275,7 @@ static bool dpi_dss_clk_calc(struct dpi_data *dpi, unsigned 
long pck,
bool ok;
 
memset(ctx, 0, sizeof(*ctx));
+   ctx->dpi = dpi;
if (pck > 1000 * i * i * i)
ctx->pck_min = max(pck - 1000 * i * i * i, 0lu);
else
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

Re: ww-mutex performance tests?

[Christian König]

Am 04.04.2018 um 20:02 schrieb Daniel Vetter:

On Tue, Apr 3, 2018 at 10:37 AM, Maarten Lankhorst
 wrote:

Op 02-04-18 om 19:49 schreef Thomas Hellstrom:

Maarten, Daniel,

Do we have any ww-mutex performance tests somewhere that can be used to test 
the impact of implementation details on various locking scenarios?

Thanks,

/Thomas



The thing that comes to my mind are some of the kms_cursor_legacy tests that 
have been proven to be sensitive to locking issues before.
All subtests with (pipe-*/all-pipes)-(single/forked/torture)-(bo/move) try to 
do cursor updates as fast as possible, and report how many updates have been 
done.

AMD folks have a bunch of tests to exercise their CS paths I think,
that should be interesting for the multi-ww_mutex/backoff paths,
adding Christian et al.


Well not a dedicated test for this, but at least I usually run glmark 
with thread offloading disabled to measure the command submission overhead.


Except for the usual stuff like copying things from userspace the last 
time I looked our command submission overhead was dominated by work done 
for individual BOs.


Not sure how much of that accounts for locking the ww_mutex of the BOs, 
but I would guess it is quite a bit.


Christian.


-Daniel


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

Re: DRM_UDL and GPU under Xserver

[Daniel Vetter]

On Wed, Apr 4, 2018 at 10:06 PM, Alexey Brodkin
 wrote:
> Hello,
>
> We're trying to use DisplayLink USB2-to-HDMI adapter to render 
> GPU-accelerated graphics.
> Hardware setup is as simple as a devboard + DisplayLink adapter.
> Devboards we use for this experiment are:
>  * Wandboard Quad (based on IMX6 SoC with Vivante GPU) or
>  * HSDK (based on Synopsys ARC HS38 SoC with Vivante GPU as well)
>
> I'm sure any other board with DRM supported GPU will work, those we just used
> as the very recent Linux kernels could be easily run on them both.
>
> Basically the problem is UDL needs to be explicitly notified about new data
> to be rendered on the screen compared to typical bit-streamers that infinitely
> scan a dedicated buffer in memory.
>
> In case of UDL there're just 2 ways for this notification:
>  1) DRM_IOCTL_MODE_PAGE_FLIP that calls drm_crtc_funcs->page_flip()
>  2) DRM_IOCTL_MODE_DIRTYFB that calls drm_framebuffer_funcs->dirty()
>
> But neither of IOCTLs happen when we run Xserver with xf86-video-armada driver
> (see 
> http://git.arm.linux.org.uk/cgit/xf86-video-armada.git/log/?h=unstable-devel).
>
> Is it something missing in Xserver or in UDL driver?

Use the -modesetting driverr for UDL, that one works correctly.
Kernel-driver specific X drivers are kinda deprecated, and stuff like
this (and other bugfixes and improvements that don't propagate around)
are the reason for that.
-Daniel

>
> Regards,
> Alexey
>
>
>
>
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/sched: Extend the documentation.

[Daniel Vetter]

On Thu, Apr 5, 2018 at 12:32 AM, Eric Anholt  wrote:
> These comments answer all the questions I had for myself when
> implementing a driver using the GPU scheduler.
>
> Signed-off-by: Eric Anholt 

Pulling all these comments into the generated kerneldoc would be
awesome, maybe as a new "GPU Scheduler" chapter at the end of
drm-mm.rst? Would mean a bit of busywork to convert the existing raw
comments into proper kerneldoc. Also has the benefit that 0day will
complain when you forget to update the comment when editing the
function prototype - kerneldoc which isn't included anywhere in .rst
won't be checked automatically.
-Daniel

> ---
>  include/drm/gpu_scheduler.h | 46 +
>  1 file changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index dfd54fb94e10..c053a32341bf 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -43,10 +43,12 @@ enum drm_sched_priority {
>  };
>
>  /**
> - * A scheduler entity is a wrapper around a job queue or a group
> - * of other entities. Entities take turns emitting jobs from their
> - * job queues to corresponding hardware ring based on scheduling
> - * policy.
> + * drm_sched_entity - A wrapper around a job queue (typically attached
> + * to the DRM file_priv).
> + *
> + * Entities will emit jobs in order to their corresponding hardware
> + * ring, and the scheduler will alternate between entities based on
> + * scheduling policy.
>  */
>  struct drm_sched_entity {
> struct list_headlist;
> @@ -78,7 +80,18 @@ struct drm_sched_rq {
>
>  struct drm_sched_fence {
> struct dma_fencescheduled;
> +
> +   /* This fence is what will be signaled by the scheduler when
> +* the job is completed.
> +*
> +* When setting up an out fence for the job, you should use
> +* this, since it's available immediately upon
> +* drm_sched_job_init(), and the fence returned by the driver
> +* from run_job() won't be created until the dependencies have
> +* resolved.
> +*/
> struct dma_fencefinished;
> +
> struct dma_fence_cb cb;
> struct dma_fence*parent;
> struct drm_gpu_scheduler*sched;
> @@ -88,6 +101,13 @@ struct drm_sched_fence {
>
>  struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
>
> +/**
> + * drm_sched_job - A job to be run by an entity.
> + *
> + * A job is created by the driver using drm_sched_job_init(), and
> + * should call drm_sched_entity_push_job() once it wants the scheduler
> + * to schedule the job.
> + */
>  struct drm_sched_job {
> struct spsc_nodequeue_node;
> struct drm_gpu_scheduler*sched;
> @@ -112,10 +132,28 @@ static inline bool drm_sched_invalidate_job(struct 
> drm_sched_job *s_job,
>   * these functions should be implemented in driver side
>  */
>  struct drm_sched_backend_ops {
> +   /* Called when the scheduler is considering scheduling this
> +* job next, to get another struct dma_fence for this job to
> +* block on.  Once it returns NULL, run_job() may be called.
> +*/
> struct dma_fence *(*dependency)(struct drm_sched_job *sched_job,
> struct drm_sched_entity *s_entity);
> +
> +   /* Called to execute the job once all of the dependencies have
> +* been resolved.  This may be called multiple times, if
> +* timedout_job() has happened and drm_sched_job_recovery()
> +* decides to try it again.
> +*/
> struct dma_fence *(*run_job)(struct drm_sched_job *sched_job);
> +
> +   /* Called when a job has taken too long to execute, to trigger
> +* GPU recovery.
> +*/
> void (*timedout_job)(struct drm_sched_job *sched_job);
> +
> +   /* Called once the job's finished fence has been signaled and
> +* it's time to clean it up.
> +*/
> void (*free_job)(struct drm_sched_job *sched_job);
>  };
>
> --
> 2.17.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel