[PATCH 0/3] drm/qxl: video mode tweaks
Gerd Hoffmann (3): drm/qxl: add mode/framebuffer check functions drm/qxl: add qxl_add_mode helper function drm/qxl: use kernel mode db drivers/gpu/drm/qxl/qxl_display.c | 138 +++--- 1 file changed, 70 insertions(+), 68 deletions(-) -- 2.9.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/qxl: use kernel mode db
Add all standard modes from the kernel's video mode data base.
Keep a few non-standard modes in the qxl mode list.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 27 +++
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index b87f72f59e..9e49472872 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -256,34 +256,20 @@ static int qxl_add_monitors_config_modes(struct
drm_connector *connector)
static struct mode_size {
int w;
int h;
-} common_modes[] = {
- { 640, 480},
+} extra_modes[] = {
{ 720, 480},
- { 800, 600},
- { 848, 480},
- {1024, 768},
{1152, 768},
- {1280, 720},
- {1280, 800},
{1280, 854},
- {1280, 960},
- {1280, 1024},
- {1440, 900},
- {1400, 1050},
- {1680, 1050},
- {1600, 1200},
- {1920, 1080},
- {1920, 1200}
};
-static int qxl_add_common_modes(struct drm_connector *connector)
+static int qxl_add_extra_modes(struct drm_connector *connector)
{
int i, ret = 0;
- for (i = 0; i < ARRAY_SIZE(common_modes); i++)
+ for (i = 0; i < ARRAY_SIZE(extra_modes); i++)
ret += qxl_add_mode(connector,
- common_modes[i].w,
- common_modes[i].h,
+ extra_modes[i].w,
+ extra_modes[i].h,
false);
return ret;
}
@@ -954,7 +940,8 @@ static int qxl_conn_get_modes(struct drm_connector
*connector)
pheight = head->height;
}
- ret += qxl_add_common_modes(connector);
+ ret += drm_add_modes_noedid(connector, 8192, 8192);
+ ret += qxl_add_extra_modes(connector);
ret += qxl_add_monitors_config_modes(connector);
drm_set_preferred_mode(connector, pwidth, pheight);
return ret;
--
2.9.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/qxl: add qxl_add_mode helper function
Add a helper function to add custom video modes to a connector.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 84 +++
1 file changed, 49 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 07a37d52c4..b87f72f59e 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -206,15 +206,36 @@ static int qxl_check_framebuffer(struct qxl_device *qdev,
return qxl_check_mode(qdev, bo->surf.width, bo->surf.height);
}
-static int qxl_add_monitors_config_modes(struct drm_connector *connector,
- unsigned *pwidth,
- unsigned *pheight)
+static int qxl_add_mode(struct drm_connector *connector,
+ unsigned int width,
+ unsigned int height,
+ bool preferred)
+{
+ struct drm_device *dev = connector->dev;
+ struct qxl_device *qdev = dev->dev_private;
+ struct drm_display_mode *mode = NULL;
+ int rc;
+
+ rc = qxl_check_mode(qdev, width, height);
+ if (rc != 0)
+ return 0;
+
+ mode = drm_cvt_mode(dev, width, height, 60, false, false, false);
+ if (preferred)
+ mode->type |= DRM_MODE_TYPE_PREFERRED;
+ mode->hdisplay = width;
+ mode->vdisplay = height;
+ drm_mode_set_name(mode);
+ drm_mode_probed_add(connector, mode);
+ return 1;
+}
+
+static int qxl_add_monitors_config_modes(struct drm_connector *connector)
{
struct drm_device *dev = connector->dev;
struct qxl_device *qdev = dev->dev_private;
struct qxl_output *output = drm_connector_to_qxl_output(connector);
int h = output->index;
- struct drm_display_mode *mode = NULL;
struct qxl_head *head;
if (!qdev->monitors_config)
@@ -229,19 +250,7 @@ static int qxl_add_monitors_config_modes(struct
drm_connector *connector,
head = &qdev->client_monitors_config->heads[h];
DRM_DEBUG_KMS("head %d is %dx%d\n", h, head->width, head->height);
- mode = drm_cvt_mode(dev, head->width, head->height, 60, false, false,
- false);
- mode->type |= DRM_MODE_TYPE_PREFERRED;
- mode->hdisplay = head->width;
- mode->vdisplay = head->height;
- drm_mode_set_name(mode);
- *pwidth = head->width;
- *pheight = head->height;
- drm_mode_probed_add(connector, mode);
- /* remember the last custom size for mode validation */
- qdev->monitors_config_width = mode->hdisplay;
- qdev->monitors_config_height = mode->vdisplay;
- return 1;
+ return qxl_add_mode(connector, head->width, head->height, true);
}
static struct mode_size {
@@ -267,22 +276,16 @@ static struct mode_size {
{1920, 1200}
};
-static int qxl_add_common_modes(struct drm_connector *connector,
-unsigned int pwidth,
-unsigned int pheight)
+static int qxl_add_common_modes(struct drm_connector *connector)
{
- struct drm_device *dev = connector->dev;
- struct drm_display_mode *mode = NULL;
- int i;
+ int i, ret = 0;
- for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
- mode = drm_cvt_mode(dev, common_modes[i].w, common_modes[i].h,
- 60, false, false, false);
- if (common_modes[i].w == pwidth && common_modes[i].h == pheight)
- mode->type |= DRM_MODE_TYPE_PREFERRED;
- drm_mode_probed_add(connector, mode);
- }
- return i - 1;
+ for (i = 0; i < ARRAY_SIZE(common_modes); i++)
+ ret += qxl_add_mode(connector,
+ common_modes[i].w,
+ common_modes[i].h,
+ false);
+ return ret;
}
static void qxl_send_monitors_config(struct qxl_device *qdev)
@@ -935,14 +938,25 @@ static int qdev_crtc_init(struct drm_device *dev, int
crtc_id)
static int qxl_conn_get_modes(struct drm_connector *connector)
{
+ struct drm_device *dev = connector->dev;
+ struct qxl_device *qdev = dev->dev_private;
+ struct qxl_output *output = drm_connector_to_qxl_output(connector);
unsigned int pwidth = 1024;
unsigned int pheight = 768;
int ret = 0;
- ret = qxl_add_monitors_config_modes(connector, &pwidth, &pheight);
- if (ret < 0)
- return ret;
- ret += qxl_add_common_modes(connector, pwidth, pheight);
+ if (qdev->client_monitors_config) {
+ struct qxl_head *head;
+ head = &qdev->client_monitors_config->heads[output->index];
+ if (head->width)
+ pwidth = head->width;
+ if (head->height)
+
[PATCH 1/3] drm/qxl: add mode/framebuffer check functions
Add a helper functions to check video modes. Also add a helper to check
framebuffer buffer objects, using the former for consistency. That way
we should not fail in qxl_primary_atomic_check() because video modes
which are too big will not be added to the mode list in the first place.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 1f8fddcc34..07a37d52c4 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -191,6 +191,21 @@ void qxl_display_read_client_monitors_config(struct
qxl_device *qdev)
}
}
+static int qxl_check_mode(struct qxl_device *qdev,
+ unsigned int width,
+ unsigned int height)
+{
+ if (width * height * 4 > qdev->vram_size)
+ return -ENOMEM;
+ return 0;
+}
+
+static int qxl_check_framebuffer(struct qxl_device *qdev,
+struct qxl_bo *bo)
+{
+ return qxl_check_mode(qdev, bo->surf.width, bo->surf.height);
+}
+
static int qxl_add_monitors_config_modes(struct drm_connector *connector,
unsigned *pwidth,
unsigned *pheight)
@@ -466,12 +481,7 @@ static int qxl_primary_atomic_check(struct drm_plane
*plane,
bo = gem_to_qxl_bo(state->fb->obj[0]);
- if (bo->surf.stride * bo->surf.height > qdev->vram_size) {
- DRM_ERROR("Mode doesn't fit in vram size (vgamem)");
- return -EINVAL;
- }
-
- return 0;
+ return qxl_check_framebuffer(qdev, bo);
}
static int qxl_primary_apply_cursor(struct drm_plane *plane)
@@ -941,20 +951,11 @@ static enum drm_mode_status qxl_conn_mode_valid(struct
drm_connector *connector,
{
struct drm_device *ddev = connector->dev;
struct qxl_device *qdev = ddev->dev_private;
- int i;
- /* TODO: is this called for user defined modes? (xrandr --add-mode)
-* TODO: check that the mode fits in the framebuffer */
+ if (qxl_check_mode(qdev, mode->hdisplay, mode->vdisplay) != 0)
+ return MODE_BAD;
- if (qdev->monitors_config_width == mode->hdisplay &&
- qdev->monitors_config_height == mode->vdisplay)
- return MODE_OK;
-
- for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
- if (common_modes[i].w == mode->hdisplay && common_modes[i].h ==
mode->vdisplay)
- return MODE_OK;
- }
- return MODE_BAD;
+ return MODE_OK;
}
static struct drm_encoder *qxl_best_encoder(struct drm_connector *connector)
--
2.9.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On Tue, 15 Jan 2019, Andrew F. Davis wrote:
> On 1/14/19 11:13 AM, Liam Mark wrote:
> > On Fri, 11 Jan 2019, Andrew F. Davis wrote:
> >
> >> Buffers may not be mapped from the CPU so skip cache maintenance here.
> >> Accesses from the CPU to a cached heap should be bracketed with
> >> {begin,end}_cpu_access calls so maintenance should not be needed anyway.
> >>
> >> Signed-off-by: Andrew F. Davis
> >> ---
> >> drivers/staging/android/ion/ion.c | 7 ---
> >> 1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/staging/android/ion/ion.c
> >> b/drivers/staging/android/ion/ion.c
> >> index 14e48f6eb734..09cb5a8e2b09 100644
> >> --- a/drivers/staging/android/ion/ion.c
> >> +++ b/drivers/staging/android/ion/ion.c
> >> @@ -261,8 +261,8 @@ static struct sg_table *ion_map_dma_buf(struct
> >> dma_buf_attachment *attachment,
> >>
> >>table = a->table;
> >>
> >> - if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
> >> - direction))
> >> + if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,
> >> +direction, DMA_ATTR_SKIP_CPU_SYNC))
> >
> > Unfortunately I don't think you can do this for a couple reasons.
> > You can't rely on {begin,end}_cpu_access calls to do cache maintenance.
> > If the calls to {begin,end}_cpu_access were made before the call to
> > dma_buf_attach then there won't have been a device attached so the calls
> > to {begin,end}_cpu_access won't have done any cache maintenance.
> >
>
> That should be okay though, if you have no attachments (or all
> attachments are IO-coherent) then there is no need for cache
> maintenance. Unless you mean a sequence where a non-io-coherent device
> is attached later after data has already been written. Does that
> sequence need supporting?
Yes, but also I think there are cases where CPU access can happen before
in Android, but I will focus on later for now.
> DMA-BUF doesn't have to allocate the backing
> memory until map_dma_buf() time, and that should only happen after all
> the devices have attached so it can know where to put the buffer. So we
> shouldn't expect any CPU access to buffers before all the devices are
> attached and mapped, right?
>
Here is an example where CPU access can happen later in Android.
Camera device records video -> software post processing -> video device
(who does compression of raw data) and writes to a file
In this example assume the buffer is cached and the devices are not
IO-coherent (quite common).
ION buffer is allocated.
//Camera device records video
dma_buf_attach
dma_map_attachment (buffer needs to be cleaned)
[camera device writes to buffer]
dma_buf_unmap_attachment (buffer needs to be invalidated)
dma_buf_detach (device cannot stay attached because it is being sent down
the pipeline and Camera doesn't know the end of the use case)
//buffer is send down the pipeline
// Usersapce software post processing occurs
mmap buffer
DMA_BUF_IOCTL_SYNC IOCT with flags DMA_BUF_SYNC_START // No CMO since no
devices attached to buffer
[CPU reads/writes to the buffer]
DMA_BUF_IOCTL_SYNC IOCTL with flags DMA_BUF_SYNC_END // No CMO since no
devices attached to buffer
munmap buffer
//buffer is send down the pipeline
// Buffer is send to video device (who does compression of raw data) and
writes to a file
dma_buf_attach
dma_map_attachment (buffer needs to be cleaned)
[video device writes to buffer]
dma_buf_unmap_attachment
dma_buf_detach (device cannot stay attached because it is being sent down
the pipeline and Video doesn't know the end of the use case)
> > Also ION no longer provides DMA ready memory, so if you are not doing CPU
> > access then there is no requirement (that I am aware of) for you to call
> > {begin,end}_cpu_access before passing the buffer to the device and if this
> > buffer is cached and your device is not IO-coherent then the cache
> > maintenance
> > in ion_map_dma_buf and ion_unmap_dma_buf is required.
> >
>
> If I am not doing any CPU access then why do I need CPU cache
> maintenance on the buffer?
>
Because ION no longer provides DMA ready memory.
Take the above example.
ION allocates memory from buddy allocator and requests zeroing.
Zeros are written to the cache.
You pass the buffer to the camera device which is not IO-coherent.
The camera devices writes directly to the buffer in DDR.
Since you didn't clean the buffer a dirty cache line (one of the zeros) is
evicted from the cache, this zero overwrites data the camera device has
written which corrupts your data.
Liam
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 202263] AMDGPU: DRM couldn't schedule ib on ring (-22)
https://bugzilla.kernel.org/show_bug.cgi?id=202263 Emre Işık (e.isi...@gmail.com) changed: What|Removed |Added CC||e.isi...@gmail.com --- Comment #7 from Emre Işık (e.isi...@gmail.com) --- Created attachment 280531 --> https://bugzilla.kernel.org/attachment.cgi?id=280531&action=edit after_patch_dmesg.log Thanks Alex for your support! I recompiled the kernel 4.19.12 with the your patch. And reinstalled TLP & rebooted the system. When I execute DRI_PRIME=1 glxgears everything works! glxinfo say's that I am using the AMD GPU. And dmesg doesn't have any errors anymore. See attachment. Thanks for the patch. Do you gonna commit this patch to the official kernel? Maybe others have the same error like me. Thanks, again! -- 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 09/10] drm/mxsfb: Improve the axi clock usage
On Mi, 2019-01-09 at 18:09 +0100, Stefan Agner wrote:
> On 09.01.2019 15:13, Robert Chiras wrote:
> >
> > Currently, the enable of the axi clock return status is ignored,
> > causing
> > issues when the enable fails then we try to disable it. Therefore,
> > it is
> > better to check the return status and disable it only when enable
> > succeeded.
> > Also, remove the helper functions around clk_axi, since we can
> > directly
> > use the clk API function for enable/disable the clock. Those
> > functions
> > are already checking for NULL clk and returning 0 if that's the
> > case.
> Can we maybe even use the runtime PM system for that (adding two
> callbacks for SET_RUNTIME_PM_OPS)?
>
> I suggested it a while ago, but did not looked deeper into it:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flk
> ml.org%2Flkml%2F2018%2F8%2F1%2F300&data=02%7C01%7Crobert.chiras%4
> 0nxp.com%7Cfb15abdd151b4c643f4f08d67655464a%7C686ea1d3bc2b4c6fa92cd99
> c5c301635%7C0%7C0%7C636826505961622511&sdata=MxpNiBXFW6gw9U8rKRF2
> 0ji73tZ5X5GqZWcninDBFWk%3D&reserved=0
>
> Since we basically enable on mxsfb_crtc_enable and disable on
> mxsfb_crtc_disable, I think it would be pretty much the same thing.
>
> --
> Stefan
>
Hi Stefan,
I don't think I fully understand your suggestion here. The axi clock is
used when we are trying to access LCDIF registers while the block is
not running. For example, when the vblank needs to be enabled/disabled
while the block is idle (so it doesn't have anything to do with
mxsfb_crtc_enable/disable). How could the PM callbacks handle this
case?
Thanks,
Robert
>
> >
> >
> > Signed-off-by: Robert Chiras
> > Acked-by: Leonard Crestez
> > ---
> > drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 8
> > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 32 +-
> > --
> > drivers/gpu/drm/mxsfb/mxsfb_drv.h | 3 ---
> > 3 files changed, 17 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > index 8d1b6a6..b9437c7 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > @@ -411,7 +411,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private
> > *mxsfb)
> > {
> > dma_addr_t paddr;
> >
> > - mxsfb_enable_axi_clk(mxsfb);
> > + clk_prepare_enable(mxsfb->clk_axi);
> > writel(0, mxsfb->base + LCDC_CTRL);
> > mxsfb_crtc_mode_set_nofb(mxsfb);
> >
> > @@ -428,7 +428,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private
> > *mxsfb)
> > void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb)
> > {
> > mxsfb_disable_controller(mxsfb);
> > - mxsfb_disable_axi_clk(mxsfb);
> > + clk_disable_unprepare(mxsfb->clk_axi);
> > }
> >
> > void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
> > @@ -456,9 +456,9 @@ void mxsfb_plane_atomic_update(struct
> > mxsfb_drm_private *mxsfb,
> >
> > paddr = mxsfb_get_fb_paddr(mxsfb);
> > if (paddr) {
> > - mxsfb_enable_axi_clk(mxsfb);
> > + clk_prepare_enable(mxsfb->clk_axi);
> > writel(paddr, mxsfb->base + mxsfb->devdata-
> > >next_buf);
> > - mxsfb_disable_axi_clk(mxsfb);
> > + clk_disable_unprepare(mxsfb->clk_axi);
> > }
> >
> > if (!fb || !old_fb)
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > index 135b8e1..5e18353 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > @@ -103,18 +103,6 @@ drm_pipe_to_mxsfb_drm_private(struct
> > drm_simple_display_pipe *pipe)
> > return container_of(pipe, struct mxsfb_drm_private, pipe);
> > }
> >
> > -void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb)
> > -{
> > - if (mxsfb->clk_axi)
> > - clk_prepare_enable(mxsfb->clk_axi);
> > -}
> > -
> > -void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
> > -{
> > - if (mxsfb->clk_axi)
> > - clk_disable_unprepare(mxsfb->clk_axi);
> > -}
> > -
> > /**
> > * mxsfb_atomic_helper_check - validate state object
> > * @dev: DRM device
> > @@ -237,25 +225,31 @@ static void mxsfb_pipe_update(struct
> > drm_simple_display_pipe *pipe,
> > static int mxsfb_pipe_enable_vblank(struct drm_simple_display_pipe
> > *pipe)
> > {
> > struct mxsfb_drm_private *mxsfb =
> > drm_pipe_to_mxsfb_drm_private(pipe);
> > + int ret = 0;
> > +
> > + ret = clk_prepare_enable(mxsfb->clk_axi);
> > + if (ret)
> > + return ret;
> >
> > /* Clear and enable VBLANK IRQ */
> > - mxsfb_enable_axi_clk(mxsfb);
> > writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1
> > + REG_CLR);
> > writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base +
> > LCDC_CTRL1 + REG_SET);
> > - mxsfb_disable_axi_clk(mxsfb);
> > + clk_disable_unprepare(mxsfb->clk_axi);
> >
> > - return 0;
> > + return ret;
> > }
> >
> > static void mxsfb_pipe_disable_vblank(struct
> > drm_simple_display_pipe *pipe)
> > {
> >
Re: [PATCH v2] drm/xen-front: Make shmem backed display buffer coherent
On 1/16/19 8:30 AM, Gerd Hoffmann wrote:
>Hi,
>
>> +if (!dma_map_sg(dev->dev, xen_obj->sgt->sgl, xen_obj->sgt->nents,
>> +DMA_BIDIRECTIONAL)) {
>> +ret = -EFAULT;
>> +goto fail_free_sgt;
>> +}
> Hmm, so it seems the arm guys could not come up with a suggestion how to
> solve that one in a better way. Ok, lets go with this then.
>
> But didn't we agree that this deserves a comment exmplaining the purpose
> of the dma_map_sg() call is to flush caches and that there is no actual
> DMA happening here?
My bad, will add the comment
> cheers,
>Gerd
>
Thank you,
Oleksandr
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next] drm/vmwgfx: Remove set but not used variable 'srf'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c: In function 'vmw_hw_surface_destroy':
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c:335:22: warning:
variable 'srf' set but not used [-Wunused-but-set-variable]
It never used since commit 543831cfc976 ï¼"drm/vmwgfx: Break out surface and
context management to separate files")
Signed-off-by: YueHaibing
---
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 80a01cd..3b6976e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -332,7 +332,6 @@ static void vmw_hw_surface_destroy(struct vmw_resource *res)
{
struct vmw_private *dev_priv = res->dev_priv;
- struct vmw_surface *srf;
void *cmd;
if (res->func->destroy == vmw_gb_surface_destroy) {
@@ -359,7 +358,6 @@ static void vmw_hw_surface_destroy(struct vmw_resource *res)
*/
mutex_lock(&dev_priv->cmdbuf_mutex);
- srf = vmw_res_to_srf(res);
dev_priv->used_memory_size -= res->backup_size;
mutex_unlock(&dev_priv->cmdbuf_mutex);
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 1/2] fbcon: Silence fbcon logo on 'quiet' boots
On Fri 2019-01-11 15:04:32, Sergey Senozhatsky wrote:
> On (01/10/19 14:03), Prarit Bhargava wrote:
> > +++ b/drivers/video/fbdev/core/fbcon.c
> > @@ -649,11 +649,14 @@ static void fbcon_prepare_logo(struct vc_data *vc,
> > struct fb_info *info,
> > kfree(save);
> > }
> >
> > + if (logo_shown == FBCON_LOGO_DONTSHOW)
> > + return;
>
> + if (console_loglevel <= CONSOLE_LOGLEVEL_QUIET)
> + return;
>
> Would this two-liner do the trick?
But then the other functions, like fbcon_scroll() or fbcon_switch(),
would think that the logo was shown because logo_shown !=
FBCON_LOGO_DONTSHOW.
It might be possible to do logo_shown = FBCON_LOGO_DONTSHOW.
But I am not sure if it is safe, see below.
> -ss
>
> > +
> > if (logo_lines > vc->vc_bottom) {
> > logo_shown = FBCON_LOGO_CANSHOW;
> > printk(KERN_INFO
> >"fbcon_init: disable boot-logo (boot-logo bigger than
> > screen).\n");
> > - } else if (logo_shown != FBCON_LOGO_DONTSHOW) {
> > + } else {
> > logo_shown = FBCON_LOGO_DRAW;
> > vc->vc_top = logo_lines;
> > }
> > @@ -1059,9 +1062,11 @@ static void fbcon_init(struct vc_data *vc, int init)
> >
> > cap = info->flags;
> >
> > - if (vc != svc || logo_shown == FBCON_LOGO_DONTSHOW ||
> > - (info->fix.type == FB_TYPE_TEXT))
> > + if (vc != svc || console_loglevel <= CONSOLE_LOGLEVEL_QUIET ||
> > + (info->fix.type == FB_TYPE_TEXT)) {
> > logo = 0;
> > + logo_shown = FBCON_LOGO_DONTSHOW;
This has still the problem mentioned in the v3 feedback. It modifies
the global variable logo_shown even when vc != svc. You still need
check and explain why it is safe. It is not clear to me.
Best Regards,
Petr
> > + }
> >
> > if (var_to_display(p, &info->var, info))
> > return;
> > --
> > 2.17.2
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Tue, Jan 15, 2019 at 02:17:26PM +, Thomas Hellstrom wrote: > Hi, Christoph, > > On Mon, 2019-01-14 at 10:48 +0100, Christoph Hellwig wrote: > > On Thu, Jan 10, 2019 at 04:42:18PM -0700, Jason Gunthorpe wrote: > > > > Changes since the RFC: > > > > - Rework vmwgfx too [CH] > > > > - Use a distinct type for the DMA page iterator [CH] > > > > - Do not have a #ifdef [CH] > > > > > > ChristophH: Will you ack? > > > > This looks generally fine. > > > > > Are you still OK with the vmwgfx reworking, or should we go back to > > > the original version that didn't have the type safety so this > > > driver > > > can be left broken? > > > > I think the map method in vmgfx that just does virt_to_phys is > > pretty broken. Thomas, can you check if you see any performance > > difference with just doing the proper dma mapping, as that gets the > > driver out of interface abuse land? > > The performance difference is not really the main problem here. The > problem is that even though we utilize the streaming DMA interface, we > use it only since we have to for DMA-Remapping and assume that the > memory is coherent. To be able to be as compliant as possible and ditch > the virt-to-phys mode, we *need* a DMA interface flag that tells us > when the dma_sync_for_xxx are no-ops. If they aren't we'll refuse to > load for now. I'm not sure, but I think also nouveau and radeon suffer > from the same issue. RDMA needs something similar as well, in this case drivers take a struct page * from get_user_pages() and need to have the DMA map fail if the platform can't DMA map in a way that does not require any additional DMA API calls to ensure coherence. (think Userspace RDMA MR's) Today we just do the normal DMA map and when it randomly doesn't work and corrupts data tell those people their platforms don't support RDMA - it would be nice to have a safer API base solution.. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/xen-front: Make shmem backed display buffer coherent
On 1/16/19 8:36 AM, Christoph Hellwig wrote:
> On Wed, Jan 16, 2019 at 07:30:02AM +0100, Gerd Hoffmann wrote:
>>Hi,
>>
>>> + if (!dma_map_sg(dev->dev, xen_obj->sgt->sgl, xen_obj->sgt->nents,
>>> + DMA_BIDIRECTIONAL)) {
>>> + ret = -EFAULT;
>>> + goto fail_free_sgt;
>>> + }
>> Hmm, so it seems the arm guys could not come up with a suggestion how to
>> solve that one in a better way. Ok, lets go with this then.
>>
>> But didn't we agree that this deserves a comment exmplaining the purpose
>> of the dma_map_sg() call is to flush caches and that there is no actual
>> DMA happening here?
> Using a dma mapping call to flush caches is complete no-go. But the
> real question is why you'd even want to flush cashes if you do not
> want a dma mapping?
>
> This whole issue keeps getting more and more confusing.
Well, I don't really do DMA here, but instead the buffers in
question are shared with other Xen domain, so effectively it
could be thought of some sort of DMA here, where the "device" is
that remote domain. If the buffers are not flushed then the
remote part sees some inconsistency which in my case results
in artifacts on screen while displaying the buffers.
When buffers are allocated via DMA API then there are no artifacts;
if buffers are allocated with shmem + DMA mapping then there are no
artifacts as well.
The only offending use-case is when I use shmem backed buffers,
but do not flush them
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] Re: HDMI/DVI spurious failure
On Tue, Jan 15, 2019 at 10:49:51AM +0100, Maxime Ripard wrote: > Hi, > > (Sending those kind of bug reports to linux-sunxi doesn't really work, > you should Cc at least the ML involved in the development of the > driver at fault). OK, sorry for that. > > On Mon, Jan 14, 2019 at 01:29:34PM +, Priit Laes wrote: > > I have a somewhat curious case with one HDMI/DVI screen that fails > > to initialize properly every 6-7 boots. The display itself is also > > somewhat flawed (missing HPD pin and the VSYNC/HSYNC pulse width > > is set to 0 in EDID), but I suspect there could be some issues > > regarding timing in A20 HDMI driver in Linux. > > ... > It doesn't look related to the clock rate itself, since it doesn't > change between the two cases. However, in one case the DDC clock is > enabled and in the other it's disabled. > > Was it taken at the same time? Maybe you can try with that patch? > http://code.bulix.org/z7jmkm-555344?raw Thanks, after doing ~50+ boots I haven't seen a single failure. Previously I had following failure cases which are now both fixed: a) Linux without u-boot HDMI, where one in every 6-7 boots failed. b) u--boot with hdmi enabled switching to simplefb and then switching to kms, where previously all boots ended up with garbled screen. Priit :) > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > > -- > You received this message because you are subscribed to the Google Groups > "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to linux-sunxi+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/ttm: force cached mappings for system RAM on ARM
Hi Will, Will Deacon writes: > [+ BenH and MPE] > > On Mon, Jan 14, 2019 at 07:21:08PM +, Koenig, Christian wrote: >> Am 14.01.19 um 20:13 schrieb Will Deacon: ... > >> > The Arm architecture (and others including Power afaiu) doesn't >> > guarantee coherency when memory is accessed using mismatched cacheability >> > attributes. ... > >> As far as I know Power doesn't really supports un-cached memory at all, >> except for a very very old and odd configuration with AGP. > > Hopefully Michael/Ben can elaborate here, but I was under the (possibly > mistaken) impression that mismatched attributes could cause a machine-check > on Power. That's what I've always been told, but I can't actually find where it's documented, I'll keep searching. But you're right that mixing cached / uncached is not really supported, and probably results in a machine check or worse. cheers ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/ttm: force cached mappings for system RAM on ARM
On Wed, 16 Jan 2019 at 08:36, Koenig, Christian wrote: > > Am 16.01.19 um 01:33 schrieb Benjamin Herrenschmidt: > > On Tue, 2019-01-15 at 22:31 +1100, Michael Ellerman wrote: > As far as I know Power doesn't really supports un-cached memory at all, > except for a very very old and odd configuration with AGP. > >>> Hopefully Michael/Ben can elaborate here, but I was under the (possibly > >>> mistaken) impression that mismatched attributes could cause a > >>> machine-check > >>> on Power. > >> That's what I've always been told, but I can't actually find where it's > >> documented, I'll keep searching. > >> > >> But you're right that mixing cached / uncached is not really supported, > >> and probably results in a machine check or worse. > > .. or worse :) It could checkstop. > > Not sure if that would be so bad, it would at least give us a clear > indicator that something is wrong instead of silently corrupting data. > > > It's also my understanding that on ARM v7 and above, it's technically > > forbidden to map the same physical page with both cached and non-cached > > mappings, since the cached one could prefetch (or speculatively load), > > thus creating collisions and inconsistencies. Am I wrong here ? > > No, but you answer the wrong question. > > See we don't want to have different mappings of cached and non-cached on > the CPU, but rather want to know if a snooped DMA from the PCIe counts > as cached access as well. > > As far as I know on x86 it doesn't, so when you have an un-cached page > you can still access it with a snooping DMA read/write operation and > don't cause trouble. > I think it is the other way around. The question is, on an otherwise cache coherent device, whether the NoSnoop attribute set by the GPU propagates all the way to the bus so that it bypasses the caches. On x86, we can tolerate if this is not the case, since uncached memory accesses by the CPU snoop the caches as well. On other architectures, uncached accesses go straight to main memory, so if the device wrote anything to the caches we won't see it. So to use this optimization, you have to either be 100% sure that NoSnoop is implemented correctly, or have a x86 CPU. > > The old hack of using non-cached mapping to avoid snoop cost in AGP and > > others is just that ... an ugly and horrible hacks that should have > > never eventuated, when the search for performance pushes HW people into > > utter insanity :) > > Well I agree that un-cached system memory makes things much more > complicated for a questionable gain. > > But fact is we now have to deal with the mess, so no point in > complaining about it to much :) > Indeed. I wonder if we should just disable it altogether unless CONFIG_X86=y ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 102909] radeon 0000:03:00.0: ring 0 stalled for more than 10000msec
https://bugs.freedesktop.org/show_bug.cgi?id=102909 Michel Dänzer changed: What|Removed |Added Resolution|--- |WONTFIX Status|NEW |RESOLVED --- Comment #5 from Michel Dänzer --- Resolving per comment 4, thanks for the report and follow-ups. -- 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: [Spice-devel] [PATCH 1/3] drm/qxl: add mode/framebuffer check functions
>
> Add a helper functions to check video modes. Also add a helper to check
> framebuffer buffer objects, using the former for consistency. That way
> we should not fail in qxl_primary_atomic_check() because video modes
> which are too big will not be added to the mode list in the first place.
>
> Signed-off-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/qxl/qxl_display.c | 37 +++--
> 1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> b/drivers/gpu/drm/qxl/qxl_display.c
> index 1f8fddcc34..07a37d52c4 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -191,6 +191,21 @@ void qxl_display_read_client_monitors_config(struct
> qxl_device *qdev)
> }
> }
>
> +static int qxl_check_mode(struct qxl_device *qdev,
> + unsigned int width,
> + unsigned int height)
> +{
> + if (width * height * 4 > qdev->vram_size)
Is someone checking for integer overflows already?
> + return -ENOMEM;
> + return 0;
> +}
> +
> +static int qxl_check_framebuffer(struct qxl_device *qdev,
> + struct qxl_bo *bo)
> +{
> + return qxl_check_mode(qdev, bo->surf.width, bo->surf.height);
> +}
> +
> static int qxl_add_monitors_config_modes(struct drm_connector *connector,
> unsigned *pwidth,
> unsigned *pheight)
> @@ -466,12 +481,7 @@ static int qxl_primary_atomic_check(struct drm_plane
> *plane,
>
> bo = gem_to_qxl_bo(state->fb->obj[0]);
>
> - if (bo->surf.stride * bo->surf.height > qdev->vram_size) {
> - DRM_ERROR("Mode doesn't fit in vram size (vgamem)");
> - return -EINVAL;
> - }
> -
> - return 0;
> + return qxl_check_framebuffer(qdev, bo);
> }
>
> static int qxl_primary_apply_cursor(struct drm_plane *plane)
> @@ -941,20 +951,11 @@ static enum drm_mode_status qxl_conn_mode_valid(struct
> drm_connector *connector,
> {
> struct drm_device *ddev = connector->dev;
> struct qxl_device *qdev = ddev->dev_private;
> - int i;
>
> - /* TODO: is this called for user defined modes? (xrandr --add-mode)
> - * TODO: check that the mode fits in the framebuffer */
> + if (qxl_check_mode(qdev, mode->hdisplay, mode->vdisplay) != 0)
> + return MODE_BAD;
>
> - if (qdev->monitors_config_width == mode->hdisplay &&
> - qdev->monitors_config_height == mode->vdisplay)
> - return MODE_OK;
> -
> - for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
> - if (common_modes[i].w == mode->hdisplay && common_modes[i].h ==
> mode->vdisplay)
> - return MODE_OK;
> - }
> - return MODE_BAD;
> + return MODE_OK;
> }
>
> static struct drm_encoder *qxl_best_encoder(struct drm_connector *connector)
Frediano
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PULL] drm-misc-next
On Tue, Jan 15, 2019 at 4:57 PM Maxime Ripard wrote:
>
> Hi Daniel, Dave,
>
> Here is the drm-misc-next PR for this week.
>
> Thanks!
> Maxime
>
> drm-misc-next-2019-01-15:
> drm-misc-next for 5.1:
>
> UAPI Changes:
>
> Cross-subsystem Changes:
>
> Core Changes:
> - Reorganisation of drm_device and drm_framebuffer headers
> - Cleanup of the drmP inclusion
> - Fix leaks in the fb-helpers
> - Allow for depth different from bpp in fb-helper fbdev emulation
>
> Driver Changes:
> - Add reflection properties to rockchip
> - a bunch of fixes for virtio
> - a bunch of fixes for dp_mst and drivers using it, and introduction of a
>new refcounting scheme
Unfortunately this had a bug in the i915 mst errno handling, which
Lyude fixed in
commit 96550555a78ca3c9fda4b358549a5622810fe32c
Author: Lyude Paul
Date: Tue Jan 15 15:08:00 2019 -0500
drm/i915: Pass down rc in intel_encoder->compute_config()
This patch is also causing quite some conflicts with in-flight
drm-intel stuff. Can you please respin your pull request, since
getting the mst stuff in a half-broken state and without the massive
conflicting patches would cause pains for intel folks?
Thanks, Daniel
> - Convertion of bochs to atomic and generic fbdev emulation
> - Allow meson to remove the firmware framebuffers
> The following changes since commit e3d093070eb0b5e3df668d3eb04100ea79343c65:
>
> Merge tag 'tilcdc-4.22' of https://github.com/jsarha/linux into drm-next
> (2019-01-11 06:29:31 +1000)
>
> are available in the Git repository at:
>
> git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2019-01-15
>
> for you to fetch changes up to ed20151a7699bb2c77eba3610199789a126940c4:
>
> drm/vblank: Allow dynamic per-crtc max_vblank_count (2019-01-14 21:23:55
> +0200)
>
>
> drm-misc-next for 5.1:
>
> UAPI Changes:
>
> Cross-subsystem Changes:
>
> Core Changes:
> - Reorganisation of drm_device and drm_framebuffer headers
> - Cleanup of the drmP inclusion
> - Fix leaks in the fb-helpers
> - Allow for depth different from bpp in fb-helper fbdev emulation
>
> Driver Changes:
> - Add reflection properties to rockchip
> - a bunch of fixes for virtio
> - a bunch of fixes for dp_mst and drivers using it, and introduction of a
>new refcounting scheme
> - Convertion of bochs to atomic and generic fbdev emulation
> - Allow meson to remove the firmware framebuffers
>
>
> Daniel Vetter (14):
> drm/todo: Better defio support in the generic fbdev emulation
> drm/crtc-helpers: WARN when used with atomic drivers
> drm/ch7006: Stop using drm_crtc_force_disable
> drm/nouveau: Stop using drm_crtc_force_disable
> drm: Unexport drm_crtc_force_disable
> drm/atomic: Add missing () to function ref in kerneldoc
> drm: Move the legacy kms disable_all helper to crtc helpers
> drm/arc: Don't set the dpms hook
> drm/tda998x: Don't set dpms hook
> drm/doc: Polish kerneldoc for drm_device.h
> drm/docs: improve docs for drm_drv.c
> drm/of: Fix kerneldoc
> drm/panel: Small documentation polish
> drm/doc: Move bridge link target to the right place
>
> Daniele Castagna (2):
> drm/rockchip: Fix YUV buffers color rendering
> drm/rockchip: Add reflection properties
>
> Enric Balletbo i Serra (1):
> drm/rockchip: update cursors asynchronously through atomic.
>
> Ezequiel Garcia (5):
> drm/virtio: Remove incorrect kfree()
> drm/virtio: Add missing virtqueue reset
> drm/virtio: Drop deprecated load/unload initialization
> drm/rockchip: Fix typo in VOP macros argument
> drm/rockchip: Separate RK3288 from RK3368 win01 registers
>
> Gerd Hoffmann (19):
> drm/virtio: log error responses
> drm/virtio: fix pageflip flush
> drm/virtio: drop virtio_gpu_fence_cleanup()
> drm/bochs: encoder cleanup
> drm/bochs: split bochs_hw_setmode
> drm/bochs: atomic: add atomic_flush+atomic_enable callbacks.
> drm/bochs: atomic: add mode_set_nofb callback.
> drm/bochs: atomic: switch planes to atomic, wire up helpers.
> drm/bochs: atomic: use atomic set_config helper
> drm/bochs: atomic: use atomic page_flip helper
> drm/bochs: atomic: use suspend/resume helpers
> drm/bochs: atomic: set DRIVER_ATOMIC
> drm/bochs: remove old bochs_crtc_* functions
> drm/bochs: drop unused gpu_addr arg from bochs_bo_pin()
> drm/bochs: move ttm_bo_(un)reserve calls into bochs_bo_{pin, unpin}
> drm/bochs: add basic prime support
> drm/bochs: switch to generic drm fbdev emulation
> drm/bochs: drop old fbdev emulation code
> drm/bochs: move remaining fb bits to kms
>
> Gustavo A. R. Silva (1):
> qxl: Use struct_size() in kzalloc()
>
> Kuo-Hsin Yang (1):
> drm/gem: Mark pinned pages as unevictable
>
> Linus Walleij (2)
[PATCH] staging/xgifb: Needs to be converted to a drm driver
The fbdev subsystem is closed for new drivers, those need to become drm ones (which generally results in smaller drivers nowadays, with the massive amounts of shared infrastructure and helper libraries drm has). Although given the lack of progress since 2010, maybe time to ditch it from staging outright? Signed-off-by: Daniel Vetter Cc: Arnaud Patard Cc: Daniel Vetter Cc: Greg Kroah-Hartman --- drivers/staging/xgifb/TODO | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/xgifb/TODO b/drivers/staging/xgifb/TODO index 7eb99140a399..a1e25957bf1b 100644 --- a/drivers/staging/xgifb/TODO +++ b/drivers/staging/xgifb/TODO @@ -9,5 +9,8 @@ TODO: - remove useless/wrong/unused code... - get rid of non-linux related stuff +This needs to become a drm driver, the fbdev subsystem doesn't take new drivers +anymore. + Please send patches to: Arnaud Patard -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] staging/vboxvideo: Don't set FBINFO_MISC_ALWAYS_SETPAR
On Tue, Jan 15, 2019 at 4:32 PM Greg Kroah-Hartman wrote: > > On Tue, Jan 15, 2019 at 04:15:49PM +0100, Daniel Vetter wrote: > > On Tue, Jan 15, 2019 at 02:45:53PM +0100, Greg Kroah-Hartman wrote: > > > On Tue, Jan 15, 2019 at 01:12:28PM +0100, Daniel Vetter wrote: > > > > On Tue, Jan 15, 2019 at 11:38:29AM +0100, Greg Kroah-Hartman wrote: > > > > > On Tue, Jan 15, 2019 at 11:27:54AM +0100, Daniel Vetter wrote: > > > > > > It's a debug hack flag useful to work around driver bugs. That's > > > > > > not a > > > > > > good idea for a new driver. Especially for a new drm driver. > > > > > > > > > > > > Aside: the fbdev support should probably be converted over to the > > > > > > new > > > > > > generic fbdev support. > > > > > > > > > > > > Signed-off-by: Daniel Vetter > > > > > > Cc: Greg Kroah-Hartman > > > > > > Cc: Hans de Goede > > > > > > Cc: Daniel Vetter > > > > > > Cc: Bartlomiej Zolnierkiewicz > > > > > > Cc: Alexander Kapshuk > > > > > > --- > > > > > > drivers/staging/vboxvideo/vbox_fb.c | 5 - > > > > > > 1 file changed, 5 deletions(-) > > > > > > > > > > > > > > > > Reviewed-by: Greg Kroah-Hartman > > > > > > > > Since Hans wants to destage vboxvideo this cycle probably best I merge > > > > this through drm-misc? Just to make sure it's not lost. > > > > > > Feel free to do so! > > > > Applied, thanks for your review. > > > > I think 2/2 is directly staging material for you (somehow > > get_maintainers.pl didn't add you to the cc: list, not sure why ...). > > Can you resend it, I don't see it anywhere... resent with you added explicitly to the Cc: list, it's Subject: [PATCH] staging/xgifb: Needs to be converted to a drm driver Date: Wed, 16 Jan 2019 11:04:40 +0100 Message-Id: <20190116100440.10071-1-daniel.vet...@ffwll.ch> Cheers, Daniel > ___ > 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
[Bug 109375] Regression: dark notebook display (no backlight) after resume
https://bugs.freedesktop.org/show_bug.cgi?id=109375
Bug ID: 109375
Summary: Regression: dark notebook display (no backlight) after
resume
Product: DRI
Version: XOrg git
Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
Severity: normal
Priority: medium
Component: DRM/AMDgpu
Assignee: dri-devel@lists.freedesktop.org
Reporter: zaj...@gmail.com
I use HP EliteBook 745 G5 with Ryzen 5 PRO 2500U.
Starting with the:
commit 262485a50fd4532a8d71165190adc7a0a19bcc9e (refs/bisect/bad)
Author: Anthony Koo
Date: Fri Oct 12 21:34:32 2018 -0400
drm/amd/display: Expand dc to use 16.16 bit backlight
[Why] We want to increase precision for backlight setting.
But DC interface takes 8 bit backlight level value only.
[How] DMCU already takes 16 bit backlight level.
Expand the DC interface to take 16.16 bit value.
Max 32 bit backlight value (0x) will represent
max backlight (100%)
Signed-off-by: Anthony Koo
Reviewed-by: Tony Cheng
Acked-by: Leo Li
Signed-off-by: Alex Deucher
After resume from RAM my notebook display is all black.
Before suspend:
> cat /sys/class/backlight/amdgpu_bl0/{actual_brightness,brightness}
65535
255
After resume:
> cat /sys/class/backlight/amdgpu_bl0/{actual_brightness,brightness}
0
255
This can be workarounded by:
echo 255 > /sys/class/backlight/amdgpu_bl0/brightness
assuming one can execute that without a working display.
This problem still exists in the 5.0.0-rc2.
--
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] lib/scatterlist: Provide a DMA page iterator
On Wed, Jan 16, 2019 at 07:28:13AM +, Koenig, Christian wrote: > Am 16.01.19 um 08:09 schrieb Thomas Hellstrom: > > On Tue, 2019-01-15 at 21:58 +0100, h...@lst.de wrote: > >> On Tue, Jan 15, 2019 at 07:13:11PM +, Koenig, Christian wrote: > >>> Thomas is correct that the interface you propose here doesn't work > >>> at > >>> all for GPUs. > >>> > >>> The kernel driver is not informed of flush/sync, but rather just > >>> setups > >>> coherent mappings between system memory and devices. > >>> > >>> In other words you have an array of struct pages and need to map > >>> that to > >>> a specific device and so create dma_addresses for the mappings. > >> If you want a coherent mapping you need to use dma_alloc_coherent > >> and dma_mmap_coherent and you are done, that is not the problem. > >> That actually is one of the vmgfx modes, so I don't understand what > >> problem we are trying to solve if you don't actually want a non- > >> coherent mapping. > > For vmwgfx, not making dma_alloc_coherent default has a couple of > > reasons: > > 1) Memory is associated with a struct device. It has not been clear > > that it is exportable to other devices. > > 2) There seems to be restrictions in the system pages allowable. GPUs > > generally prefer highmem pages but dma_alloc_coherent returns a virtual > > address implying GFP_KERNEL? While not used by vmwgfx, TTM typically > > prefers HIGHMEM pages to facilitate caching mode switching without > > having to touch the kernel map. > > 3) Historically we had APIs to allow coherent access to user-space > > defined pages. That has gone away not but the infrastructure was built > > around it. > > > > dma_mmap_coherent isn't use because as the data moves between system > > memory, swap and VRAM, PTEs of user-space mappings are adjusted > > accordingly, meaning user-space doesn't have to unmap when an operation > > is initiated that might mean the data is moved. > > To summarize once more: We have an array of struct pages and want to > coherently map that to a device. > > If that is not possible because of whatever reason we want to get an > error code or even not load the driver from the beginning. I guess to make this work we'd also need information about how we're allowed to mmap this on the cpu side, both from the kernel (kmap or vmap) and for userspace. At least for i915 we use all kinds of combinations, e.g. cpu mmap ptes as cached w/ coherent device transactions, or cached+clflush on the cpu side, and non-coherent device transactions (the no-snoop thing), or wc mode in the cpu ptes and non-coherent device transactions- Plus some debug mode so we catch abuse, because reality is that most of the gpu driver work happens on x86, where all of this just works. Even if you do some really serious layering violations (which is why this isn't that high a priority for gpu folks). -Daniel > > > > > > >> Although last time I had that discussion with Daniel Vetter > >> I was under the impressions that GPUs really wanted non-coherent > >> mappings. > > Intel historically has done things a bit differently. And it's also > > possible that embedded platforms and ARM prefer this mode of operation, > > but I haven't caught up on that discussion. > > > >> But if you want a coherent mapping you can't go to a struct page, > >> because on many systems you can't just map arbitrary memory as > >> uncachable. It might either come from very special limited pools, > >> or might need other magic applied to it so that it is not visible > >> in the normal direct mapping, or at least not access through it. > > > > The TTM subsystem has been relied on to provide coherent memory with > > the option to switch caching mode of pages. But only on selected and > > well tested platforms. On other platforms we simply do not load, and > > that's fine for now. > > > > But as mentioned multiple times, to make GPU drivers more compliant, > > we'd really want that > > > > bool dma_streaming_is_coherent(const struct device *) > > > > API to help us decide when to load or not. > > Yes, please. > > Christian. > > > > > Thanks, > > Thomas > > > > > > > > > > > > > > > -- 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 v2 1/1] drm: fix alpha build after drm_util.h change
On Tue, Jan 15, 2019 at 10:48:45PM +0100, Sam Ravnborg wrote:
> 0-DAY reported the following bug:
>
> tree: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> head: 21376e2c3c5bad5e87ba700c055c8a8235c2bfd5
> commit: e9eafcb589213395232084a2378e2e90f67feb29 [1/2] drm: move
> drm_can_sleep() to drm_util.h
> config: alpha-allmodconfig (attached as .config)
> ...
>In file included from include/linux/irqflags.h:16:0,
> from include/drm/drm_util.h:35,
> from drivers/gpu/drm/qxl/qxl_cmd.c:28:
> >> arch/alpha/include/asm/irqflags.h:58:15: error: unknown type name 'bool'
> static inline bool arch_irqs_disabled_flags(unsigned long flags)
> ^~~~
>
> And later following bug:
> tree: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> head: 21376e2c3c5bad5e87ba700c055c8a8235c2bfd5
> commit: e9eafcb589213395232084a2378e2e90f67feb29 [1/2] drm: move
> drm_can_sleep() to drm_util.h
> config: ia64-allyesconfig (attached as .config)
> ...
>In file included from arch/ia64/include/asm/irqflags.h:14,
> from include/linux/irqflags.h:16,
> from include/drm/drm_util.h:35,
> from drivers/gpu/drm/qxl/qxl_cmd.c:28:
>arch/ia64/include/asm/pal.h: In function 'ia64_pal_tr_read':
>arch/ia64/include/asm/pal.h:1703:64: error: implicit declaration of
> function 'ia64_tpa'; did you mean 'ia64_pal'?
> [-Werror=implicit-function-declaration]
> PAL_CALL_PHYS_STK(iprv, PAL_VM_TR_READ, reg_num,
> tr_type,(u64)ia64_tpa(tr_buffer));
>^~~~
> ...
>
> So we have a situation where we do not pull in
> when building for alpha and for ia64 we need even more definitions
> are required.
>
> Two invasive fixes where considered:
> - Change all declarations of arch_irqs_disabled_flags() to use bool
> - Add include of to all files that uses bool for
> arch_irqs_disabled_flags
>
> To invasive with a too high pain/benefit ratio, so dropped.
> They would not cover ia64 either.
>
> Some less invasive fixes was also considered:
> - Add include of to drm_util.h
> - Add include of to drm_util.h
>
> The first was dropped as this did not cover the ia64 case.
>
> The latter was considered the best option as there could
> be other similar cases and we would like the header files below
> include/drm/ to be selfcontained.
> So we end up pulling in a lot of stuff not needed, but this is
> the price we pay in drm/ because the kernel headers are not all
> selfcontained.
>
> While at it, ordred the includefiles in drm_util in alphabetical order.
>
> Build tested with alpha,ia64,arm,x86 with allmodconfig and allyesconfig.
>
> v2:
> - fix ia64 build, changed to include interrupt.h
> - sort include files alphabetically
>
> Fixes: 733748ac37b45 ("drm: move drm_can_sleep() to drm_util.h")
> Signed-off-by: Sam Ravnborg
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Sean Paul
> Cc: David Airlie
> Cc: Daniel Vetter
Applied, thanks for your patch.
-Daniel
> ---
> include/drm/drm_util.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h
> index 0500da65b1d1..8163d35f8327 100644
> --- a/include/drm/drm_util.h
> +++ b/include/drm/drm_util.h
> @@ -32,9 +32,9 @@
> * Macros and inline functions that does not naturally belong in other places
> */
>
> -#include
> -#include
> +#include
> #include
> +#include
> #include
>
> /*
> --
> 2.12.0
>
--
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
[Bug 109375] Regression: dark notebook display (no backlight) after resume
https://bugs.freedesktop.org/show_bug.cgi?id=109375
--- Comment #1 from Rafał Miłecki ---
For a comparison: behavior with the commit 262485a50fd4~1 (before the
regression):
Before suspend:
> cat /sys/class/backlight/amdgpu_bl0/{actual_brightness,brightness}
255
255
After resume:
> cat /sys/class/backlight/amdgpu_bl0/{actual_brightness,brightness}
250
255
Looks like a problem with amdgpu restoring backlight after resume.
--
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 v2] drm/i915: Pass down rc in intel_encoder->compute_config()
On Wed, Jan 16, 2019 at 09:47:17AM +0200, Jani Nikula wrote:
> On Tue, 15 Jan 2019, Lyude Paul wrote:
> > Something that I completely missed when implementing the new MST VCPI
> > atomic helpers is that with those helpers, there's technically a chance
> > of us having to grab additional modeset locks in ->compute_config() and
> > furthermore, that means we have the potential to hit a normal modeset
> > deadlock. However, because ->compute_config() only returns a bool this
> > means we can't return -EDEADLK when we need to drop locks and try again
> > which means we end up just failing the atomic check permanently. Whoops.
> >
> > So, fix this by modifying ->compute_config() to pass down an actual
> > error code instead of a bool so that the atomic check can be restarted
> > on modeset deadlocks.
> >
> > Thanks to Ville Syrjälä for pointing this out!
> >
> > Changes since v1:
> > * Add some newlines
> > * Return only -EINVAL from hsw_crt_compute_config()
> > * Propogate return code from intel_dp_compute_dsc_params()
> > * Change all of the intel_dp_compute_link_config*() variants
> > * Don't miss if (hdmi_port_clock_valid()) branch in
> > intel_hdmi_compute_config()
> >
> > Signed-off-by: Lyude Paul
> > Cc: Ville Syrjälä
> > Fixes: eceae1472467 ("drm/dp_mst: Start tracking per-port VCPI allocations")
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109320
> > ---
> > drivers/gpu/drm/i915/icl_dsi.c | 8 ++--
> > drivers/gpu/drm/i915/intel_crt.c | 35 +++---
> > drivers/gpu/drm/i915/intel_ddi.c | 6 +--
> > drivers/gpu/drm/i915/intel_display.c | 11 +++--
> > drivers/gpu/drm/i915/intel_dp.c | 71 +++-
> > drivers/gpu/drm/i915/intel_dp_mst.c | 12 ++---
> > drivers/gpu/drm/i915/intel_drv.h | 18 +++
> > drivers/gpu/drm/i915/intel_dvo.c | 11 +++--
> > drivers/gpu/drm/i915/intel_hdmi.c| 14 +++---
> > drivers/gpu/drm/i915/intel_lvds.c| 12 ++---
> > drivers/gpu/drm/i915/intel_sdvo.c| 14 +++---
> > drivers/gpu/drm/i915/intel_tv.c | 8 ++--
> > drivers/gpu/drm/i915/vlv_dsi.c | 14 +++---
> > 13 files changed, 122 insertions(+), 112 deletions(-)
>
> Despite being an all i915 patch, this got applied to drm-misc-next,
> causing conflicts where there really should have been none. :(
>
> I am tempted to suggest reverting and re-applying to drm-intel, because
> it will take weeks to sync both to drm-next and backmerge, and applying
> further work on top in drm-intel will just cause more trouble.
>
> Other ideas?
We discussed this a bit on irc. I think the best option is to cherry-pick
this patch over to drm-intel-next-queued and try to get the backmerges
sorted asap.
With hindsight a topic branch for all the mst stuff would have been really
good choice. And as Jani mentioned on irc, pls get maintainer's ack when
merging stuff through different trees. I wanted to ask about that but
figured I have time until nouveau stuff is reviewed, but then it landed
right away. Anyway, conflict isn't bad enough that sfr couldn't handle it,
so we didn't screw up too badly :-)
Thanks, Daniel
>
> BR,
> Jani.
>
>
> >
> > diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> > index f3a5f03646ce..355b48d1c937 100644
> > --- a/drivers/gpu/drm/i915/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/icl_dsi.c
> > @@ -1188,9 +1188,9 @@ static void gen11_dsi_get_config(struct intel_encoder
> > *encoder,
> > pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
> > }
> >
> > -static bool gen11_dsi_compute_config(struct intel_encoder *encoder,
> > -struct intel_crtc_state *pipe_config,
> > -struct drm_connector_state *conn_state)
> > +static int gen11_dsi_compute_config(struct intel_encoder *encoder,
> > + struct intel_crtc_state *pipe_config,
> > + struct drm_connector_state *conn_state)
> > {
> > struct intel_dsi *intel_dsi = container_of(encoder, struct intel_dsi,
> >base);
> > @@ -1215,7 +1215,7 @@ static bool gen11_dsi_compute_config(struct
> > intel_encoder *encoder,
> > pipe_config->clock_set = true;
> > pipe_config->port_clock = intel_dsi_bitrate(intel_dsi) / 5;
> >
> > - return true;
> > + return 0;
> > }
> >
> > static u64 gen11_dsi_get_power_domains(struct intel_encoder *encoder,
> > diff --git a/drivers/gpu/drm/i915/intel_crt.c
> > b/drivers/gpu/drm/i915/intel_crt.c
> > index 33bd2addcbdd..081c333f30d2 100644
> > --- a/drivers/gpu/drm/i915/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > @@ -345,51 +345,52 @@ intel_crt_mode_valid(struct drm_connector *connector,
> > return MODE_OK;
> > }
> >
> > -static bool intel_crt_compute_config(struct intel_encoder *encoder,
> > -struct intel_crtc_state *pipe_config,
> > -struct drm_conne
Re: [PATCH 1/2] staging/vboxvideo: Don't set FBINFO_MISC_ALWAYS_SETPAR
On Tue, Jan 15, 2019 at 11:27:54AM +0100, Daniel Vetter wrote: > It's a debug hack flag useful to work around driver bugs. That's not a > good idea for a new driver. Especially for a new drm driver. > > Aside: the fbdev support should probably be converted over to the new > generic fbdev support. > > Signed-off-by: Daniel Vetter > Cc: Greg Kroah-Hartman > Cc: Hans de Goede > Cc: Daniel Vetter > Cc: Bartlomiej Zolnierkiewicz > Cc: Alexander Kapshuk > --- > drivers/staging/vboxvideo/vbox_fb.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/drivers/staging/vboxvideo/vbox_fb.c > b/drivers/staging/vboxvideo/vbox_fb.c > index 2181c36c19ab..1da4cb7647b8 100644 > --- a/drivers/staging/vboxvideo/vbox_fb.c > +++ b/drivers/staging/vboxvideo/vbox_fb.c > @@ -91,11 +91,6 @@ int vboxfb_create(struct drm_fb_helper *helper, > fb = &vbox->afb.base; > helper->fb = fb; > > - /* > - * The last flag forces a mode set on VT switches even if the kernel > - * does not think it is needed. > - */ > - info->flags = FBINFO_MISC_ALWAYS_SETPAR; For the record I screwed up rebasing this one because it was in some patch that also removed FBINFO_DEFAULT (which is 0, so pointless to set since the structure is kzalloc'ed). Actually merged patch also removes FBINFO_DEFAULT :-/ No real harm done though since as explained 0 doesn't matter here. -Daniel > info->fbops = &vboxfb_ops; > > /* > -- > 2.20.1 > -- 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] lib/scatterlist: Provide a DMA page iterator
Am 15.01.19 um 22:25 schrieb Jason Gunthorpe: On Tue, Jan 15, 2019 at 02:17:26PM +, Thomas Hellstrom wrote: Hi, Christoph, On Mon, 2019-01-14 at 10:48 +0100, Christoph Hellwig wrote: On Thu, Jan 10, 2019 at 04:42:18PM -0700, Jason Gunthorpe wrote: Changes since the RFC: - Rework vmwgfx too [CH] - Use a distinct type for the DMA page iterator [CH] - Do not have a #ifdef [CH] ChristophH: Will you ack? This looks generally fine. Are you still OK with the vmwgfx reworking, or should we go back to the original version that didn't have the type safety so this driver can be left broken? I think the map method in vmgfx that just does virt_to_phys is pretty broken. Thomas, can you check if you see any performance difference with just doing the proper dma mapping, as that gets the driver out of interface abuse land? The performance difference is not really the main problem here. The problem is that even though we utilize the streaming DMA interface, we use it only since we have to for DMA-Remapping and assume that the memory is coherent. To be able to be as compliant as possible and ditch the virt-to-phys mode, we *need* a DMA interface flag that tells us when the dma_sync_for_xxx are no-ops. If they aren't we'll refuse to load for now. I'm not sure, but I think also nouveau and radeon suffer from the same issue. RDMA needs something similar as well, in this case drivers take a struct page * from get_user_pages() and need to have the DMA map fail if the platform can't DMA map in a way that does not require any additional DMA API calls to ensure coherence. (think Userspace RDMA MR's) Today we just do the normal DMA map and when it randomly doesn't work and corrupts data tell those people their platforms don't support RDMA - it would be nice to have a safer API base solution.. Oh, yes really good point. We have to support get_user_pages (or HMM) in a similar manner on GPUs as well. Regards, Christian. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Spice-devel] [PATCH 1/3] drm/qxl: add mode/framebuffer check functions
> > +static int qxl_check_mode(struct qxl_device *qdev,
> > + unsigned int width,
> > + unsigned int height)
> > +{
> > + if (width * height * 4 > qdev->vram_size)
>
> Is someone checking for integer overflows already?
Need to have a look. This is just ...
> > - if (bo->surf.stride * bo->surf.height > qdev->vram_size) {
> > - DRM_ERROR("Mode doesn't fit in vram size (vgamem)");
> > - return -EINVAL;
> > - }
... that check moved into the new function.
cheers,
Gerd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 202263] AMDGPU: DRM couldn't schedule ib on ring (-22)
https://bugzilla.kernel.org/show_bug.cgi?id=202263 Emre Işık (e.isi...@gmail.com) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |CODE_FIX -- 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 2/2] drm: plug memory leak on drm_setup() failure
On 2019/01/14, Daniel Vetter wrote:
> On Mon, Jan 14, 2019 at 08:44:10AM +, Emil Velikov wrote:
> > From: Emil Velikov
> >
> > Currently we fail to free and detach the drm_file when drm_setup() fails.
> > Use the drm_close_helper to do address that.
> >
> > Signed-off-by: Emil Velikov
> > ---
> > drivers/gpu/drm/drm_file.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > index 149506a20bdc..871dcd8c7545 100644
> > --- a/drivers/gpu/drm/drm_file.c
> > +++ b/drivers/gpu/drm/drm_file.c
> > @@ -330,8 +330,10 @@ int drm_open(struct inode *inode, struct file *filp)
> > goto err_undo;
> > if (need_setup) {
> > retcode = drm_setup(dev);
> > - if (retcode)
> > + if (retcode) {
> > + drm_close_helper(filp);
>
> I freaked out mildly because drm_open_helper already adds the drm_file to
> the filelist (hence publishes it), and publishing objects before they're
> fully set up is a Bad Idea :-)
>
> But drm_setup only does legacy setup, so who cares. On both patches:
>
> Reviewed-by: Daniel Vetter
>
Thanks.
> And if you feel like doing an s/drm_setup()/drm_legacy_setup()/, with or
> w/o changing the condition to if (need_setup &&
> drm_core_check_feature(dev, DRIVER_LEGACY)), then that patch would also
> have my
>
> Reviewed-by: Daniel Vetter
>
I'll leave that for another day, since the inverse vfunc (firstopen <>
lastclose) is not a legacy only one. Which kind of irks me a bit.
-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm: plug memory leak on drm_setup() failure
On Wed, Jan 16, 2019 at 11:40:41AM +, Emil Velikov wrote:
> On 2019/01/14, Daniel Vetter wrote:
> > On Mon, Jan 14, 2019 at 08:44:10AM +, Emil Velikov wrote:
> > > From: Emil Velikov
> > >
> > > Currently we fail to free and detach the drm_file when drm_setup() fails.
> > > Use the drm_close_helper to do address that.
> > >
> > > Signed-off-by: Emil Velikov
> > > ---
> > > drivers/gpu/drm/drm_file.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > > index 149506a20bdc..871dcd8c7545 100644
> > > --- a/drivers/gpu/drm/drm_file.c
> > > +++ b/drivers/gpu/drm/drm_file.c
> > > @@ -330,8 +330,10 @@ int drm_open(struct inode *inode, struct file *filp)
> > > goto err_undo;
> > > if (need_setup) {
> > > retcode = drm_setup(dev);
> > > - if (retcode)
> > > + if (retcode) {
> > > + drm_close_helper(filp);
> >
> > I freaked out mildly because drm_open_helper already adds the drm_file to
> > the filelist (hence publishes it), and publishing objects before they're
> > fully set up is a Bad Idea :-)
> >
> > But drm_setup only does legacy setup, so who cares. On both patches:
> >
> > Reviewed-by: Daniel Vetter
> >
> Thanks.
>
> > And if you feel like doing an s/drm_setup()/drm_legacy_setup()/, with or
> > w/o changing the condition to if (need_setup &&
> > drm_core_check_feature(dev, DRIVER_LEGACY)), then that patch would also
> > have my
> >
> > Reviewed-by: Daniel Vetter
> >
> I'll leave that for another day, since the inverse vfunc (firstopen <>
> lastclose) is not a legacy only one. Which kind of irks me a bit.
We need lastclose for the fbdev emulation. Which is already fixed through
the new drm_client stuff and the new generic fbdev. It's a matter of
rolling that out. The other use is for delayed vgaswitcheroo changes,
which I think is a bit a hack ... Since I don't have a solution for that I
guess lasclose will stay with us for a while longer.
-Daniel
--
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: [Intel-gfx] [PATCH v5 2/3] drm/i915: Move on the new pm runtime interface
On Monday, January 7, 2019 3:21:49 PM CET Rafael J. Wysocki wrote: > On Mon, Jan 7, 2019 at 3:04 PM Vincent Guittot > wrote: > > > > Hi Tvrtko, > > > > On Mon, 31 Dec 2018 at 13:32, Tvrtko Ursulin > > wrote: > > > > > > > > > On 21/12/2018 13:26, Vincent Guittot wrote: > > > > On Fri, 21 Dec 2018 at 12:33, Tvrtko Ursulin > > > > [snip] > > > > > >> > > > >> If it is always monotonic, then worst case we report one wrong sample, > > > >> which I guess is still not ideal since someone could be querying the > > > >> PMU > > > >> with quite low frequency. > > > >> > > > >> There are tests which probably can hit this, but to run them > > > >> automatically your patches would need to be rebased on drm-tip and > > > >> maybe > > > >> sent to our trybot. I can do that after the holiday break if you are > > > >> okay with having the series waiting until then. > > > > > > > > yes looks good to me > > > > > > Looks good to me as well. And our CI agrees with it. So: > > > > > > Reviewed-by: Tvrtko Ursulin > > > > Thanks for the review and the test > > > > > > > > I assume you will take the patch through some power related tree and we > > > will eventually pull it back to drm-tip. > > > > Rafael, > > How do you want to proceed with this patch and the 2 others of the serie ? > > I'm going to queue up the whole series for 5.1. And it has been queued up now, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] drm: allow render capable master with DRM_AUTH ioctls
On 2019/01/14, Daniel Vetter wrote:
> On Mon, Jan 14, 2019 at 08:54:08AM +, Emil Velikov wrote:
> > From: Emil Velikov
> >
> > There are cases (in mesa and applications) where one would open the
> > primary node without properly authenticating the client.
> >
> > Sometimes we don't check if the authentication succeeds, but there's
> > also cases we simply forget to do it.
> >
> > The former was a case for Mesa where it did not not check the return
> > value of drmGetMagic() [1]. That was fixed recently although, there's
> > the question of older drivers or other apps that exbibit this behaviour.
> >
> > While omitting the call results in issues as seen in [2] and [3].
> >
> > In the libva case, libva itself doesn't authenticate the DRM client and
> > the vaGetDisplayDRM documentation doesn't mention if the app should
> > either.
> >
> > As of today, the official vainfo utility doesn't authenticate.
> >
> > To workaround issues like these, some users resort to running their apps
> > under sudo. Which admittedly isn't always a good idea.
> >
> > Since any DRIVER_RENDER driver has sufficient isolation between clients,
> > we can use that, for unauthenticated [primary node] ioctls that require
> > DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW.
> >
> > v2:
> > - Rework/simplify if check (Daniel V)
> > - Add examples to commit messages, elaborate. (Daniel V)
> >
> > [1]
> > https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136
> > [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html
> > [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1
> > Testcase: igt/core_unauth_vs_render
> > Cc: intel-...@lists.freedesktop.org
> > Signed-off-by: Emil Velikov
> > ---
> > Daniel, the if conditionals did not work exactly as you put them.
> > This is the closest thing that I can think of.
> > ---
> > drivers/gpu/drm/drm_ioctl.c | 20
> > 1 file changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index 94bd872d56c4..08a0b4cc3a76 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -507,6 +507,13 @@ int drm_version(struct drm_device *dev, void *data,
> > return err;
> > }
> >
> > +static inline bool
> > +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags)
> > +{
> > + return drm_core_check_feature(dev, DRIVER_RENDER) &&
> > + (flags & DRM_RENDER_ALLOW);
> > +}
> > +
> > /**
> > * drm_ioctl_permit - Check ioctl permissions against caller
> > *
> > @@ -521,14 +528,19 @@ int drm_version(struct drm_device *dev, void *data,
> > */
> > int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
> > {
> > + const struct drm_device *dev = file_priv->minor->dev;
> > +
> > /* ROOT_ONLY is only for CAP_SYS_ADMIN */
> > if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN)))
> > return -EACCES;
> >
> > - /* AUTH is only for authenticated or render client */
> > - if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) &&
> > -!file_priv->authenticated))
> > - return -EACCES;
> > + /* AUTH is only for master ... */
> > + if ((flags & DRM_AUTH) && drm_is_primary_client(file_priv)) {
> > + /* authenticated ones, or render capable on DRM_RENDER_ALLOW. */
> > + if (unlikely(!file_priv->authenticated) &&
> > + unlikely(!drm_render_driver_and_ioctl(dev, flags)))
>
> The double-unlikely looks a bit strange, I'd move it out so there's only
> one. But this is correct too (because unlikely() && unlikely == unlikely(
> && )). Either way:
>
> Reviewed-by: Daniel Vetter
>
Ack, thanks. To stay consistent with the surrounding code I'll use:
if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) {
/* authenticated ones, or render capable on DRM_RENDER_ALLOW. */
if (!file_priv->authenticated &&
!drm_render_driver_and_ioctl(dev, flags))
Btw, if you can skim through the IGT test that would be appreciated.
Cheers,
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Spice-devel] [PATCH 1/3] drm/qxl: add mode/framebuffer check functions
On Wed, Jan 16, 2019 at 12:28:00PM +0100, Gerd Hoffmann wrote:
> > > +static int qxl_check_mode(struct qxl_device *qdev,
> > > + unsigned int width,
> > > + unsigned int height)
> > > +{
> > > + if (width * height * 4 > qdev->vram_size)
> >
> > Is someone checking for integer overflows already?
>
> Need to have a look. This is just ...
The addfb ioctl checks for integer overflows for you.
-Daniel
>
> > > - if (bo->surf.stride * bo->surf.height > qdev->vram_size) {
> > > - DRM_ERROR("Mode doesn't fit in vram size (vgamem)");
> > > - return -EINVAL;
> > > - }
>
> ... that check moved into the new function.
>
> cheers,
> Gerd
>
--
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
[Bug 108940] QHD bug? drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:1613 core_link_enable_stream+0xc14/0x1040
https://bugs.freedesktop.org/show_bug.cgi?id=108940 H.Habighorst changed: What|Removed |Added CC||h.habigho...@protonmail.com --- Comment #13 from H.Habighorst --- Created attachment 143140 --> https://bugs.freedesktop.org/attachment.cgi?id=143140&action=edit dmesg 4.20.2-gentoo AMD Ryzen 5 2400G dmesg of boot - linux-firmware: 2019-01-14 - kernel: 4.20.2 - AMD Ryzen 5 2400G - X11 driver: modeset - Xorg Log: Modeline "1920x1200"x0.0 154.00 1920 1968 2000 2080 1200 1203 1209 1235 +hsync -vsync (74.0 kHz eP) -- 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
[Bug 108940] QHD bug? drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:1613 core_link_enable_stream+0xc14/0x1040
https://bugs.freedesktop.org/show_bug.cgi?id=108940 --- Comment #14 from H.Habighorst --- Can confirm. But I'm a bit worried - there is a part before the "core_link_enable_stream" that seems wrong to me in initializing the device, which seems to occure in all dmesg outputs. I've attached line numbers via less -N - maybe it helps. 837 [8.686299] [drm] BIOS signature incorrect 0 0 838 [8.686326] ATOM BIOS: 113-RAVEN-110 839 [8.686357] [drm] vm size is 262144 GB, 4 levels, block size is 9-bit, fragment size is 9-bit ... 854 [8.688113] [drm] Found VCN firmware Version: 1.73 Family ID: 18 ... 857 [8.797714] [drm] DM_PPLIB: values for Invalid clock 858 [8.797715] [drm] DM_PPLIB: 0 in kHz 859 [8.797715] [drm] DM_PPLIB: 40 in kHz 860 [8.797716] [drm] DM_PPLIB: 933000 in kHz 861 [8.797716] [drm] DM_PPLIB: 1067000 in kHz 862 [8.797809] WARNING: CPU: 6 PID: 725 at drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:1380 dcn_bw_update_from_pplib+0x16b/0x280 [amdgpu] This seems plain wrong to me - shouldn't it mention here the engine clock? This WARNING stems from not getting the clock values right - seems intended. The same warning occurs again, I wildly guess for the memory (?!). Line 862-1017 - only afterwards the mentioned "core_link_enable_stream" bug occurs. It seems to me - as a total layman - that the initialization fails and afterwards bogus happens. -- 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
Dealing with vblank and
Hi, I am using drm_simple_kms_helper and writing a new driver. The code is here: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git/log/?h=ux500-mcde It's starting to look acceptable, but I just don't wanna post the driver until I get a clean probe. This is using a command mode-only DSI panel, which naturally will not start generating any vblank interrups all by itself: you tell it explicitly to scan out a frame. However I have set it up to scan out frames continously and generate interrupts continously at each completed frame if struct drm_simple_display_pipe_funcs .enable_vblank() is called, and stop it if .disable_vblank() is called. This works mostly: graphics come out on the display and are then continously updated. Some interrupts come out too: 77:569 0 GIC-0 80 Level mcde As expected with framebuffer emulation, there is some vblank use when setting it up, then it turns vblanks off again and just count on the display being continously streamed out. But I get a snag when starting up: mcde a035.mcde: mcde_display_enable_vblank [ cut here ] WARNING: CPU: 0 PID: 1 at ../drivers/gpu/drm/drm_atomic_helper.c:1424 drm_atomic_helper_wait_for_vblanks.part.1+0x29c/0x2a0 [CRTC:34:crtc-0] vblank wait timed out Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.0.0-rc1-5-ga1691ef3e833-dirty #487 Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support) (unwind_backtrace) from [] (show_stack+0x10/0x14) (show_stack) from [] (dump_stack+0x88/0x9c) (dump_stack) from [] (__warn+0xdc/0xf4) (__warn) from [] (warn_slowpath_fmt+0x48/0x6c) (warn_slowpath_fmt) from [] (drm_atomic_helper_wait_for_vblanks.part.1+0x29c/0x2a0) (drm_atomic_helper_wait_for_vblanks.part.1) from [] (drm_atomic_helper_commit_tail+0x5c/0x6c) (drm_atomic_helper_commit_tail) from [] (commit_tail+0x68/0x6c) (commit_tail) from [] (drm_atomic_helper_commit+0xbc/0x128) (drm_atomic_helper_commit) from [] (restore_fbdev_mode_atomic+0x1c8/0x1d8) (restore_fbdev_mode_atomic) from [] (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0) (drm_fb_helper_restore_fbdev_mode_unlocked) from [] (drm_fb_helper_set_par+0x30/0x54) (drm_fb_helper_set_par) from [] (fbcon_init+0x464/0x574) (fbcon_init) from [] (visual_init+0xbc/0x104) (visual_init) from [] (do_bind_con_driver+0x1e0/0x3f4) (do_bind_con_driver) from [] (do_take_over_console+0x84/0x1dc) (do_take_over_console) from [] (do_fbcon_takeover+0x74/0xcc) (do_fbcon_takeover) from [] (notifier_call_chain+0x48/0x84) (notifier_call_chain) from [] (blocking_notifier_call_chain+0x44/0x5c) (blocking_notifier_call_chain) from [] (register_framebuffer+0x228/0x310) (register_framebuffer) from [] (__drm_fb_helper_initial_config_and_unlock+0x234/0x460) (__drm_fb_helper_initial_config_and_unlock) from [] (drm_fb_helper_fbdev_setup+0xb4/0x1e8) (drm_fb_helper_fbdev_setup) from [] (drm_fbdev_cma_init+0x8c/0xbc) (drm_fbdev_cma_init) from [] (drm_fb_cma_fbdev_init+0x8/0x14) (drm_fb_cma_fbdev_init) from [] (mcde_drm_bind+0xec/0x114) (mcde_drm_bind) from [] (try_to_bring_up_master+0x140/0x17c) (try_to_bring_up_master) from [] (component_master_add_with_match+0xc8/0xfc) (component_master_add_with_match) from [] (mcde_probe+0x544/0x58c) (mcde_probe) from [] (platform_drv_probe+0x48/0x98) (platform_drv_probe) from [] (really_probe+0x228/0x2d0) (really_probe) from [] (driver_probe_device+0x60/0x164) (driver_probe_device) from [] (__driver_attach+0xd0/0xd4) (__driver_attach) from [] (bus_for_each_dev+0x74/0xb4) (bus_for_each_dev) from [] (bus_add_driver+0x188/0x20c) (bus_add_driver) from [] (driver_register+0x7c/0x114) (driver_register) from [] (do_one_initcall+0x54/0x194) (do_one_initcall) from [] (kernel_init_freeable+0x148/0x1e4) (kernel_init_freeable) from [] (kernel_init+0x8/0x10c) (kernel_init) from [] (ret_from_fork+0x14/0x2c) I *think* this is because of a catch 22: the FB emulation want to send the first frame out, so it starts the vblank and wait for the first blank before sending the first new fresh framebuffer update by calling .update(). But since this DSI command mode panel is not going to give any IRQs and thus no vblanks before the first frame is sent, it waits in vain. Video mode DSI panels does not have this characteristic and it seems all other panels we handle are video mode. After the timeout and sending the first frame anyways, of course the vblank starts working. So it seems to me that the FB helper does not really deal very well with this kind of displays that do not support vblank interrupts until you send the first frame. The crtcs state no_vblank should according to documentation be used to overcome these situations. At least sometimes. I could not get that to help at all, but it seems unclear whether any of the helper-based drivers actually use that. My driver will deal with arming the vblank event on the first update (drm_crtc_vblank_get(crtc) == 0)) and sending vblank events in response to updates whe
Re: [PATCH v4 2/3] locking: Implement an algorithm choice for Wound-Wait mutexes
So, I guess this is to do w/ the magic of merge commits, but it looks like the hunk changing the crtc_ww_class got lost: ~/src/linux master git show --pretty=short 08295b3b5beec9aac0f7a9db86f0fc3792039da3 drivers/gpu/drm/drm_modeset_lock.c commit 08295b3b5beec9aac0f7a9db86f0fc3792039da3 Author: Thomas Hellstrom locking: Implement an algorithm choice for Wound-Wait mutexes diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index 8a5100685875..638be2eb67b4 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -70,7 +70,7 @@ * lists and lookup data structures. */ -static DEFINE_WW_CLASS(crtc_ww_class); +static DEFINE_WD_CLASS(crtc_ww_class); /** * drm_modeset_lock_all - take all modeset locks ~/src/linux master git log --pretty=format:%H drivers/gpu/drm/drm_modeset_lock.c | grep 08295b3b5beec9aac0f7a9db86f0fc3792039da3 ~/src/linux master 1 BR, -R On Tue, Jun 19, 2018 at 4:29 AM Thomas Hellstrom wrote: > > The current Wound-Wait mutex algorithm is actually not Wound-Wait but > Wait-Die. Implement also Wound-Wait as a per-ww-class choice. Wound-Wait > is, contrary to Wait-Die a preemptive algorithm and is known to generate > fewer backoffs. Testing reveals that this is true if the > number of simultaneous contending transactions is small. > As the number of simultaneous contending threads increases, Wait-Wound > becomes inferior to Wait-Die in terms of elapsed time. > Possibly due to the larger number of held locks of sleeping transactions. > > Update documentation and callers. > > Timings using git://people.freedesktop.org/~thomash/ww_mutex_test > tag patch-18-06-15 > > Each thread runs 10 batches of lock / unlock 800 ww mutexes randomly > chosen out of 10. Four core Intel x86_64: > > Algorithm#threads Rollbacks time > Wound-Wait 4 ~100 ~17s. > Wait-Die 4 ~15~19s. > Wound-Wait 16 ~36~109s. > Wait-Die 16 ~45~82s. > > Cc: Ingo Molnar > Cc: Jonathan Corbet > Cc: Gustavo Padovan > Cc: Maarten Lankhorst > Cc: Sean Paul > Cc: David Airlie > Cc: Davidlohr Bueso > Cc: "Paul E. McKenney" > Cc: Josh Triplett > Cc: Thomas Gleixner > Cc: Kate Stewart > Cc: Philippe Ombredanne > Cc: Greg Kroah-Hartman > Cc: linux-...@vger.kernel.org > Cc: linux-me...@vger.kernel.org > Cc: linaro-mm-...@lists.linaro.org > Co-authored-by: Peter Zijlstra > Signed-off-by: Thomas Hellstrom > > --- > v2: > * Update API according to review comment by Greg Kroah-Hartman. > * Address review comments by Peter Zijlstra: > - Avoid _Bool in composites > - Fix typo > - Use __mutex_owner() where applicable > - Rely on built-in barriers for the main loop exit condition, > struct ww_acquire_ctx::wounded. Update code comments. > - Explain unlocked use of list_empty(). > v3: > * Adapt to and incorporate cleanup by Peter Zijlstra > * Remove unlocked use of list_empty(). > v4: > * Move code related to adding a waiter to the lock waiter list to a > separate function. > --- > Documentation/locking/ww-mutex-design.txt | 57 +-- > drivers/dma-buf/reservation.c | 2 +- > drivers/gpu/drm/drm_modeset_lock.c| 2 +- > include/linux/ww_mutex.h | 17 ++- > kernel/locking/locktorture.c | 2 +- > kernel/locking/mutex.c| 165 > +++--- > kernel/locking/test-ww_mutex.c| 2 +- > lib/locking-selftest.c| 2 +- > 8 files changed, 213 insertions(+), 36 deletions(-) > > diff --git a/Documentation/locking/ww-mutex-design.txt > b/Documentation/locking/ww-mutex-design.txt > index 2fd7f2a2af21..f0ed7c30e695 100644 > --- a/Documentation/locking/ww-mutex-design.txt > +++ b/Documentation/locking/ww-mutex-design.txt > @@ -1,4 +1,4 @@ > -Wait/Wound Deadlock-Proof Mutex Design > +Wound/Wait Deadlock-Proof Mutex Design > == > > Please read mutex-design.txt first, as it applies to wait/wound mutexes too. > @@ -32,10 +32,26 @@ the oldest task) wins, and the one with the higher > reservation id (i.e. the > younger task) unlocks all of the buffers that it has already locked, and then > tries again. > > -In the RDBMS literature this deadlock handling approach is called wait/die: > -The older tasks waits until it can acquire the contended lock. The younger > tasks > -needs to back off and drop all the locks it is currently holding, i.e. the > -younger task dies. > +In the RDBMS literature, a reservation ticket is associated with a > transaction. > +and the deadlock handling approach is called Wait-Die. The name is based on > +the actions of a locking thread when it encounters an already locked mutex. > +If the transaction holding the lock is younger, the locking transaction > waits. > +If the transaction holding the lock is older, the locking transact
[Bug 109135] R9 390 hangs at boot with DPM/DC enabled for kernels 4.19.x and above, says KMS not supported
https://bugs.freedesktop.org/show_bug.cgi?id=109135 --- Comment #20 from i...@yahoo.com --- (In reply to Alex Deucher from comment #12) > (In reply to iive from comment #10) [...] > > Most of the new changes are done before RC1 and it is quite common that > > there are major breakages there, in systems we do not want to bother with. > > These breakages are usually fixed (or reverted) in later Release Candidates. > > This is not always the case; in most cases bisects are pretty smooth. If > you run into unrelated problems with a particular commit, you can always > skip it during the bisect (git bisect skip). Let's say that they merge a change that breaks booting on my motherboard and then they merge the radeon repo. Then they fix booting at rc2. I will have to `git bisect skip` all commits between the boot-breaking merge and rc2, as they all will produce broken kernels. Since all/most suspected radeon commits are in this range, you can't bisect them. One tick to avoid such problem is to do mass revert. Starting from the final release (e.g. 4.19.0) and then reverting all commits in a given subsystem (e.g. amdgpu) up to the previous release. Then doing the bisect in the reverted commits. If there are no interlinked changes, this method mostly works. But even small cosmetic changes or simple API modification outside the subsystem can complicate things. Anyway, bisect was done successfully. I just still kind of don't understand why it landed on that commit. It doesn't look like this is the commit that changes the default amdgpu.dpm method. Also, why not use dpm=1 for DPM and dpm=2 for PowerPlay? -- 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 4/4] drm/atmel-hlcdc: do not immediately disable planes, wait for next frame
On Fri, 11 Jan 2019 14:29:28 +
Peter Rosin wrote:
> On 2019-01-10 20:25, Boris Brezillon wrote:
> > On Thu, 10 Jan 2019 18:51:21 +
> > Peter Rosin wrote:
> >
> >> On 2019-01-10 18:29, Boris Brezillon wrote:
> >>> On Thu, 10 Jan 2019 15:10:48 +
> >>> Peter Rosin wrote:
> >>>
> The A2Q and UPDATE bits have no effect in the channel disable registers.
> However, since they are present, assume that the intention is to disable
> planes, not immediately as indicated by the RST bit, but on the next
> frame shift since that is what A2Q and UPDATE means in the channel enable
> registers.
>
> Disabling the plane on the next frame shift is done with the EN bit,
> so use that.
> >>>
> >>> It's been a long time, but I think I had a good reason for forcing a
> >>> reset. IIRC, when you don't do that and the CRTC is disabled before the
> >>> plane, the EN bit stays around, and next time you queue a plane update,
> >>> you'll start with an invalid buf pointer.
> >>
> >> It might be possible to clear the EN bit in ...CHDR before enabling the
> >> plane in ...CHER. Or is that too late?
> >
> > I think I tried that, but I'm not sure (BTW, this change was done in
> > bd4248bb5e8b ("drm: atmel-hlcdc: reset layer A2Q and UPDATE bits when
>
> That patch is a big fat NOP if you read the documentation. Those bits
> are marked with a '-' for all LCDC channel disable registers, for all
> supported chips IIUC. Are the effects of those bits mentioned in any
> errata?
IIRC, it was not documented in the datasheet, but this came out during
a discussion with the IP designer.
>
> It would be good with a comment that the present undocumented disable
> method is intentional.
Yes, I should have added a comment about that, my bad.
> That would have kept me from assuming the whole
> thing was just copy-paste junk from ..._enable that happened to work.
>
> >> disabling it")). Anyway, I'm not even sure this is still needed now
> >> that atomic updates have a wait_for_flip_done/vblank() in the commit
> >> path.
>
> The documentation for the RST bit states "Resets the layer immediately.
> The frame is aborted." which sounds a bit scary and heavy-handed. But
> again, I don't know what that actually means and what the effects are
> but that was the reason for me wanting to avoid the RST bit.
As I said, I'm not even sure the problem I was trying to fix still
exists.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109370] [Runelite GPU plugin] Enabling GPU plugin produces incorrect rendering
https://bugs.freedesktop.org/show_bug.cgi?id=109370 MIka R changed: What|Removed |Added Severity|minor |normal Priority|low |medium -- 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
[Bug 109375] Regression: dark notebook display (no backlight) after resume
https://bugs.freedesktop.org/show_bug.cgi?id=109375 --- Comment #2 from Leo Li --- Created attachment 143141 --> https://bugs.freedesktop.org/attachment.cgi?id=143141&action=edit drm/amd/display: Detach backlight from stream Hi Rafał, Please give the attached patch a shot. It's in our staging branch, but not in 5.0-rc2 yet. -- 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 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
Hi :-)
On Tue, Jan 15, 2019 at 12:40:16PM -0600, Andrew F. Davis wrote:
> On 1/15/19 12:38 PM, Andrew F. Davis wrote:
> > On 1/15/19 11:45 AM, Liam Mark wrote:
> >> On Tue, 15 Jan 2019, Andrew F. Davis wrote:
> >>
> >>> On 1/14/19 11:13 AM, Liam Mark wrote:
> On Fri, 11 Jan 2019, Andrew F. Davis wrote:
>
> > Buffers may not be mapped from the CPU so skip cache maintenance here.
> > Accesses from the CPU to a cached heap should be bracketed with
> > {begin,end}_cpu_access calls so maintenance should not be needed anyway.
> >
> > Signed-off-by: Andrew F. Davis
> > ---
> > drivers/staging/android/ion/ion.c | 7 ---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/android/ion/ion.c
> > b/drivers/staging/android/ion/ion.c
> > index 14e48f6eb734..09cb5a8e2b09 100644
> > --- a/drivers/staging/android/ion/ion.c
> > +++ b/drivers/staging/android/ion/ion.c
> > @@ -261,8 +261,8 @@ static struct sg_table *ion_map_dma_buf(struct
> > dma_buf_attachment *attachment,
> >
> > table = a->table;
> >
> > - if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
> > - direction))
> > + if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,
> > + direction, DMA_ATTR_SKIP_CPU_SYNC))
>
> Unfortunately I don't think you can do this for a couple reasons.
> You can't rely on {begin,end}_cpu_access calls to do cache maintenance.
> If the calls to {begin,end}_cpu_access were made before the call to
> dma_buf_attach then there won't have been a device attached so the calls
> to {begin,end}_cpu_access won't have done any cache maintenance.
>
> >>>
> >>> That should be okay though, if you have no attachments (or all
> >>> attachments are IO-coherent) then there is no need for cache
> >>> maintenance. Unless you mean a sequence where a non-io-coherent device
> >>> is attached later after data has already been written. Does that
> >>> sequence need supporting?
> >>
> >> Yes, but also I think there are cases where CPU access can happen before
> >> in Android, but I will focus on later for now.
> >>
> >>> DMA-BUF doesn't have to allocate the backing
> >>> memory until map_dma_buf() time, and that should only happen after all
> >>> the devices have attached so it can know where to put the buffer. So we
> >>> shouldn't expect any CPU access to buffers before all the devices are
> >>> attached and mapped, right?
> >>>
> >>
> >> Here is an example where CPU access can happen later in Android.
> >>
> >> Camera device records video -> software post processing -> video device
> >> (who does compression of raw data) and writes to a file
> >>
> >> In this example assume the buffer is cached and the devices are not
> >> IO-coherent (quite common).
> >>
> >
> > This is the start of the problem, having cached mappings of memory that
> > is also being accessed non-coherently is going to cause issues one way
> > or another. On top of the speculative cache fills that have to be
> > constantly fought back against with CMOs like below; some coherent
> > interconnects behave badly when you mix coherent and non-coherent access
> > (snoop filters get messed up).
> >
> > The solution is to either always have the addresses marked non-coherent
> > (like device memory, no-map carveouts), or if you really want to use
> > regular system memory allocated at runtime, then all cached mappings of
> > it need to be dropped, even the kernel logical address (area as painful
> > as that would be).
Ouch :-( I wasn't aware about these potential interconnect issues. How
"real" is that? It seems that we aren't really hitting that today on
real devices.
> >
> >> ION buffer is allocated.
> >>
> >> //Camera device records video
> >> dma_buf_attach
> >> dma_map_attachment (buffer needs to be cleaned)
> >
> > Why does the buffer need to be cleaned here? I just got through reading
> > the thread linked by Laura in the other reply. I do like +Brian's
>
> Actually +Brian this time :)
>
> > suggestion of tracking if the buffer has had CPU access since the last
> > time and only flushing the cache if it has. As unmapped heaps never get
> > CPU mapped this would never be the case for unmapped heaps, it solves my
> > problem.
> >
> >> [camera device writes to buffer]
> >> dma_buf_unmap_attachment (buffer needs to be invalidated)
> >
> > It doesn't know there will be any further CPU access, it could get freed
> > after this for all we know, the invalidate can be saved until the CPU
> > requests access again.
We don't have any API to allow the invalidate to happen on CPU access
if all devices already detached. We need a struct device pointer to
give to the DMA API, otherwise on arm64 there'll be no invalidate.
I had a chat with a few people internally after the previous
discussion with Liam. One suggestion
[PATCH] drm/etnaviv: don't restrict to certain architectures
The Vivante GPU cores are found in many different SoCs and the driver does not depend on anything architecture specific, so just drop the architecture restriction. Signed-off-by: Lucas Stach --- drivers/gpu/drm/etnaviv/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig index 041a77e400d4..21df44b78df3 100644 --- a/drivers/gpu/drm/etnaviv/Kconfig +++ b/drivers/gpu/drm/etnaviv/Kconfig @@ -2,7 +2,6 @@ config DRM_ETNAVIV tristate "ETNAVIV (DRM support for Vivante GPU IP cores)" depends on DRM - depends on ARCH_MXC || ARCH_DOVE || (ARM && COMPILE_TEST) depends on MMU select SHMEM select SYNC_FILE -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/14] staging: android: ion: Allow heap name to be null
Hi Andrew,
On Fri, Jan 11, 2019 at 12:05:20PM -0600, Andrew F. Davis wrote:
> The heap name can be used for debugging but otherwise does not seem
> to be required and no other part of the code will fail if left NULL
> except here. We can make it required and check for it at some point,
> for now lets just prevent this from causing a NULL pointer exception.
I'm not so keen on this one. In the "new" API with heap querying, the
name string is the only way to identify the heap. I think Laura
mentioned at XDC2017 that it was expected that userspace should use
the strings to find the heap they want.
I'd actually be in favor of making the string a more strict UAPI than
allowing it to be empty (at least, if heap name strings is the API we
decide on for identifying heaps - which is another discussion).
Cheers,
-Brian
>
> Signed-off-by: Andrew F. Davis
> ---
> drivers/staging/android/ion/ion.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c
> index bba5f682bc25..14e48f6eb734 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -467,7 +467,7 @@ static int ion_query_heaps(struct ion_heap_query *query)
> max_cnt = query->cnt;
>
> plist_for_each_entry(heap, &dev->heaps, node) {
> - strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
> + strncpy(hdata.name, heap->name ?: "(null)", MAX_HEAP_NAME);
> hdata.name[sizeof(hdata.name) - 1] = '\0';
> hdata.type = heap->type;
> hdata.heap_id = heap->id;
> --
> 2.19.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: NULL vs IS_ERR() buf in etnaviv_core_dump()
Am Montag, den 14.01.2019, 13:49 +0300 schrieb Dan Carpenter:
> The etnaviv_gem_get_pages() never returns NULL. It returns error
> pointers on error.
>
> Fixes: a8c21a5451d8 ("drm/etnaviv: add initial etnaviv DRM driver")
> Signed-off-by: Dan Carpenter
Thanks, applied to etnaviv/next.
> ---
> drivers/gpu/drm/etnaviv/etnaviv_dump.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> index 3fbb4855396c..33854c94cb85 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> @@ -215,7 +215,7 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu)
> mutex_lock(&obj->lock);
> pages = etnaviv_gem_get_pages(obj);
> mutex_unlock(&obj->lock);
> - if (pages) {
> + if (!IS_ERR(pages)) {
> int j;
>
> iter.hdr->data[0] = bomap - bomap_start;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] staging/xgifb: Needs to be converted to a drm driver
On Wed, Jan 16, 2019 at 11:04:40AM +0100, Daniel Vetter wrote: > The fbdev subsystem is closed for new drivers, those need to become > drm ones (which generally results in smaller drivers nowadays, with > the massive amounts of shared infrastructure and helper libraries drm > has). > > Although given the lack of progress since 2010, maybe time to ditch it > from staging outright? > > Signed-off-by: Daniel Vetter > Cc: Arnaud Patard > Cc: Daniel Vetter > Cc: Greg Kroah-Hartman > --- > drivers/staging/xgifb/TODO | 3 +++ > 1 file changed, 3 insertions(+) No one has done anything with this code for over the past year, so I'll just go delete it, after applying this patch, so that if it does get reverted, the TODO item will stay :) thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109375] Regression: dark notebook display (no backlight) after resume
https://bugs.freedesktop.org/show_bug.cgi?id=109375
--- Comment #3 from Rafał Miłecki ---
It didn't apply cleanly on top of 5.0.0-rc2 (a trivial conflict in the
dc_link.c.rej). After fixing that, compiling & testing I can confirm is solves
the problem for me! Thanks!
Before suspend:
> cat /sys/class/backlight/amdgpu_bl0/{actual_brightness,brightness}
65535
255
After resume:
> cat /sys/class/backlight/amdgpu_bl0/{actual_brightness,brightness}
64000
255
As it fixes a regression, can you submit it as a fix for the 5.0 release cycle,
please?
--
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 v5 1/2] drm/sched: Refactor ring mirror list handling.
On 01/16/2019 02:46 AM, Christian König wrote: Am 15.01.19 um 23:01 schrieb Grodzovsky, Andrey: On 01/11/2019 05:03 PM, Andrey Grodzovsky wrote: On 01/11/2019 02:11 PM, Koenig, Christian wrote: Am 11.01.19 um 16:37 schrieb Grodzovsky, Andrey: On 01/11/2019 04:42 AM, Koenig, Christian wrote: Am 10.01.19 um 16:56 schrieb Grodzovsky, Andrey: [SNIP] But we will not be adding the cb back in drm_sched_stop anymore, now we are only going to add back the cb in drm_sched_startr after rerunning those jobs in drm_sched_resubmit_jobs and assign them a new parent there anyway. Yeah, but when we find that we don't need to reset anything anymore then adding the callbacks again won't be possible any more. Christian. I am not sure I understand it, can u point me to example of how this will happen ? I am attaching my latest patches with waiting only for the last job's fence here just so we are on same page regarding the code. Well the whole idea is to prepare all schedulers, then check once more if the offending job hasn't completed in the meantime. If the job completed we need to be able to rollback everything and continue as if nothing had happened. Christian. Oh, but this piece of functionality - skipping HW ASIC reset in case the guilty job done is totally missing form this patch series and so needs to be added. So what you say actually is that for the case were we skip HW asic reset because the guilty job did complete we also need to skip resubmitting the jobs in drm_sched_resubmit_jobs and hence preserve the old parent fence pointer for reuse ? If so I would like to add all this functionality as a third patch since the first 2 patches are more about resolving race condition with jobs in flight while doing reset - what do you think ? Yeah, sounds perfectly fine to me. Christian. I realized there is another complication now for XGMI hive use case, we currently skip gpu recover for adev in case another gpu recover for different adev in same hive is running, under the assumption that we are going to reset all devices in hive anyway because that should cover our own dev too. But if we chose to skip now HW asic reset if our guilty job did finish we will aslo not HW reset any other devices in the hive even if one of them might actually had a bad job, wanted to do gpu recover but skipped it because our recover was in progress in that time. My general idea on that is to keep a list of guilty jobs per hive, when you start gpu recover you first add you guilty job to the hive and trylock hive->reset_lock. Any owner of hive->reset_lock (gpu recovery in progress) once he finished his recovery and released hive->reset_lock should go over hive->bad_jobs_list and if at least one of them is still not signaled (not done) trigger another gpu recovery and so on. If you do manage to trylock you also go over the list, clean it and perform recovery. This list probably needs to be protected with per hive lock. I also think we can for now at least finish reviewing the first 2 patches and submit them since as I said before they are not dealing with this topic and fixing existing race conditions. If you are OK with that I can send for review the last iteration of the first 2 patches where I wait for the last fence in ring mirror list. Andrey I implemented HW reset avoidance including XGMI use case according to the plan i specified. Patch is attached but I can't test it yet due to XGMI regression in PSP which is supposed to be fixed soon. Please take a look. Looks a bit too complicated on first glance. In general we should probably get away from handling a hive in any special way. Yes, I guess i can do it the same way as the generic handling in amdgpu_device_gpu_recover - there is a list of devices to process which is of size 1 for non xgmi use case or more then 1 for XGMI. Multiple timeout jobs in a hive are identical to multiple timeout jobs on different engines on a single device. How about the following handling: 1. Add the timeout job to a list. 2. Try to grab a lock to handle the reset, if that doesn't work because there is already a reset underway return immediately. 3. Stop all schedulers on all affected devices including stopping the timeouts and detaching all callbacks. 4. Double check the list of timed out jobs, if all hw fences of all jobs are completed now we actually don't need to do anything. What if all the jobs on the timed out list did complete but other job (or jobs) for which we removed the time out timer became hanged ? Wouldn't we miss a required reset in this case and wouldn't even have any indication of their hang ? 5. Do the reset on all affected devices. 6. Drop the lock. 7. Add callbacks again and restart the schedulers. I see your steps don't include flushing any tdr in progress from drm_sched_job_finish (or as I did it from amdgpu_job_free_cb) does it mean you don't think we need to flush in order to avoid freeing job while it still might be accessed from time
Re: [PATCH v5 1/2] drm/sched: Refactor ring mirror list handling.
Am 16.01.19 um 16:45 schrieb Grodzovsky, Andrey: On 01/16/2019 02:46 AM, Christian König wrote: Am 15.01.19 um 23:01 schrieb Grodzovsky, Andrey: On 01/11/2019 05:03 PM, Andrey Grodzovsky wrote: On 01/11/2019 02:11 PM, Koenig, Christian wrote: Am 11.01.19 um 16:37 schrieb Grodzovsky, Andrey: On 01/11/2019 04:42 AM, Koenig, Christian wrote: Am 10.01.19 um 16:56 schrieb Grodzovsky, Andrey: [SNIP] But we will not be adding the cb back in drm_sched_stop anymore, now we are only going to add back the cb in drm_sched_startr after rerunning those jobs in drm_sched_resubmit_jobs and assign them a new parent there anyway. Yeah, but when we find that we don't need to reset anything anymore then adding the callbacks again won't be possible any more. Christian. I am not sure I understand it, can u point me to example of how this will happen ? I am attaching my latest patches with waiting only for the last job's fence here just so we are on same page regarding the code. Well the whole idea is to prepare all schedulers, then check once more if the offending job hasn't completed in the meantime. If the job completed we need to be able to rollback everything and continue as if nothing had happened. Christian. Oh, but this piece of functionality - skipping HW ASIC reset in case the guilty job done is totally missing form this patch series and so needs to be added. So what you say actually is that for the case were we skip HW asic reset because the guilty job did complete we also need to skip resubmitting the jobs in drm_sched_resubmit_jobs and hence preserve the old parent fence pointer for reuse ? If so I would like to add all this functionality as a third patch since the first 2 patches are more about resolving race condition with jobs in flight while doing reset - what do you think ? Yeah, sounds perfectly fine to me. Christian. I realized there is another complication now for XGMI hive use case, we currently skip gpu recover for adev in case another gpu recover for different adev in same hive is running, under the assumption that we are going to reset all devices in hive anyway because that should cover our own dev too. But if we chose to skip now HW asic reset if our guilty job did finish we will aslo not HW reset any other devices in the hive even if one of them might actually had a bad job, wanted to do gpu recover but skipped it because our recover was in progress in that time. My general idea on that is to keep a list of guilty jobs per hive, when you start gpu recover you first add you guilty job to the hive and trylock hive->reset_lock. Any owner of hive->reset_lock (gpu recovery in progress) once he finished his recovery and released hive->reset_lock should go over hive->bad_jobs_list and if at least one of them is still not signaled (not done) trigger another gpu recovery and so on. If you do manage to trylock you also go over the list, clean it and perform recovery. This list probably needs to be protected with per hive lock. I also think we can for now at least finish reviewing the first 2 patches and submit them since as I said before they are not dealing with this topic and fixing existing race conditions. If you are OK with that I can send for review the last iteration of the first 2 patches where I wait for the last fence in ring mirror list. Andrey I implemented HW reset avoidance including XGMI use case according to the plan i specified. Patch is attached but I can't test it yet due to XGMI regression in PSP which is supposed to be fixed soon. Please take a look. Looks a bit too complicated on first glance. In general we should probably get away from handling a hive in any special way. Yes, I guess i can do it the same way as the generic handling in amdgpu_device_gpu_recover - there is a list of devices to process which is of size 1 for non xgmi use case or more then 1 for XGMI. Multiple timeout jobs in a hive are identical to multiple timeout jobs on different engines on a single device. How about the following handling: 1. Add the timeout job to a list. 2. Try to grab a lock to handle the reset, if that doesn't work because there is already a reset underway return immediately. 3. Stop all schedulers on all affected devices including stopping the timeouts and detaching all callbacks. 4. Double check the list of timed out jobs, if all hw fences of all jobs are completed now we actually don't need to do anything. What if all the jobs on the timed out list did complete but other job (or jobs) for which we removed the time out timer became hanged ? Wouldn't we miss a required reset in this case and wouldn't even have any indication of their hang ? If the timeout triggers before we disable it we will have the job on the list of jobs which are hanging. If we found that we don't reset and re-enable the timeout it will trigger a bit later and we can do the check again. Thinking about this a bit more we actually don't need to change the handling in any wa
[Bug 202263] AMDGPU: DRM couldn't schedule ib on ring (-22)
https://bugzilla.kernel.org/show_bug.cgi?id=202263 --- Comment #8 from Alex Deucher (alexdeuc...@gmail.com) --- (In reply to Emre Işık from comment #7) > Thanks for the patch. > Do you gonna commit this patch to the official kernel? > Maybe others have the same error like me. Yes, I'll make sure the patches gets upstream and into stable kernels. Thanks for the report and testing. -- 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 00/14] Misc ION cleanups and adding unmapped heap
On 1/15/19 12:58 PM, Laura Abbott wrote: > On 1/15/19 9:47 AM, Andrew F. Davis wrote: >> On 1/14/19 8:39 PM, Laura Abbott wrote: >>> On 1/11/19 10:05 AM, Andrew F. Davis wrote: Hello all, This is a set of (hopefully) non-controversial cleanups for the ION framework and current set of heaps. These were found as I start to familiarize myself with the framework to help in whatever way I can in getting all this up to the standards needed for de-staging. I would like to get some ideas of what is left to work on to get ION out of staging. Has there been some kind of agreement on what ION should eventually end up being? To me it looks like it is being whittled away at to it's most core functions. To me that is looking like being a DMA-BUF user-space front end, simply advertising available memory backings in a system and providing allocations as DMA-BUF handles. If this is the case then it looks close to being ready to me at least, but I would love to hear any other opinions and concerns. >>> >>> Yes, at this point the only functionality that people are really >>> depending on is the ability to allocate a dma_buf easily from userspace. >>> Back to this patchset, the last patch may be a bit different than the others, it adds an unmapped heaps type and creation helper. I wanted to get this in to show off another heap type and maybe some issues we may have with the current ION framework. The unmapped heap is used when the backing memory should not (or cannot) be touched. Currently this kind of heap is used for firewalled secure memory that can be allocated like normal heap memory but only used by secure devices (OP-TEE, crypto HW, etc). It is basically just copied from the "carveout" heap type with the only difference being it is not mappable to userspace and we do not clear the memory (as we should not map it either). So should this really be a new heap type? Or maybe advertised as a carveout heap but with an additional allocation flag? Perhaps we do away with "types" altogether and just have flags, coherent/non-coherent, mapped/unmapped, etc. Maybe more thinking will be needed afterall.. >>> >>> So the cleanup looks okay (I need to finish reviewing) but I'm not a >>> fan of adding another heaptype without solving the problem of adding >>> some sort of devicetree binding or other method of allocating and >>> placing Ion heaps. That plus uncached buffers are one of the big >>> open problems that need to be solved for destaging Ion. See >>> https://lore.kernel.org/lkml/20181120164636.jcw7li2uaa3cmwc3@DESKTOP-E1NTVVP.localdomain/ >>> >>> >>> for some background on that problem. >>> >> >> I'm under the impression that adding heaps like carveouts/chunk will be >> rather system specific and so do not lend themselves well to a universal >> DT style exporter. For instance a carveout memory space can be reported >> by a device at runtime, then the driver managing that device should go >> and use the carveout heap helpers to export that heap. If this is the >> case then I'm not sure it is a problem for the ION core framework to >> solve, but rather the users of it to figure out how best to create the >> various heaps. All Ion needs to do is allow exporting and advertising >> them IMHO. >> > > I think it is a problem for the Ion core framework to take care of. > Ion is useless if you don't actually have the heaps. Nobody has > actually gotten a full Ion solution end-to-end with a carveout heap > working in mainline because any proposals have been rejected. I think > we need at least one example in mainline of how creating a carveout > heap would work. In our evil vendor trees we have several examples. The issue being that Ion is still staging and attempts for generic DT heap definitions haven't seemed to go so well. So for now we just keep it specific to our platforms until upstream makes a direction decision. > >> Thanks for the background thread link, I've been looking for some info >> on current status of all this and "ion" is a bit hard to search the >> lists for. The core reason for posting this cleanup series is to throw >> my hat into the ring of all this Ion work and start getting familiar >> with the pending issues. The last two patches are not all that important >> to get in right now. >> >> In that thread you linked above, it seems we may have arrived at a >> similar problem for different reasons. I think the root issue is the Ion >> core makes too many assumptions about the heap memory. My proposal would >> be to allow the heap exporters more control over the DMA-BUF ops, maybe >> even going as far as letting them provide their own complete struct >> dma_buf_ops. >> >> Let me give an example where I think this is going to be useful. We have >> the classic constraint solving problem on our SoCs. Our SoCs are full of >> various coh
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Wed, Jan 16, 2019 at 07:28:13AM +, Koenig, Christian wrote: > To summarize once more: We have an array of struct pages and want to > coherently map that to a device. And the answer to that is very simple: you can't. What is so hard to understand about? If you want to map arbitrary memory it simply can't be done in a coherent way on about half of our platforms. > If that is not possible because of whatever reason we want to get an > error code or even not load the driver from the beginning. That is a bullshit attitude. Just like everyone else makes their drivers work you should not be lazy. > > bool dma_streaming_is_coherent(const struct device *) > > > > API to help us decide when to load or not. > > Yes, please. Hell no. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Tue, Jan 15, 2019 at 02:25:01PM -0700, Jason Gunthorpe wrote: > RDMA needs something similar as well, in this case drivers take a > struct page * from get_user_pages() and need to have the DMA map fail > if the platform can't DMA map in a way that does not require any > additional DMA API calls to ensure coherence. (think Userspace RDMA > MR's) Any time you dma map pages you need to do further DMA API calls to ensure coherent, that is the way it is implemented. These calls just happen to be no-ops sometimes. > Today we just do the normal DMA map and when it randomly doesn't work > and corrupts data tell those people their platforms don't support RDMA > - it would be nice to have a safer API base solution.. Now that all these drivers are consolidated in rdma-core you can fix the code to actually do the right thing. It isn't that userspace DMA coherent is any harder than in-kernel DMA coherenence. It just is that no one bothered to do it properly. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 202263] AMDGPU: DRM couldn't schedule ib on ring (-22)
https://bugzilla.kernel.org/show_bug.cgi?id=202263 --- Comment #9 from Emre Işık (e.isi...@gmail.com) --- Thank you, too, for writing the patch so quickly. It has been a great pleasure to cooperate with you, Alex Deucher. It's a great thing helping other people's. Which u a nice day! Best regards, Emre Isik -- 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 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On 1/15/19 1:05 PM, Laura Abbott wrote:
> On 1/15/19 10:38 AM, Andrew F. Davis wrote:
>> On 1/15/19 11:45 AM, Liam Mark wrote:
>>> On Tue, 15 Jan 2019, Andrew F. Davis wrote:
>>>
On 1/14/19 11:13 AM, Liam Mark wrote:
> On Fri, 11 Jan 2019, Andrew F. Davis wrote:
>
>> Buffers may not be mapped from the CPU so skip cache maintenance
>> here.
>> Accesses from the CPU to a cached heap should be bracketed with
>> {begin,end}_cpu_access calls so maintenance should not be needed
>> anyway.
>>
>> Signed-off-by: Andrew F. Davis
>> ---
>> drivers/staging/android/ion/ion.c | 7 ---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 14e48f6eb734..09cb5a8e2b09 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -261,8 +261,8 @@ static struct sg_table *ion_map_dma_buf(struct
>> dma_buf_attachment *attachment,
>> table = a->table;
>> - if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
>> - direction))
>> + if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,
>> + direction, DMA_ATTR_SKIP_CPU_SYNC))
>
> Unfortunately I don't think you can do this for a couple reasons.
> You can't rely on {begin,end}_cpu_access calls to do cache
> maintenance.
> If the calls to {begin,end}_cpu_access were made before the call to
> dma_buf_attach then there won't have been a device attached so the
> calls
> to {begin,end}_cpu_access won't have done any cache maintenance.
>
That should be okay though, if you have no attachments (or all
attachments are IO-coherent) then there is no need for cache
maintenance. Unless you mean a sequence where a non-io-coherent device
is attached later after data has already been written. Does that
sequence need supporting?
>>>
>>> Yes, but also I think there are cases where CPU access can happen before
>>> in Android, but I will focus on later for now.
>>>
DMA-BUF doesn't have to allocate the backing
memory until map_dma_buf() time, and that should only happen after all
the devices have attached so it can know where to put the buffer. So we
shouldn't expect any CPU access to buffers before all the devices are
attached and mapped, right?
>>>
>>> Here is an example where CPU access can happen later in Android.
>>>
>>> Camera device records video -> software post processing -> video device
>>> (who does compression of raw data) and writes to a file
>>>
>>> In this example assume the buffer is cached and the devices are not
>>> IO-coherent (quite common).
>>>
>>
>> This is the start of the problem, having cached mappings of memory that
>> is also being accessed non-coherently is going to cause issues one way
>> or another. On top of the speculative cache fills that have to be
>> constantly fought back against with CMOs like below; some coherent
>> interconnects behave badly when you mix coherent and non-coherent access
>> (snoop filters get messed up).
>>
>> The solution is to either always have the addresses marked non-coherent
>> (like device memory, no-map carveouts), or if you really want to use
>> regular system memory allocated at runtime, then all cached mappings of
>> it need to be dropped, even the kernel logical address (area as painful
>> as that would be).
>>
>
> I agree it's broken, hence my desire to remove it :)
>
> The other problem is that uncached buffers are being used for
> performance reason so anything that would involve getting
> rid of the logical address would probably negate any performance
> benefit.
>
I wouldn't go as far as to remove them just yet.. Liam seems pretty
adamant that they have valid uses. I'm just not sure performance is one
of them, maybe in the case of software locks between devices or
something where there needs to be a lot of back and forth interleaved
access on small amounts of data?
>>> ION buffer is allocated.
>>>
>>> //Camera device records video
>>> dma_buf_attach
>>> dma_map_attachment (buffer needs to be cleaned)
>>
>> Why does the buffer need to be cleaned here? I just got through reading
>> the thread linked by Laura in the other reply. I do like +Brian's
>> suggestion of tracking if the buffer has had CPU access since the last
>> time and only flushing the cache if it has. As unmapped heaps never get
>> CPU mapped this would never be the case for unmapped heaps, it solves my
>> problem.
>>
>>> [camera device writes to buffer]
>>> dma_buf_unmap_attachment (buffer needs to be invalidated)
>>
>> It doesn't know there will be any further CPU access, it could get freed
>> after this for all we know, the invalidate can be saved until the CPU
>> requests access again.
>>
>>> dma_buf_detach (device cannot stay attached because it is being sent
>>>
Re: [PATCH 14/14] staging: android: ion: Add UNMAPPED heap type and helper
On 1/15/19 1:11 PM, Laura Abbott wrote:
> On 1/15/19 10:43 AM, Laura Abbott wrote:
>> On 1/15/19 7:58 AM, Andrew F. Davis wrote:
>>> On 1/14/19 8:32 PM, Laura Abbott wrote:
On 1/11/19 10:05 AM, Andrew F. Davis wrote:
> The "unmapped" heap is very similar to the carveout heap except
> the backing memory is presumed to be unmappable by the host, in
> my specific case due to firewalls. This memory can still be
> allocated from and used by devices that do have access to the
> backing memory.
>
> Based originally on the secure/unmapped heap from Linaro for
> the OP-TEE SDP implementation, this was re-written to match
> the carveout heap helper code.
>
> Suggested-by: Etienne Carriere
> Signed-off-by: Andrew F. Davis
> ---
> drivers/staging/android/ion/Kconfig | 10 ++
> drivers/staging/android/ion/Makefile | 1 +
> drivers/staging/android/ion/ion.h | 16 +++
> .../staging/android/ion/ion_unmapped_heap.c | 123
> ++
> drivers/staging/android/uapi/ion.h | 3 +
> 5 files changed, 153 insertions(+)
> create mode 100644 drivers/staging/android/ion/ion_unmapped_heap.c
>
> diff --git a/drivers/staging/android/ion/Kconfig
> b/drivers/staging/android/ion/Kconfig
> index 0fdda6f62953..a117b8b91b14 100644
> --- a/drivers/staging/android/ion/Kconfig
> +++ b/drivers/staging/android/ion/Kconfig
> @@ -42,3 +42,13 @@ config ION_CMA_HEAP
> Choose this option to enable CMA heaps with Ion. This heap is
> backed
> by the Contiguous Memory Allocator (CMA). If your system has
> these
> regions, you should say Y here.
> +
> +config ION_UNMAPPED_HEAP
> + bool "ION unmapped heap support"
> + depends on ION
> + help
> + Choose this option to enable UNMAPPED heaps with Ion. This
> heap is
> + backed in specific memory pools, carveout from the Linux
> memory.
> + Unlike carveout heaps these are assumed to be not mappable by
> + kernel or user-space.
> + Unless you know your system has these regions, you should say N
> here.
> diff --git a/drivers/staging/android/ion/Makefile
> b/drivers/staging/android/ion/Makefile
> index 17f3a7569e3d..c71a1f3de581 100644
> --- a/drivers/staging/android/ion/Makefile
> +++ b/drivers/staging/android/ion/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o
> ion_page_pool.o
> obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o
> obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o
> obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o
> +obj-$(CONFIG_ION_UNMAPPED_HEAP) += ion_unmapped_heap.o
> diff --git a/drivers/staging/android/ion/ion.h
> b/drivers/staging/android/ion/ion.h
> index 97b2876b165a..ce74332018ba 100644
> --- a/drivers/staging/android/ion/ion.h
> +++ b/drivers/staging/android/ion/ion.h
> @@ -341,4 +341,20 @@ static inline struct ion_heap
> *ion_chunk_heap_create(phys_addr_t base, size_t si
> }
> #endif
> +#ifdef CONFIG_ION_UNMAPPED_HEAP
> +/**
> + * ion_unmapped_heap_create
> + * @base: base address of carveout memory
> + * @size: size of carveout memory region
> + *
> + * Creates an unmapped ion_heap using the passed in data
> + */
> +struct ion_heap *ion_unmapped_heap_create(phys_addr_t base, size_t
> size);
> +#else
> +struct ion_heap *ion_unmapped_heap_create(phys_addr_t base, size_t
> size)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +#endif
> +
> #endif /* _ION_H */
> diff --git a/drivers/staging/android/ion/ion_unmapped_heap.c
> b/drivers/staging/android/ion/ion_unmapped_heap.c
> new file mode 100644
> index ..7602b659c2ec
> --- /dev/null
> +++ b/drivers/staging/android/ion/ion_unmapped_heap.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ION Memory Allocator unmapped heap helper
> + *
> + * Copyright (C) 2015-2016 Texas Instruments Incorporated -
> http://www.ti.com/
> + * Andrew F. Davis
> + *
> + * ION "unmapped" heaps are physical memory heaps not by default
> mapped into
> + * a virtual address space. The buffer owner can explicitly request
> kernel
> + * space mappings but the underlying memory may still not be
> accessible for
> + * various reasons, such as firewalls.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include "ion.h"
> +
> +#define ION_UNMAPPED_ALLOCATE_FAIL -1
> +
> +struct ion_unmapped_heap {
> + struct ion_heap heap;
> + struct gen_pool *pool;
> +};
> +
> +static phys_addr_t ion_unmapped_allocate(struct ion_he
Re: [PATCH] etnaviv: allow to build on ARC
On Mon, Jan 14, 2019 at 07:31:57PM +0300, Eugeniy Paltsev wrote: > ARC HSDK SoC has Vivante GPU IP so allow build etnaviv for ARC. > > Signed-off-by: Eugeniy Paltsev > --- > drivers/gpu/drm/etnaviv/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig > index 342591a1084e..49a9957c3373 100644 > --- a/drivers/gpu/drm/etnaviv/Kconfig > +++ b/drivers/gpu/drm/etnaviv/Kconfig > @@ -2,7 +2,7 @@ > config DRM_ETNAVIV > tristate "ETNAVIV (DRM support for Vivante GPU IP cores)" > depends on DRM > - depends on ARCH_MXC || ARCH_DOVE || ARM || COMPILE_TEST > + depends on ARCH_MXC || ARCH_DOVE || ARM || ARC || COMPILE_TEST Is there any reason to not just remove the dependencies entirely? It seems like it could literally build everywhere, and who knows what other SOCs the IP blocks end up in sooner or later? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] etnaviv: allow to build on ARC
Am Mittwoch, den 16.01.2019, 08:21 -0800 schrieb Christoph Hellwig: > On Mon, Jan 14, 2019 at 07:31:57PM +0300, Eugeniy Paltsev wrote: > > ARC HSDK SoC has Vivante GPU IP so allow build etnaviv for ARC. > > > > Signed-off-by: Eugeniy Paltsev > > --- > > drivers/gpu/drm/etnaviv/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/etnaviv/Kconfig > > b/drivers/gpu/drm/etnaviv/Kconfig > > index 342591a1084e..49a9957c3373 100644 > > --- a/drivers/gpu/drm/etnaviv/Kconfig > > +++ b/drivers/gpu/drm/etnaviv/Kconfig > > @@ -2,7 +2,7 @@ > > config DRM_ETNAVIV > > tristate "ETNAVIV (DRM support for Vivante GPU IP cores)" > > depends on DRM > > - depends on ARCH_MXC || ARCH_DOVE || ARM || COMPILE_TEST > > + depends on ARCH_MXC || ARCH_DOVE || ARM || ARC || > > COMPILE_TEST > > Is there any reason to not just remove the dependencies entirely? > It seems like it could literally build everywhere, and who knows what > other SOCs the IP blocks end up in sooner or later? I've just sent out a patch to do exactly this instead of playing whack- a-mole with all the architectures. The patch has been chewed on by the 0-day robot since yesterday and didn't turn up any obvious fallout yet. Regards, Lucas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Wed, 16 Jan 2019 at 16:06, h...@lst.de wrote: > On Wed, Jan 16, 2019 at 07:28:13AM +, Koenig, Christian wrote: > > To summarize once more: We have an array of struct pages and want to > > coherently map that to a device. > > And the answer to that is very simple: you can't. What is so hard > to understand about? If you want to map arbitrary memory it simply > can't be done in a coherent way on about half of our platforms. > > > If that is not possible because of whatever reason we want to get an > > error code or even not load the driver from the beginning. > > That is a bullshit attitude. Just like everyone else makes their > drivers work you should not be lazy. Can you not talk to people like that? Even if you think that is an OK way to treat anyone - which it isn't, certainly not on dri-devel@ with the fd.o Code of Conduct, and not according to the kernel's either - I have absolutely no idea how you can look at the work the AMD people have put in over many years and conclude that they're 'lazy'. If this makes you so angry, step back from the keyboard for a few minutes, and if you still can't participate in reasonable discussion like an adult, maybe step out of the thread entirely. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: don't restrict to certain architectures
On Wed, Jan 16, 2019 at 04:27:58PM +0100, Lucas Stach wrote: > The Vivante GPU cores are found in many different SoCs and the driver > does not depend on anything architecture specific, so just drop the > architecture restriction. > > Signed-off-by: Lucas Stach For good measure I throwed this after my (limited) build setup. Passed build with no warnings on ia64, alpha and x86. So: Acked-by: Sam Ravnborg ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109135] R9 390 hangs at boot with DPM/DC enabled for kernels 4.19.x and above, says KMS not supported
https://bugs.freedesktop.org/show_bug.cgi?id=109135 --- Comment #21 from Alex Deucher --- (In reply to iive from comment #20) > Anyway, bisect was done successfully. > > I just still kind of don't understand why it landed on that commit. > It doesn't look like this is the commit that changes the default amdgpu.dpm > method. The default change may have happened before that change and that change may have broken the old dpm code since presumably dpm=1 was set while bisecting. -- 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 v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd, line option
On 01/07/2019 11:35 AM, Peter Rosin wrote: > On 2019-01-07 10:11, Geert Uytterhoeven wrote: >> Hi Peter, >> >> On Mon, Jan 7, 2019 at 10:03 AM Peter Rosin wrote: >>> On 2019-01-07 09:59, Peter Rosin wrote: On 2019-01-07 09:40, Geert Uytterhoeven wrote: > On Mon, Jan 7, 2019 at 9:26 AM Peter Rosin wrote: >> On 2019-01-06 10:33, Geert Uytterhoeven wrote: >>> On Mon, Nov 26, 2018 at 10:59 PM Peter Rosin wrote: If there are extra logos (CONFIG_FB_LOGO_EXTRA) the heights of these extra logos are not considered when centering the first logo vertically. Signed-off-by: Peter Rosin >>> --- a/drivers/video/logo/Kconfig +++ b/drivers/video/logo/Kconfig @@ -10,6 +10,15 @@ menuconfig LOGO if LOGO +config FB_LOGO_CENTER + bool "Center the logo" + depends on FB=y + help + When this option is selected, the bootup logo is centered both + horizontally and vertically. If more than one logo is displayed + due to multiple CPUs, the collected line of logos is centered + as a whole. + >>> >>> Isn't a kernel command line option more suitable to configure the >>> position >>> of the logo? >> >> That didn't occur to me previously, but it does make sense now that you >> mention it. This is on top of v5.0-rc1, and if applied before v5.0 we >> can avoid possible regressions for folks who might start to rely on >> CONFIG_FB_LOGO_CENTER if v5.0 is released w/o this. >> >> Cheers, >> Peter >> >> From de7353ab519ba9b5c9ea3f62d607bb8e94b687cc Mon Sep 17 00:00:00 2001 >> From: Peter Rosin >> Cc: Bartlomiej Zolnierkiewicz >> Cc: Jonathan Corbet >> Cc: Peter Rosin >> Cc: dri-devel@lists.freedesktop.org >> Cc: linux-fb...@vger.kernel.org >> Cc: linux-...@vger.kernel.org >> To: linux-ker...@vger.kernel.org >> Date: Mon, 7 Jan 2019 08:35:26 +0100 >> Subject: [PATCH] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd >> line >> option >> >> A command line option is much more flexible than a config option and >> the supporting code is small. Gets rid of #ifdefs in the code too... >> >> Suggested-by: Geert Uytterhoeven >> Signed-off-by: Peter Rosin > > Thanks for your patch! > >> --- a/Documentation/fb/fbcon.txt >> +++ b/Documentation/fb/fbcon.txt >> @@ -163,6 +163,12 @@ C. Boot options >> be preserved until there actually is some text is output to the >> console. >> This option causes fbcon to bind immediately to the fbdev device. >> >> +7. fbcon=center-logo >> + >> + When this option is selected, the bootup logo is centered both >> + horizontally and vertically. If more than one logo is displayed >> due to >> + multiple CPUs, the collected line of logos is centered as a >> whole. >> + > > What about making this more generic, than an option specific for > centering? > Else people want fbcon=left-logo or fbcon=right-logo soon? > > fbcon=logo-pos: > > With being "center", "left", "top", "right", "bottom", and > combinations > like "top,center"? Or is that complicating stuff too much? I'm not too keen on implementing all that when I have zero use for it. I can however rename the trigger for the existing fb_center_logo bool to fbcon=logo-pos:center and add a check for "top,left" to reset the bool to false. Then the other variants can be added later by whomever and when needed. Is that good enough? >>> >>> Ouch, I just realized that using a comma is a no-go, as that is already >>> separating fbcon suboptions, so I suggest top-left instead. >> >> Sure, sounds fine to me. I just want to avoid having a parameter system >> that complicates future extension. >> I think you can drop the check for top-left, as the bool defaults to false. > > Right. So, here's an update... > > Again, it would probably be best if this went in before 5.0 is released. > > Changes since v1: > - rename from fbcon=center-logo option to fbcon=logo-pos:center (and adjust > the help text accordingly). > > Cheers, > Peter > > From ebfb2feb7ea4298ac9c222e6ea6f9c9a92452084 Mon Sep 17 00:00:00 2001 > From: Peter Rosin > Cc: Bartlomiej Zolnierkiewicz > Cc: Jonathan Corbet > Cc: Peter Rosin > Cc: dri-devel@lists.freedesktop.org > Cc: linux-fb...@vger.kernel.org > Cc: linux-...@vger.kernel.org > To: linux-ker...@vger.kernel.org > Date: Mon, 7 Jan 2019 08:35:26 +0100 > Subject: [PATCH v2] fbdev: fbmem: convert CONFIG_FB_LOGO_CENTER into a cmd > line option > > A command line option is much more flexible than a config option and > the supporting code is sm
Re: [PATCH v4 2/3] locking: Implement an algorithm choice for Wound-Wait mutexes
Hi, On Wed, 2019-01-16 at 09:24 -0500, Rob Clark wrote: > So, I guess this is to do w/ the magic of merge commits, but it looks > like the hunk changing the crtc_ww_class got lost: So what happened here is that this commit changed it to DEFINE_WD_CLASS and the following commit changed it back again to DEFINE_WW_CLASS so that the algorithm change and only that came with the last commit. Apparently the history of that line got lost when merging since the merge didn't change it; git blame doesn't show it changed, but when looking at the commit history in gitk it's all clear. I guess that's a feature of git. The modeset locks now use true wound-wait rather than wait-die. /Thomas > > ~/src/linux master git show --pretty=short > 08295b3b5beec9aac0f7a9db86f0fc3792039da3 > drivers/gpu/drm/drm_modeset_lock.c > commit 08295b3b5beec9aac0f7a9db86f0fc3792039da3 > Author: Thomas Hellstrom > > locking: Implement an algorithm choice for Wound-Wait mutexes > > diff --git a/drivers/gpu/drm/drm_modeset_lock.c > b/drivers/gpu/drm/drm_modeset_lock.c > index 8a5100685875..638be2eb67b4 100644 > --- a/drivers/gpu/drm/drm_modeset_lock.c > +++ b/drivers/gpu/drm/drm_modeset_lock.c > @@ -70,7 +70,7 @@ > * lists and lookup data structures. > */ > > -static DEFINE_WW_CLASS(crtc_ww_class); > +static DEFINE_WD_CLASS(crtc_ww_class); > > /** > * drm_modeset_lock_all - take all modeset locks > ~/src/linux master git log --pretty=format:%H > drivers/gpu/drm/drm_modeset_lock.c | grep > 08295b3b5beec9aac0f7a9db86f0fc3792039da3 > ~/src/linux master 1 > > > BR, > -R > > > On Tue, Jun 19, 2018 at 4:29 AM Thomas Hellstrom < > thellst...@vmware.com> wrote: > > The current Wound-Wait mutex algorithm is actually not Wound-Wait > > but > > Wait-Die. Implement also Wound-Wait as a per-ww-class choice. > > Wound-Wait > > is, contrary to Wait-Die a preemptive algorithm and is known to > > generate > > fewer backoffs. Testing reveals that this is true if the > > number of simultaneous contending transactions is small. > > As the number of simultaneous contending threads increases, Wait- > > Wound > > becomes inferior to Wait-Die in terms of elapsed time. > > Possibly due to the larger number of held locks of sleeping > > transactions. > > > > Update documentation and callers. > > > > Timings using git://people.freedesktop.org/~thomash/ww_mutex_test > > tag patch-18-06-15 > > > > Each thread runs 10 batches of lock / unlock 800 ww mutexes > > randomly > > chosen out of 10. Four core Intel x86_64: > > > > Algorithm#threads Rollbacks time > > Wound-Wait 4 ~100 ~17s. > > Wait-Die 4 ~15~19s. > > Wound-Wait 16 ~36~109s. > > Wait-Die 16 ~45~82s. > > > > Cc: Ingo Molnar > > Cc: Jonathan Corbet > > Cc: Gustavo Padovan > > Cc: Maarten Lankhorst > > Cc: Sean Paul > > Cc: David Airlie > > Cc: Davidlohr Bueso > > Cc: "Paul E. McKenney" > > Cc: Josh Triplett > > Cc: Thomas Gleixner > > Cc: Kate Stewart > > Cc: Philippe Ombredanne > > Cc: Greg Kroah-Hartman > > Cc: linux-...@vger.kernel.org > > Cc: linux-me...@vger.kernel.org > > Cc: linaro-mm-...@lists.linaro.org > > Co-authored-by: Peter Zijlstra > > Signed-off-by: Thomas Hellstrom > > > > --- > > v2: > > * Update API according to review comment by Greg Kroah-Hartman. > > * Address review comments by Peter Zijlstra: > > - Avoid _Bool in composites > > - Fix typo > > - Use __mutex_owner() where applicable > > - Rely on built-in barriers for the main loop exit condition, > > struct ww_acquire_ctx::wounded. Update code comments. > > - Explain unlocked use of list_empty(). > > v3: > > * Adapt to and incorporate cleanup by Peter Zijlstra > > * Remove unlocked use of list_empty(). > > v4: > > * Move code related to adding a waiter to the lock waiter list to a > > separate function. > > --- > > Documentation/locking/ww-mutex-design.txt | 57 +-- > > drivers/dma-buf/reservation.c | 2 +- > > drivers/gpu/drm/drm_modeset_lock.c| 2 +- > > include/linux/ww_mutex.h | 17 ++- > > kernel/locking/locktorture.c | 2 +- > > kernel/locking/mutex.c| 165 > > +++--- > > kernel/locking/test-ww_mutex.c| 2 +- > > lib/locking-selftest.c| 2 +- > > 8 files changed, 213 insertions(+), 36 deletions(-) > > > > diff --git a/Documentation/locking/ww-mutex-design.txt > > b/Documentation/locking/ww-mutex-design.txt > > index 2fd7f2a2af21..f0ed7c30e695 100644 > > --- a/Documentation/locking/ww-mutex-design.txt > > +++ b/Documentation/locking/ww-mutex-design.txt > > @@ -1,4 +1,4 @@ > > -Wait/Wound Deadlock-Proof Mutex Design > > +Wound/Wait Deadlock-Proof Mutex Design > > == > > > > Please read mutex-design.txt first, as it applies to wait/wound > > mutexes too. > > @
Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On 1/16/19 9:19 AM, Brian Starkey wrote:
> Hi :-)
>
> On Tue, Jan 15, 2019 at 12:40:16PM -0600, Andrew F. Davis wrote:
>> On 1/15/19 12:38 PM, Andrew F. Davis wrote:
>>> On 1/15/19 11:45 AM, Liam Mark wrote:
On Tue, 15 Jan 2019, Andrew F. Davis wrote:
> On 1/14/19 11:13 AM, Liam Mark wrote:
>> On Fri, 11 Jan 2019, Andrew F. Davis wrote:
>>
>>> Buffers may not be mapped from the CPU so skip cache maintenance here.
>>> Accesses from the CPU to a cached heap should be bracketed with
>>> {begin,end}_cpu_access calls so maintenance should not be needed anyway.
>>>
>>> Signed-off-by: Andrew F. Davis
>>> ---
>>> drivers/staging/android/ion/ion.c | 7 ---
>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/staging/android/ion/ion.c
>>> b/drivers/staging/android/ion/ion.c
>>> index 14e48f6eb734..09cb5a8e2b09 100644
>>> --- a/drivers/staging/android/ion/ion.c
>>> +++ b/drivers/staging/android/ion/ion.c
>>> @@ -261,8 +261,8 @@ static struct sg_table *ion_map_dma_buf(struct
>>> dma_buf_attachment *attachment,
>>>
>>> table = a->table;
>>>
>>> - if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
>>> - direction))
>>> + if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,
>>> + direction, DMA_ATTR_SKIP_CPU_SYNC))
>>
>> Unfortunately I don't think you can do this for a couple reasons.
>> You can't rely on {begin,end}_cpu_access calls to do cache maintenance.
>> If the calls to {begin,end}_cpu_access were made before the call to
>> dma_buf_attach then there won't have been a device attached so the calls
>> to {begin,end}_cpu_access won't have done any cache maintenance.
>>
>
> That should be okay though, if you have no attachments (or all
> attachments are IO-coherent) then there is no need for cache
> maintenance. Unless you mean a sequence where a non-io-coherent device
> is attached later after data has already been written. Does that
> sequence need supporting?
Yes, but also I think there are cases where CPU access can happen before
in Android, but I will focus on later for now.
> DMA-BUF doesn't have to allocate the backing
> memory until map_dma_buf() time, and that should only happen after all
> the devices have attached so it can know where to put the buffer. So we
> shouldn't expect any CPU access to buffers before all the devices are
> attached and mapped, right?
>
Here is an example where CPU access can happen later in Android.
Camera device records video -> software post processing -> video device
(who does compression of raw data) and writes to a file
In this example assume the buffer is cached and the devices are not
IO-coherent (quite common).
>>>
>>> This is the start of the problem, having cached mappings of memory that
>>> is also being accessed non-coherently is going to cause issues one way
>>> or another. On top of the speculative cache fills that have to be
>>> constantly fought back against with CMOs like below; some coherent
>>> interconnects behave badly when you mix coherent and non-coherent access
>>> (snoop filters get messed up).
>>>
>>> The solution is to either always have the addresses marked non-coherent
>>> (like device memory, no-map carveouts), or if you really want to use
>>> regular system memory allocated at runtime, then all cached mappings of
>>> it need to be dropped, even the kernel logical address (area as painful
>>> as that would be).
>
> Ouch :-( I wasn't aware about these potential interconnect issues. How
> "real" is that? It seems that we aren't really hitting that today on
> real devices.
>
Sadly there is at least one real device like this now (TI AM654). We
spent some time working with the ARM interconnect spec designers to see
if this was allowed behavior, final conclusion was mixing coherent and
non-coherent accesses is never a good idea.. So we have been working to
try to minimize any cases of mixed attributes [0], if a region is
coherent then everyone in the system needs to treat it as such and
vice-versa, even clever CMO that work on other systems wont save you
here. :(
[0] https://github.com/ARM-software/arm-trusted-firmware/pull/1553
>>>
ION buffer is allocated.
//Camera device records video
dma_buf_attach
dma_map_attachment (buffer needs to be cleaned)
>>>
>>> Why does the buffer need to be cleaned here? I just got through reading
>>> the thread linked by Laura in the other reply. I do like +Brian's
>>
>> Actually +Brian this time :)
>>
>>> suggestion of tracking if the buffer has had CPU access since the last
>>> time and only flushing the cache if it has. As unmapped heaps never get
>>> CPU mapped this would never be the case for unmapped heaps,
Re: [PATCH 11/14] staging: android: ion: Allow heap name to be null
On 1/16/19 9:28 AM, Brian Starkey wrote:
> Hi Andrew,
>
> On Fri, Jan 11, 2019 at 12:05:20PM -0600, Andrew F. Davis wrote:
>> The heap name can be used for debugging but otherwise does not seem
>> to be required and no other part of the code will fail if left NULL
>> except here. We can make it required and check for it at some point,
>> for now lets just prevent this from causing a NULL pointer exception.
>
> I'm not so keen on this one. In the "new" API with heap querying, the
> name string is the only way to identify the heap. I think Laura
> mentioned at XDC2017 that it was expected that userspace should use
> the strings to find the heap they want.
>
Right now the names are only for debug. I accidentally left the name
null once and got a kernel crash. This is the only spot where it is
needed so I fixed it up. The other option is to make the name mandatory
and properly error out, I don't want to do that right now until the
below discussion is had to see if names really do matter or not.
> I'd actually be in favor of making the string a more strict UAPI than
> allowing it to be empty (at least, if heap name strings is the API we
> decide on for identifying heaps - which is another discussion).
>
I would think identifying heaps by name would be less portable, but as
you said that is a whole different discussion..
Thanks,
Andrew
> Cheers,
> -Brian
>
>>
>> Signed-off-by: Andrew F. Davis
>> ---
>> drivers/staging/android/ion/ion.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index bba5f682bc25..14e48f6eb734 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -467,7 +467,7 @@ static int ion_query_heaps(struct ion_heap_query *query)
>> max_cnt = query->cnt;
>>
>> plist_for_each_entry(heap, &dev->heaps, node) {
>> -strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
>> +strncpy(hdata.name, heap->name ?: "(null)", MAX_HEAP_NAME);
>> hdata.name[sizeof(hdata.name) - 1] = '\0';
>> hdata.type = heap->type;
>> hdata.heap_id = heap->id;
>> --
>> 2.19.1
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/2] drm/sched: Refactor ring mirror list handling.
On 01/16/2019 11:02 AM, Koenig, Christian wrote: Am 16.01.19 um 16:45 schrieb Grodzovsky, Andrey: On 01/16/2019 02:46 AM, Christian König wrote: Am 15.01.19 um 23:01 schrieb Grodzovsky, Andrey: On 01/11/2019 05:03 PM, Andrey Grodzovsky wrote: On 01/11/2019 02:11 PM, Koenig, Christian wrote: Am 11.01.19 um 16:37 schrieb Grodzovsky, Andrey: On 01/11/2019 04:42 AM, Koenig, Christian wrote: Am 10.01.19 um 16:56 schrieb Grodzovsky, Andrey: [SNIP] But we will not be adding the cb back in drm_sched_stop anymore, now we are only going to add back the cb in drm_sched_startr after rerunning those jobs in drm_sched_resubmit_jobs and assign them a new parent there anyway. Yeah, but when we find that we don't need to reset anything anymore then adding the callbacks again won't be possible any more. Christian. I am not sure I understand it, can u point me to example of how this will happen ? I am attaching my latest patches with waiting only for the last job's fence here just so we are on same page regarding the code. Well the whole idea is to prepare all schedulers, then check once more if the offending job hasn't completed in the meantime. If the job completed we need to be able to rollback everything and continue as if nothing had happened. Christian. Oh, but this piece of functionality - skipping HW ASIC reset in case the guilty job done is totally missing form this patch series and so needs to be added. So what you say actually is that for the case were we skip HW asic reset because the guilty job did complete we also need to skip resubmitting the jobs in drm_sched_resubmit_jobs and hence preserve the old parent fence pointer for reuse ? If so I would like to add all this functionality as a third patch since the first 2 patches are more about resolving race condition with jobs in flight while doing reset - what do you think ? Yeah, sounds perfectly fine to me. Christian. I realized there is another complication now for XGMI hive use case, we currently skip gpu recover for adev in case another gpu recover for different adev in same hive is running, under the assumption that we are going to reset all devices in hive anyway because that should cover our own dev too. But if we chose to skip now HW asic reset if our guilty job did finish we will aslo not HW reset any other devices in the hive even if one of them might actually had a bad job, wanted to do gpu recover but skipped it because our recover was in progress in that time. My general idea on that is to keep a list of guilty jobs per hive, when you start gpu recover you first add you guilty job to the hive and trylock hive->reset_lock. Any owner of hive->reset_lock (gpu recovery in progress) once he finished his recovery and released hive->reset_lock should go over hive->bad_jobs_list and if at least one of them is still not signaled (not done) trigger another gpu recovery and so on. If you do manage to trylock you also go over the list, clean it and perform recovery. This list probably needs to be protected with per hive lock. I also think we can for now at least finish reviewing the first 2 patches and submit them since as I said before they are not dealing with this topic and fixing existing race conditions. If you are OK with that I can send for review the last iteration of the first 2 patches where I wait for the last fence in ring mirror list. Andrey I implemented HW reset avoidance including XGMI use case according to the plan i specified. Patch is attached but I can't test it yet due to XGMI regression in PSP which is supposed to be fixed soon. Please take a look. Looks a bit too complicated on first glance. In general we should probably get away from handling a hive in any special way. Yes, I guess i can do it the same way as the generic handling in amdgpu_device_gpu_recover - there is a list of devices to process which is of size 1 for non xgmi use case or more then 1 for XGMI. Multiple timeout jobs in a hive are identical to multiple timeout jobs on different engines on a single device. How about the following handling: 1. Add the timeout job to a list. 2. Try to grab a lock to handle the reset, if that doesn't work because there is already a reset underway return immediately. 3. Stop all schedulers on all affected devices including stopping the timeouts and detaching all callbacks. 4. Double check the list of timed out jobs, if all hw fences of all jobs are completed now we actually don't need to do anything. What if all the jobs on the timed out list did complete but other job (or jobs) for which we removed the time out timer became hanged ? Wouldn't we miss a required reset in this case and wouldn't even have any indication of their hang ? If the timeout triggers before we disable it we will have the job on the list of jobs which are hanging. If we found that we don't reset and re-enable the timeout it will trigger a bit later and we can do the check again. Thinking about this a bit more we ac
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Mon, Jan 14, 2019 at 03:16:54PM -0700, Jason Gunthorpe wrote:
> On Sat, Jan 12, 2019 at 01:03:05PM -0600, Shiraz Saleem wrote:
> > On Sat, Jan 12, 2019 at 06:37:58PM +, Jason Gunthorpe wrote:
> > > On Sat, Jan 12, 2019 at 12:27:05PM -0600, Shiraz Saleem wrote:
> > > > On Fri, Jan 04, 2019 at 10:35:43PM +, Jason Gunthorpe wrote:
> > > > > Commit 2db76d7c3c6d ("lib/scatterlist: sg_page_iter: support sg lists
> > > > > w/o
> > > > > backing pages") introduced the sg_page_iter_dma_address() function
> > > > > without
> > > > > providing a way to use it in the general case. If the sg_dma_len is
> > > > > not
> > > > > equal to the dma_length callers cannot safely use the
> > > > > for_each_sg_page/sg_page_iter_dma_address combination.
> > > > >
> > > > > Resolve this API mistake by providing a DMA specific iterator,
> > > > > for_each_sg_dma_page(), that uses the right length so
> > > > > sg_page_iter_dma_address() works as expected with all sglists. A new
> > > > > iterator type is introduced to provide compile-time safety against
> > > > > wrongly
> > > > > mixing accessors and iterators.
> > > > [..]
> > > >
> > > > >
> > > > > +/*
> > > > > + * sg page iterator for DMA addresses
> > > > > + *
> > > > > + * This is the same as sg_page_iter however you can call
> > > > > + * sg_page_iter_dma_address(@dma_iter) to get the page's DMA
> > > > > + * address. sg_page_iter_page() cannot be called on this iterator.
> > > > > + */
> > > > Does it make sense to have a variant of sg_page_iter_page() to get the
> > > > page descriptor with this dma_iter? This can be used when walking
> > > > DMA-mapped
> > > > SG lists with for_each_sg_dma_page.
> > >
> > > I think that would be a complicated cacluation to find the right
> > > offset into the page sg list to get the page pointer back. We can't
> > > just naively use the existing iterator location.
> > >
> > > Probably places that need this are better to run with two iterators,
> > > less computationally expensive.
> > >
> > > Did you find a need for this?
> > >
> >
> > Well I was trying convert the RDMA drivers to use your new iterator variant
> > and saw the need for it in locations where we need virtual address of the
> > pages
> > contained in the SGEs.
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/qplib_res.c
> > b/drivers/infiniband/hw/bnxt_re/qplib_res.c
> > index 59eeac5..7d26903 100644
> > --- a/drivers/infiniband/hw/bnxt_re/qplib_res.c
> > +++ b/drivers/infiniband/hw/bnxt_re/qplib_res.c
> > @@ -85,7 +85,7 @@ static void __free_pbl(struct pci_dev *pdev, struct
> > bnxt_qplib_pbl *pbl,
> > static int __alloc_pbl(struct pci_dev *pdev, struct bnxt_qplib_pbl *pbl,
> >struct scatterlist *sghead, u32 pages, u32 pg_size)
> > {
> > - struct scatterlist *sg;
> > + struct sg_dma_page_iter sg_iter;
> > bool is_umem = false;
> > int i;
> >
> > @@ -116,12 +116,13 @@ static int __alloc_pbl(struct pci_dev *pdev, struct
> > bnxt_qplib_pbl *pbl,
> > } else {
> > i = 0;
> > is_umem = true;
> > - for_each_sg(sghead, sg, pages, i) {
> > - pbl->pg_map_arr[i] = sg_dma_address(sg);
> > - pbl->pg_arr[i] = sg_virt(sg);
> > + for_each_sg_dma_page(sghead, &sg_iter, pages, 0) {
> > + pbl->pg_map_arr[i] =
> > sg_page_iter_dma_address(&sg_iter);
> > + pbl->pg_arr[i] =
> > page_address(sg_page_iter_page(&sg_iter.base)); ???
> > ^
>
> I concur with CH, pg_arr only looks used in the !umem case, so set to
> NULL here. Check with Selvin & Devesh ?
That makes sense to me. Any concerns with that Devesh?
> > @@ -191,16 +190,16 @@ int rxe_mem_init_user(struct rxe_pd *pd, u64 start,
> > goto err1;
> > }
> >
> > - mem->page_shift = umem->page_shift;
> > - mem->page_mask = BIT(umem->page_shift) - 1;
> > + mem->page_shift = PAGE_SHIFT;
> > + mem->page_mask = PAGE_SIZE - 1;
> >
> > num_buf = 0;
> > map = mem->map;
> > if (length > 0) {
> > buf = map[0]->buf;
> >
> > - for_each_sg(umem->sg_head.sgl, sg, umem->nmap, entry) {
> > - vaddr = page_address(sg_page(sg));
> > + for_each_sg_dma_page(umem->sg_head.sgl, &sg_iter,
> > umem->nmap, 0) {
> > + vaddr =
> > page_address(sg_page_iter_page(&sg_iter.base)); ?
> >
>
> rxe doesn't use DMA addreses, so just leave as for_each_sg_page
>
OK.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109135] R9 390 hangs at boot with DPM/DC enabled for kernels 4.19.x and above, says KMS not supported
https://bugs.freedesktop.org/show_bug.cgi?id=109135 --- Comment #22 from i...@yahoo.com --- (In reply to Alex Deucher from comment #21) > (In reply to iive from comment #20) > > Anyway, bisect was done successfully. > > > > I just still kind of don't understand why it landed on that commit. > > It doesn't look like this is the commit that changes the default amdgpu.dpm > > method. > > The default change may have happened before that change and that change may > have broken the old dpm code since presumably dpm=1 was set while bisecting. You haven't look at the commit. It is code refactoring. It doesn't remove, add or modify any functionality. It just changes how some functions are called. (1 function pointer and switch/case, instead of 3 function pointers.) I honestly could not spot what might be wrong with it. -- 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
[PATCH v8] arm64: dts: sdm845: Add gpu and gmu device nodes
Add the nodes to describe the Adreno GPU and GMU devices for sdm845.
Signed-off-by: Jordan Crouse
Reviewed-by: Douglas Anderson
Tested-by: Douglas Anderson
---
This has the following dependencies:
[v11,1/9] dt-bindings: opp: Introduce opp-level bindings
https://patchwork.kernel.org/patch/10755199/
And the following patches already in Rob's msm-next:
msm-next:d1c9cadea6f7 ("drm/msm/gpu: Remove hardcoded interrupt name")
msm-next:b08b92546807 ("drm/msm: drop interrupt-names")
msm-next:24937c540917 ("dt-bindings: drm/msm/a6xx: Document GMU and update GPU
bindings")
And finally drm/msm: Fix A6XX support for opp-level
https://patchwork.freedesktop.org/patch/276756/
Which is not merged because it too depends on Rajendra's stack.
arch/arm64/boot/dts/qcom/sdm845.dtsi | 121 +++
1 file changed, 121 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index f07c4ca..90766fc 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1618,6 +1618,127 @@
};
};
+
+ gpu@500 {
+ compatible = "qcom,adreno-630.2", "qcom,adreno";
+ #stream-id-cells = <16>;
+
+ reg = <0x500 0x4>, <0x509e000 0x10>;
+ reg-names = "kgsl_3d0_reg_memory", "cx_mem";
+
+ /*
+* Look ma, no clocks! The GPU clocks and power are
+* controlled entirely by the GMU
+*/
+
+ interrupts = ;
+
+ iommus = <&adreno_smmu 0>;
+
+ operating-points-v2 = <&gpu_opp_table>;
+
+ qcom,gmu = <&gmu>;
+
+ gpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-71000 {
+ opp-hz = /bits/ 64 <71000>;
+ opp-level =
;
+ };
+
+ opp-67500 {
+ opp-hz = /bits/ 64 <67500>;
+ opp-level =
;
+ };
+
+ opp-59600 {
+ opp-hz = /bits/ 64 <59600>;
+ opp-level =
;
+ };
+
+ opp-52000 {
+ opp-hz = /bits/ 64 <52000>;
+ opp-level = ;
+ };
+
+ opp-41400 {
+ opp-hz = /bits/ 64 <41400>;
+ opp-level =
;
+ };
+
+ opp-34200 {
+ opp-hz = /bits/ 64 <34200>;
+ opp-level = ;
+ };
+
+ opp-25700 {
+ opp-hz = /bits/ 64 <25700>;
+ opp-level =
;
+ };
+ };
+ };
+
+ adreno_smmu: iommu@504 {
+ compatible = "qcom,sdm845-smmu-v2", "qcom,smmu-v2";
+ reg = <0x504 0x1>;
+ #iommu-cells = <1>;
+ #global-interrupts = <2>;
+ interrupts = ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ;
+ clocks = <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
+ <&gcc GCC_GPU_CFG_AHB_CLK>;
+ clock-names = "bus", "iface";
+
+ power-domains = <&gpucc GPU_CX_GDSC>;
+ };
+
+ gmu: gmu@506a000 {
+ compatible="qcom,adreno-gmu-630.2", "qcom,adreno-gmu";
+
+ reg = <0x506a000 0x3>,
+ <0xb28 0x1>,
+ <0xb48 0x1>;
+ reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
+
+ interrupts = ,
+;
+ interrupt-names = "hfi", "gmu";
+
+ clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
+ <&gpucc GP
Re: [PATCH] drm: Split out drm_probe_helper.h
Hi Daniel. > v5: Actually try to sort them, and while at it, sort all the ones I > touch. Applied this variant on top of drm-misc and did a build test. Looked good for ia64, x86 and alpha. Took a closer look at the changes to atmel_hlcd - and they looked OK. But I noticed that atmel_hlcdc uses only drm_kms_helper_poll_init() and drm_kms_helper_poll_fini(). But there are no hits on DRM_CONNECTOR_POLL - so I think we maybe have a driver here where we have plugged the drm_poll infrastructure, but it is not in use. > include/drm/drm_crtc_helper.h | 16 --- The list of include files in this file could be dropped and replaced by: struct drm_connector; struct drm_device; struct drm_display_mode; struct drm_encoder; struct drm_framebuffer; struct drm_mode_set; struct drm_modeset_acquire_ctx; I tried to do so on top of your patch. But there were too many build errros and I somehow lost the motivation. > include/drm/drm_probe_helper.h| 27 +++ This on the other hand is fine - as expected as this is a new file. But the above is just some random comments so: Acked-by: Sam Ravnborg ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 202261] general protection fault: 0000 [#1] PREEMPT SMP Workqueue: events ttm_bo_delayed_workqueue
https://bugzilla.kernel.org/show_bug.cgi?id=202261 --- Comment #5 from Johannes Hirte (johannes.hi...@datenkhaos.de) --- With the change from https://lists.linuxfoundation.org/pipermail/iommu/2019-January/032651.html see bug seems to be fixed. I've not encountered any Oops since testing with this change. -- 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
[Bug 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows
https://bugs.freedesktop.org/show_bug.cgi?id=106175 --- Comment #103 from Hans D --- With the latest xf86 DDX driver 3d works better in amdgpu.dc=0, but still lags, even though a built-in counter is showing high fps. Does amdgpu.dc=1 improve 3d performance that much? -- 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
[PATCH] drm/doc: Make igts for cross-driver stuff mandatory
Compared to the RFC[1] no changes to the patch itself, but igt moved forward a lot: - gitlab CI builds with: reduced configs/libraries, arm cross build and a sysroot build (should address all the build/cross platform concerns raised in the RFC discussions). - tests reorganized into subdirectories so that the i915-gem tests don't clog the main/shared tests directory anymore - quite a few more non-intel people contributing/reviewing/committing igt tests patches. I think this addresses all the concerns raised in the RFC discussions, and assuming there's enough Acks and no new issues that pop up, we can go ahead with this. 1: https://patchwork.kernel.org/patch/10648851/ Cc: Petri Latvala Cc: Arkadiusz Hiler Cc: Liviu Dudau Cc: Sean Paul Cc: Eric Anholt Cc: Alex Deucher Cc: Dave Airlie Signed-off-by: Daniel Vetter --- Documentation/gpu/drm-uapi.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index a752aa561ea4..413915d6b7d2 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has the slightly unintuitive meaning of Testing and validation == +Testing Requirements for userspace API +-- + +New cross-driver userspace interface extensions, like new IOCTL, new KMS +properties, new files in sysfs or anything else that constitutes an API change +need to have driver-agnostic testcases in IGT for that feature. + Validating changes with IGT --- -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Expose more EDID fields to userspace
On Mon, 7 Jan 2019 17:07:09 + Brian Starkey wrote: > On Mon, Jan 07, 2019 at 07:57:54AM -0800, Keith Packard wrote: > > Daniel Vetter writes: > > > > > Best to pull in some other compositor people and get them to agree. From a > > > kernel pov I'm fine with whatever userspace preferes. > > > > Hrm. It would be good to have everyone using the same interpretation of > > EDID data; in particular, where the kernel has quirks that change the > > interpretation, user space should be consistent with that. > > > > Unless we expose all of the EDID data, then user space may still have to > > parse EDID. If the kernel has EDID quirks, it might be good to to make > > those affect the "raw" EDID data visible to use space so that values the > > kernel supplies separately are consistent with values extracted from the > > "raw" EDID data. > > If the quirks can be re-encoded back into an EDID representation, then > this sounds like a fairly good approach to me. > > > > > Doing this in the kernel does make it harder to quickly supply fixes for > > a specific user space application. This will probably lead to > > kludge-arounds in user space that could depend on kernel > > version. Perhaps these EDID capabilities in the kernel should be > > versioned separately? > > > > I see good benefits from having user space able to see how the kernel is > > interpreting EDID so that it can adapt as appropriate, but we should be > > cautious about moving functionality into the kernel that would be more > > easily maintained up in user space. > > > > I agree. It seems likely that whatever happens (some) userspace is > still going to implement (some) EDID parsing functionality, so it's > hard to reason about what belongs where. Shared code in userspace > (libdrm?) may well be better than exposing it from the kernel. > > If it is exposed by the kernel, then it's still non-obvious to me > how the kernel exposes that information/interpretation. Adding > a property for every potentially-useful field really doesn't scale > well, and what fields are useful isn't obvious - e.g. serial string vs > serial no., as mentioned by Simon. > > Uma's recent series: "Add HDR Metadata Parsing and handling in DRM > layer"[1] is a good example of more stuff which userspace would want to > parse out of the EDID (supported display colorimetry and transfer > functions). > > It would be nice to avoid duplicating all the CEA extension parsing > code, but the EDID/CEA data structure is extensible by design. So the > kernel API would need to be similarly extensible, or we'll just > balloon loads of properties... and then the kernel API would likely > just end up just looking similar to the CEA block anyway. > > Cheers, > -Brian > > [1] https://lists.freedesktop.org/archives/dri-devel/2018-December/200154.html I would agree with an effort to establish a userspace EDID parsing library in any case. As mentioned above, there will probably be too much to expose via kernel UABI, or it will become just another encoded format that again should have a shared parser library in userspace. Would it be possible to architect the library so that it would be shared with the kernel? Maybe the quirks database could be shared with the kernel as well? That way both kernel and userspace would more or less agree on the parsing details. I've been dreaming of a "liboutput" that would e.g. parse EDID, generate CVT video mode timings, and whatnot that all display servers more or less will duplicate. Once upon a time Ajax started minitru IIRC... Another thing for "liboutput" was a device description database, whose first use would have been the "non-desktop" property. Because we don't expose monitors through udev to have udev rules tag them with interesting bits. Imagine this: monitors exposed as devices via udev, tagged with helpers as regular monitor, HMD, TV, projector, ... and all EDID fields decoded as well. And quirks in hwdb. But I suppose that won't happen, because a "monitor device node" would have no other use. Except... programmatical monitor controls? Like backlight, brightness, contrast, and so on. Ah, nevermind. :-) Thanks, pq pgpEMyokRsivN.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Armada DRM: bridge with componentized devices
On Tue, Jan 15, 2019 at 11:47 PM Rafael J. Wysocki wrote: > > [CC Greg] > > On Tuesday, January 15, 2019 1:04:02 AM CET Rafael J. Wysocki wrote: > > [CC Lukas and linux-pm] > > > > On Mon, Jan 14, 2019 at 1:32 PM Rafael J. Wysocki wrote: > > > > > > On Fri, Jan 11, 2019 at 3:49 PM Russell King - ARM Linux > > > wrote: > > > > [cut] > > > > > > > > > > This thread is discussing how to deal with Armada DRM, its use of the > > > > component helper, TDA998x's hybrid use of the component helper and > > > > DRM bridges. > > > > > > > > Currently, DRM bridges register into the DRM system and are added to > > > > a global list of bridges. When a DRM driver binds, it looks up the > > > > DRM bridge and attaches to it. There is no way in generic DRM code > > > > for the DRM driver to know if the DRM bridge is unbound from DRM, > > > > consequently a DRM driver may continue trying to call functions in > > > > the DRM bridge driver using memory that has already been freed. > > > > > > > > We had similar issues with imx-drm, and a number of years ago at a > > > > kernel summit, this was discussed, and I proposed a system which is > > > > now known as the component helper. This handles the problem of a > > > > multi-component system. > > > > > > > > However, DRM bridge was already established, and there appears to be > > > > no desire to convert DRM bridges and DRM drivers to use the component > > > > helper. > > > > > > > > We are presently in the situation where Armada DRM is incompatible > > > > with the DRM bridges way of doing things, and making it compatible > > > > essentially means introducing potential use-after-free bugs into the > > > > code. > > > > > > > > Device links in their stateful form appear to provide an alternative > > > > to the component helper, in that a stateful device link will remove > > > > consumers of a resource when the supplier is going away - which is > > > > exactly the problem which the component helper is solving. The > > > > difference is that device links look like being a cleaner solution. > > > > > > > > Just like the component helper, a stateful link would unbind the > > > > consumer of a resource when the supplier goes away - which is exactly > > > > the behaviour we're wanting. > > > > > > > > The problem is that the connection between various drivers is only > > > > really known when the drivers obtain their resources, and the > > > > following can happen: > > > > > > > > supplierconsumer > > > > > > > > probe() > > > > alloc > > > > probe() > > > > publish > > > > obtain supplier's resource > > > > return > > > > > > > > Where things go wrong is if a stateful link is created when the > > > > consumer obtains the suppliers resource before the supplier has > > > > finished probing - which from what's been said is illegal. > > > > > > It just doesn't work (which means "invalid" rather than "illegal" I > > > guess, but whatever :-)). > > > > > > Admittedly, the original design didn't take this particular case into > > > account and I'm not actually sure how it can be taken into account > > > either. > > > > > > If the link is created by the consumer in the scenario above, its > > > status will be updated twice in a row after the consumer probe returns > > > and after the supplier probe returns. It looks like this update would > > > need to work regardless of the order it which this happened which > > > sounds somewhat challenging. I would need to think about that. > > > > So if I'm not mistaken it can be made work with the help of the > > (completely untested) attached patch (of course, the documentation > > would need to be updated too). > > > > The key observation here is that it should be fine to create a link > > from the consumer driver's probe while the supplier is still probing > > if the consumer has some way to find out that the supplier is > > functional at that point (like when it has published itself already in > > your example above). The initial state of the link can be "consumer > > probe" in that case and I don't see a reason why that might not work. > > > > Below is a more complete patch that should take all of the corner cases > into account unless I have missed anything. Testing it would be > appreciated. :-) > > --- > From: Rafael J. Wysocki > Subject: [PATCH] driver core: Fix adding device links to probing suppliers > > Currently, it is not valid to add a device link from a consumer > driver ->probe() callback to a supplier that is still probing too, but > generally this is a valid use case. For example, if the consumer has > just acquired a resource that can only be available when the supplier > is functional, adding a device link to that supplier right away > should be safe (and even desirable arguably), but device_link_add() > doesn't handle that case correctly and the initial state of the link > created by it is wrong then. > > To address this problem, change the initial stat
[PATCH v2 2/2] drm/msm: Cleanup A6XX opp-level reading
The patch ("OPP: Add support for parsing the 'opp-level' property")
adds an API enabling a cleaner way to read the opp-level. Let's use
the new API.
Signed-off-by: Douglas Anderson
Reviewed-by: Jordan Crouse
---
Obviously this can't land until we have a tree that contains the patch
adding the API. I believe that means we'll want to target this patch
for 5.2. Luckily it's fine to wait since this patch has no functional
changes--it's all cleanup.
Changes in v2:
- Split into two patches to facilitate landing.
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index ce1b3cc4bf6d..900f18dc1577 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -928,25 +928,20 @@ static int a6xx_gmu_memory_probe(struct a6xx_gmu *gmu)
}
/* Return the 'arc-level' for the given frequency */
-static u32 a6xx_gmu_get_arc_level(struct device *dev, unsigned long freq)
+static unsigned int a6xx_gmu_get_arc_level(struct device *dev,
+ unsigned long freq)
{
struct dev_pm_opp *opp;
- struct device_node *np;
- u32 val = 0;
+ unsigned int val;
if (!freq)
return 0;
- opp = dev_pm_opp_find_freq_exact(dev, freq, true);
+ opp = dev_pm_opp_find_freq_exact(dev, freq, true);
if (IS_ERR(opp))
return 0;
- np = dev_pm_opp_get_of_node(opp);
-
- if (np) {
- of_property_read_u32(np, "opp-level", &val);
- of_node_put(np);
- }
+ val = dev_pm_opp_get_level(opp);
dev_pm_opp_put(opp);
@@ -982,7 +977,7 @@ static int a6xx_gmu_rpmh_arc_votes_init(struct device *dev,
u32 *votes,
/* Construct a vote for each frequency */
for (i = 0; i < freqs_count; i++) {
u8 pindex = 0, sindex = 0;
- u32 level = a6xx_gmu_get_arc_level(dev, freqs[i]);
+ unsigned int level = a6xx_gmu_get_arc_level(dev, freqs[i]);
/* Get the primary index that matches the arc level */
for (j = 0; j < pri_count; j++) {
--
2.20.1.97.g81188d93c3-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/msm: Fix A6XX support for opp-level
The bindings for Qualcomm opp levels changed after being Acked but
before landing. Thus the code in the GPU driver that was relying on
the old bindings is now broken.
Let's change the code to match the new bindings by adjusting the old
string 'qcom,level' to the new string 'opp-level'. See the patch
("dt-bindings: opp: Introduce opp-level bindings").
NOTE: we will do additional cleanup to totally remove the string from
the code and use the new dev_pm_opp_get_level() but we'll do it in a
future patch. This will facilitate getting the important code fix in
sooner without having to deal with cross-maintainer dependencies.
This patch needs to land before the patch ("arm64: dts: sdm845: Add
gpu and gmu device nodes") since if a tree contains the device tree
patch but not this one you'll get a crash at bootup.
Fixes: 4b565ca5a2cb ("drm/msm: Add A6XX device support")
Signed-off-by: Douglas Anderson
---
Changes in v2:
- Split into two patches to facilitate landing.
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 5beb83d1cf87..ce1b3cc4bf6d 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -944,7 +944,7 @@ static u32 a6xx_gmu_get_arc_level(struct device *dev,
unsigned long freq)
np = dev_pm_opp_get_of_node(opp);
if (np) {
- of_property_read_u32(np, "qcom,level", &val);
+ of_property_read_u32(np, "opp-level", &val);
of_node_put(np);
}
--
2.20.1.97.g81188d93c3-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109135] R9 390 hangs at boot with DPM/DC enabled for kernels 4.19.x and above, says KMS not supported
https://bugs.freedesktop.org/show_bug.cgi?id=109135
--- Comment #23 from Alex Deucher ---
(In reply to iive from comment #22)
>
> It is code refactoring. It doesn't remove, add or modify any functionality.
> It just changes how some functions are called. (1 function pointer and
> switch/case, instead of 3 function pointers.)
> I honestly could not spot what might be wrong with it.
this changed:
@@ -1751,10 +1751,10 @@ void amdgpu_dpm_enable_uvd(struct amdgpu_device *adev,
bool enable)
void amdgpu_dpm_enable_vce(struct amdgpu_device *adev, bool enable)
{
- if (adev->powerplay.pp_funcs->powergate_vce) {
+ if (adev->powerplay.pp_funcs->set_powergating_by_smu) {
CI asic never had a powergate_vce callback before, so that code was never
called for vce previously, at least for the old dpm implementation. For the
new on, it actually had a callback for vce powergating, but perhaps there was a
bug in that code around the time this code was changed.
--
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 1/1] doc: bindings: fix bad reference to ARM CPU bindings
On Tue, Jan 8, 2019 at 5:47 PM Otto Sabart wrote:
>
> The primecell.txt and cpus.txt files were converted into YAML. This
> patch updates old references with new ones.
>
> Fixes: d3c207eeb905 ("dt-bindings: arm: Convert primecell binding to
> json-schema")
> Fixes: 672951cbd1b7 ("dt-bindings: arm: Convert cpu binding to json-schema")
> Signed-off-by: Otto Sabart
> ---
> Documentation/devicetree/bindings/arm/cpu-capacity.txt | 2 +-
> Documentation/devicetree/bindings/arm/idle-states.txt | 2 +-
> Documentation/devicetree/bindings/arm/sp810.txt | 2 +-
> Documentation/devicetree/bindings/arm/topology.txt | 2 +-
> Documentation/devicetree/bindings/display/arm,pl11x.txt | 2 +-
> .../devicetree/bindings/interrupt-controller/arm,gic-v3.txt | 2 +-
> 6 files changed, 6 insertions(+), 6 deletions(-)
Applied, thanks.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Expose more EDID fields to userspace
On Wed, Jan 16, 2019 at 08:35:26PM +0200, Pekka Paalanen wrote: > On Mon, 7 Jan 2019 17:07:09 + > Brian Starkey wrote: > > > On Mon, Jan 07, 2019 at 07:57:54AM -0800, Keith Packard wrote: > > > Daniel Vetter writes: > > > > > > > Best to pull in some other compositor people and get them to agree. > > > > From a > > > > kernel pov I'm fine with whatever userspace preferes. > > > > > > Hrm. It would be good to have everyone using the same interpretation of > > > EDID data; in particular, where the kernel has quirks that change the > > > interpretation, user space should be consistent with that. > > > > > > Unless we expose all of the EDID data, then user space may still have to > > > parse EDID. If the kernel has EDID quirks, it might be good to to make > > > those affect the "raw" EDID data visible to use space so that values the > > > kernel supplies separately are consistent with values extracted from the > > > "raw" EDID data. > > > > If the quirks can be re-encoded back into an EDID representation, then > > this sounds like a fairly good approach to me. > > > > > > > > Doing this in the kernel does make it harder to quickly supply fixes for > > > a specific user space application. This will probably lead to > > > kludge-arounds in user space that could depend on kernel > > > version. Perhaps these EDID capabilities in the kernel should be > > > versioned separately? > > > > > > I see good benefits from having user space able to see how the kernel is > > > interpreting EDID so that it can adapt as appropriate, but we should be > > > cautious about moving functionality into the kernel that would be more > > > easily maintained up in user space. > > > > > > > I agree. It seems likely that whatever happens (some) userspace is > > still going to implement (some) EDID parsing functionality, so it's > > hard to reason about what belongs where. Shared code in userspace > > (libdrm?) may well be better than exposing it from the kernel. > > > > If it is exposed by the kernel, then it's still non-obvious to me > > how the kernel exposes that information/interpretation. Adding > > a property for every potentially-useful field really doesn't scale > > well, and what fields are useful isn't obvious - e.g. serial string vs > > serial no., as mentioned by Simon. > > > > Uma's recent series: "Add HDR Metadata Parsing and handling in DRM > > layer"[1] is a good example of more stuff which userspace would want to > > parse out of the EDID (supported display colorimetry and transfer > > functions). > > > > It would be nice to avoid duplicating all the CEA extension parsing > > code, but the EDID/CEA data structure is extensible by design. So the > > kernel API would need to be similarly extensible, or we'll just > > balloon loads of properties... and then the kernel API would likely > > just end up just looking similar to the CEA block anyway. > > > > Cheers, > > -Brian > > > > [1] > > https://lists.freedesktop.org/archives/dri-devel/2018-December/200154.html > > I would agree with an effort to establish a userspace EDID parsing > library in any case. As mentioned above, there will probably be too > much to expose via kernel UABI, or it will become just another > encoded format that again should have a shared parser library in > userspace. > > Would it be possible to architect the library so that it would be > shared with the kernel? Maybe the quirks database could be shared > with the kernel as well? That way both kernel and userspace would > more or less agree on the parsing details. IIRC long ago I did manage to build drm_edid.c in userspace. My idea was to potentially fuzz it without oopsing the kernel. But I didn't get beyond just making it build before getting distracted by something shinier. Taking that further would involve pulling the edid parser into a userspace helper entirely. Unfortunately that would open up the big can of "how do we parse the edid during boot?" worms. But I do kinda like the idea of not having the kernel parse untrusted input directly. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108711] GPU crash in some opengl 1.x applications on vega gpu
https://bugs.freedesktop.org/show_bug.cgi?id=108711 --- Comment #1 from mittorn --- Re-uploaded traces here http://mittorn.the-swank.pp.ua/xash64.1.trace.xz http://mittorn.the-swank.pp.ua/xash64.trace.xz -- 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 v8] arm64: dts: sdm845: Add gpu and gmu device nodes
Hi,
On Wed, Jan 16, 2019 at 10:03 AM Jordan Crouse wrote:
>
> Add the nodes to describe the Adreno GPU and GMU devices for sdm845.
>
> Signed-off-by: Jordan Crouse
> Reviewed-by: Douglas Anderson
> Tested-by: Douglas Anderson
> ---
> This has the following dependencies:
>
> [v11,1/9] dt-bindings: opp: Introduce opp-level bindings
> https://patchwork.kernel.org/patch/10755199/
>
> And the following patches already in Rob's msm-next:
> msm-next:d1c9cadea6f7 ("drm/msm/gpu: Remove hardcoded interrupt name")
> msm-next:b08b92546807 ("drm/msm: drop interrupt-names")
> msm-next:24937c540917 ("dt-bindings: drm/msm/a6xx: Document GMU and update
> GPU bindings")
>
> And finally drm/msm: Fix A6XX support for opp-level
> https://patchwork.freedesktop.org/patch/276756/
>
> Which is not merged because it too depends on Rajendra's stack.
Newer version:
https://lore.kernel.org/patchwork/patch/1032724/
...that one should be able to land earlier since it's simpler.
---
Don't forget that your patch also depends on:
https://lkml.kernel.org/r/20181128185743.75328-2-diand...@chromium.org
...because you won't compile unless gpucc is there.
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 121
> +++
> 1 file changed, 121 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index f07c4ca..90766fc 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -1618,6 +1618,127 @@
> };
> };
>
> +
> + gpu@500 {
Heads up to Andy that when this lands it should be re-sorted to be
right before gpucc so that the unit addresses are sorted. :-) ...and
while doing that we can make sure there's not an extra blank line.
-Doug
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Dealing with vblank and
On Wed, Jan 16, 2019 at 3:06 PM Linus Walleij wrote: > > Hi, I am using drm_simple_kms_helper and writing a new driver. > > The code is here: > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git/log/?h=ux500-mcde > It's starting to look acceptable, but I just don't wanna post the driver > until I get a clean probe. > > This is using a command mode-only DSI panel, which naturally > will not start generating any vblank interrups all by itself: you > tell it explicitly to scan out a frame. > > However I have set it up to scan out frames continously and > generate interrupts continously at each completed frame if > struct drm_simple_display_pipe_funcs > .enable_vblank() is called, and stop it if .disable_vblank() > is called. > > This works mostly: graphics come out on the display and > are then continously updated. Some interrupts come out > too: > > 77:569 0 GIC-0 80 Level mcde > > As expected with framebuffer emulation, there is some vblank > use when setting it up, then it turns vblanks off again and > just count on the display being continously streamed out. > > But I get a snag when starting up: > > mcde a035.mcde: mcde_display_enable_vblank > [ cut here ] > WARNING: CPU: 0 PID: 1 at ../drivers/gpu/drm/drm_atomic_helper.c:1424 > drm_atomic_helper_wait_for_vblanks.part.1+0x29c/0x2a0 > [CRTC:34:crtc-0] vblank wait timed out > Modules linked in: > CPU: 0 PID: 1 Comm: swapper/0 Not tainted > 5.0.0-rc1-5-ga1691ef3e833-dirty #487 > Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support) > (unwind_backtrace) from [] (show_stack+0x10/0x14) > (show_stack) from [] (dump_stack+0x88/0x9c) > (dump_stack) from [] (__warn+0xdc/0xf4) > (__warn) from [] (warn_slowpath_fmt+0x48/0x6c) > (warn_slowpath_fmt) from [] > (drm_atomic_helper_wait_for_vblanks.part.1+0x29c/0x2a0) > (drm_atomic_helper_wait_for_vblanks.part.1) from [] > (drm_atomic_helper_commit_tail+0x5c/0x6c) > (drm_atomic_helper_commit_tail) from [] (commit_tail+0x68/0x6c) > (commit_tail) from [] (drm_atomic_helper_commit+0xbc/0x128) > (drm_atomic_helper_commit) from [] > (restore_fbdev_mode_atomic+0x1c8/0x1d8) > (restore_fbdev_mode_atomic) from [] > (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0) > (drm_fb_helper_restore_fbdev_mode_unlocked) from [] > (drm_fb_helper_set_par+0x30/0x54) > (drm_fb_helper_set_par) from [] (fbcon_init+0x464/0x574) > (fbcon_init) from [] (visual_init+0xbc/0x104) > (visual_init) from [] (do_bind_con_driver+0x1e0/0x3f4) > (do_bind_con_driver) from [] (do_take_over_console+0x84/0x1dc) > (do_take_over_console) from [] (do_fbcon_takeover+0x74/0xcc) > (do_fbcon_takeover) from [] (notifier_call_chain+0x48/0x84) > (notifier_call_chain) from [] > (blocking_notifier_call_chain+0x44/0x5c) > (blocking_notifier_call_chain) from [] > (register_framebuffer+0x228/0x310) > (register_framebuffer) from [] > (__drm_fb_helper_initial_config_and_unlock+0x234/0x460) > (__drm_fb_helper_initial_config_and_unlock) from [] > (drm_fb_helper_fbdev_setup+0xb4/0x1e8) > (drm_fb_helper_fbdev_setup) from [] (drm_fbdev_cma_init+0x8c/0xbc) > (drm_fbdev_cma_init) from [] (drm_fb_cma_fbdev_init+0x8/0x14) > (drm_fb_cma_fbdev_init) from [] (mcde_drm_bind+0xec/0x114) > (mcde_drm_bind) from [] (try_to_bring_up_master+0x140/0x17c) > (try_to_bring_up_master) from [] > (component_master_add_with_match+0xc8/0xfc) > (component_master_add_with_match) from [] (mcde_probe+0x544/0x58c) > (mcde_probe) from [] (platform_drv_probe+0x48/0x98) > (platform_drv_probe) from [] (really_probe+0x228/0x2d0) > (really_probe) from [] (driver_probe_device+0x60/0x164) > (driver_probe_device) from [] (__driver_attach+0xd0/0xd4) > (__driver_attach) from [] (bus_for_each_dev+0x74/0xb4) > (bus_for_each_dev) from [] (bus_add_driver+0x188/0x20c) > (bus_add_driver) from [] (driver_register+0x7c/0x114) > (driver_register) from [] (do_one_initcall+0x54/0x194) > (do_one_initcall) from [] (kernel_init_freeable+0x148/0x1e4) > (kernel_init_freeable) from [] (kernel_init+0x8/0x10c) > (kernel_init) from [] (ret_from_fork+0x14/0x2c) > > I *think* this is because of a catch 22: the FB emulation want to > send the first frame out, so it starts the vblank and wait for the > first blank before sending the first new fresh framebuffer update > by calling .update(). This shouldn't have anything to do with the fb emulation or fb helpers - all it doesn is light up the display like any other kms clients. Doing a dpms off/on with X or similar should confirm that. > But since this DSI command mode panel is not going to give any > IRQs and thus no vblanks before the first frame is sent, it waits > in vain. Video mode DSI panels does not have this characteristic > and it seems all other panels we handle are video mode. > > After the timeout and sending the first frame anyways, of course > the vblank starts working. > > So it seems to me that the FB helper does not really deal very well > with this kind of displays that do not supp
Re: [PATCH v5 07/17] drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay
On Sun, Jan 13, 2019 at 01:07:41AM +0530, Jagan Teki wrote: > > > > > > Again, I cannot help you without the datasheet for the panels you're > > > > > > trying to submit. > > > > > > > > > > The panel bound with Sitronix ST7701 IC > > > > > http://www.startek-lcd.com/res/starteklcd/pdres/201705/20170512144242904.pdf > > > > > > > > It's the controller, not the panel > > > > > > As I said before, the datasheet of that panel doesn't have any > > > information except electrical characteristics and used IC which is > > > ST7701. > > > > > > I have one more panel, which is from Bananapi, S070WV20-CT16 ICN621 > > > please find the attachment for the same. > > > > > > Here is some more details of the same. > > > > > > https://github.com/yesnoandor/x300/blob/master/kernel/arch/arm/boot/dts/erobbing/x300/x300.dtsi#L81 > > > https://github.com/wxzed/Raspberry_5MIPI_Display/blob/master/I2C_Slave/USER/main.c#L15 > > > > > > https://github.com/eliot-shao/qcom/blob/master/icn6211_cxn0102/kernel/drivers/video/msm/mdss/mdss_i2c_interface.c#L152 > > > matches timings for > > > https://github.com/eliot-shao/qcom/blob/master/icn6211_cxn0102/kernel/arch/arm/boot/dts/qcom/dsi-mipi-2-rgb_1280p_video.dtsi#L20 > > > > > > https://github.com/zestroly/micromat/blob/master/test/raspberry/ICN6211.cpp#L169 > > > > Where did you get the timings from then? > > You mean drm_mode timings, the same panel with RGB available in > panel-simple[1], and dsi driver in Mailing list[2] and actual DSI > sequence commands from BSP[3] > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/panel/panel-simple.c?id=7ad8b41cd8f5c2842646d01cdd576663caee04a7 > [2] https://patchwork.kernel.org/patch/10680331/ > [3] > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/lcd/S070WV20_MIPI_RGB.c So you had no reliable source for the timings then? How do you know if all your patches that are timing related are right then? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/sun4i: backend: add missing of_node_puts
On Sun, Jan 13, 2019 at 09:47:44AM +0100, Julia Lawall wrote:
> The device node iterators perform an of_node_get on each
> iteration, so a jump out of the loop requires an of_node_put.
>
> Remote and port also have augmented reference counts, so drop them
> on each iteration and at the end of the function, respectively.
> Remote is only used for the address it contains, not for the
> contents of that address, so the reference count can be dropped
> immediately.
>
> The semantic patch that fixes the first part of this problem is
> as follows (http://coccinelle.lip6.fr):
>
> //
> @@
> expression root,e;
> local idexpression child;
> iterator name for_each_child_of_node;
> @@
>
> for_each_available_child_of_node(root, child) {
>... when != of_node_put(child)
>when != e = child
> + of_node_put(child);
> ? break;
>...
> }
> ... when != child
> //
>
> Signed-off-by: Julia Lawall
Applied, thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] Re: HDMI/DVI spurious failure
Hi Priit, On Wed, Jan 16, 2019 at 07:58:54AM +, Priit Laes wrote: > > On Mon, Jan 14, 2019 at 01:29:34PM +, Priit Laes wrote: > > > I have a somewhat curious case with one HDMI/DVI screen that fails > > > to initialize properly every 6-7 boots. The display itself is also > > > somewhat flawed (missing HPD pin and the VSYNC/HSYNC pulse width > > > is set to 0 in EDID), but I suspect there could be some issues > > > regarding timing in A20 HDMI driver in Linux. > > > > ... > > > It doesn't look related to the clock rate itself, since it doesn't > > change between the two cases. However, in one case the DDC clock is > > enabled and in the other it's disabled. > > > > Was it taken at the same time? Maybe you can try with that patch? > > http://code.bulix.org/z7jmkm-555344?raw > > Thanks, after doing ~50+ boots I haven't seen a single failure. > > Previously I had following failure cases which are now both fixed: > > a) Linux without u-boot HDMI, where one in every 6-7 boots failed. > b) u--boot with hdmi enabled switching to simplefb and then switching > to kms, where previously all boots ended up with garbled screen. So it's not really a fix, but it really looks like the clock is not enabled when it should. Can you describe your test scenario a bit more? What are you doing exactly, just booting? When do you start using the display? When did you capture the debugfs output that you pasted? Thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Expose more EDID fields to userspace
On Wed, 2019-01-16 at 20:35 +0200, Pekka Paalanen wrote: > I would agree with an effort to establish a userspace EDID parsing > library in any case. As mentioned above, there will probably be too > much to expose via kernel UABI, or it will become just another > encoded format that again should have a shared parser library in > userspace. > > Would it be possible to architect the library so that it would be > shared with the kernel? Maybe the quirks database could be shared > with the kernel as well? That way both kernel and userspace would > more or less agree on the parsing details. If the kernel wanted to expose its quirks db somehow the library could certainly be taught to use it. However there are likely to be quirks relevant only to userspace (see below), making the kernel carry that doesn't make a ton of sense. > I've been dreaming of a "liboutput" that would e.g. parse EDID, > generate CVT video mode timings, and whatnot that all display > servers more or less will duplicate. Once upon a time Ajax started > minitru IIRC... It's still not the worst idea, I'd be happy to revisit it. Part of the problem with the idea is that EDID parsing is not unambiguous. There are several fields for "physical output size" with slightly different semantics, which one do you want? There are both structured and free-form ways to encode monitor name and serial number. There are multiple ways to ask for 1920x1080, how many slightly different timings do you want? Some more-modern displays have DisplayID blocks too, which we're not fetching, and which can easily disagree with EDID; which version of reality would you like? Me personally I'm happy to make policy decisions about this kind of thing, but there are likely to be cases where a display server wants to make more subtle decisions, in which case it's likely to ignore any "cooked" queries you might of the library and instead try parsing the data itself. Many of these decisions currently don't affect the kernel at all (we don't use physical size for anything, but userspace thinks it wants to know about DPI...). > Another thing for "liboutput" was a device description database, > whose first use would have been the "non-desktop" property. Because > we don't expose monitors through udev to have udev rules tag them > with interesting bits. > > Imagine this: monitors exposed as devices via udev, tagged with > helpers as regular monitor, HMD, TV, projector, ... and all EDID > fields decoded as well. And quirks in hwdb. But I suppose that > won't happen, because a "monitor device node" would have no other > use. Except... programmatical monitor controls? Like backlight, > brightness, contrast, and so on. This could be interesting. I'd half-written DDC/CI support for drm a while ago, which would allow us to expose that kind of thing. But to be broadly useful you'd want to mirror the same kind of knobs for like platform backlight control, and so far the kernel has steadfastly refused to believe it can possibly associate a backlight controller with a drm output in any userspace-useful way. - ajax ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] drm/msm: Fix A6XX support for opp-level
On Wed, Jan 16, 2019 at 10:46:21AM -0800, Douglas Anderson wrote:
> The bindings for Qualcomm opp levels changed after being Acked but
> before landing. Thus the code in the GPU driver that was relying on
> the old bindings is now broken.
>
> Let's change the code to match the new bindings by adjusting the old
> string 'qcom,level' to the new string 'opp-level'. See the patch
> ("dt-bindings: opp: Introduce opp-level bindings").
>
> NOTE: we will do additional cleanup to totally remove the string from
> the code and use the new dev_pm_opp_get_level() but we'll do it in a
> future patch. This will facilitate getting the important code fix in
> sooner without having to deal with cross-maintainer dependencies.
>
> This patch needs to land before the patch ("arm64: dts: sdm845: Add
> gpu and gmu device nodes") since if a tree contains the device tree
> patch but not this one you'll get a crash at bootup.
>
> Fixes: 4b565ca5a2cb ("drm/msm: Add A6XX device support")
> Signed-off-by: Douglas Anderson
I agree that splitting these out make sense for the workflow.
Reviewed-by: Jordan Crouse
> ---
>
> Changes in v2:
> - Split into two patches to facilitate landing.
>
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 5beb83d1cf87..ce1b3cc4bf6d 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -944,7 +944,7 @@ static u32 a6xx_gmu_get_arc_level(struct device *dev,
> unsigned long freq)
> np = dev_pm_opp_get_of_node(opp);
>
> if (np) {
> - of_property_read_u32(np, "qcom,level", &val);
> + of_property_read_u32(np, "opp-level", &val);
> of_node_put(np);
> }
>
> --
> 2.20.1.97.g81188d93c3-goog
>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next
Hi Daniel, Dave,
Here is a re-run of the previous drm-misc-next as asked by Daniel.
Thanks!
Maxime
drm-misc-next-2019-01-16:
drm-misc-next for 5.1:
UAPI Changes:
- New fourcc identifier for ARM Framebuffer Compression v1.3
Cross-subsystem Changes:
Core Changes:
- Reorganisation of drm_device and drm_framebuffer headers
- Cleanup of the drmP inclusion
- Fix leaks in the fb-helpers
- Allow for depth different from bpp in fb-helper fbdev emulation
- Remove drm_mode_object from drm_display_mode
Driver Changes:
- Add reflection properties to rockchip
- a bunch of fixes for virtio
- a bunch of fixes for dp_mst and drivers using it, and introduction of a
new refcounting scheme
- Convertion of bochs to atomic and generic fbdev emulation
- Allow meson to remove the firmware framebuffers
The following changes since commit e3d093070eb0b5e3df668d3eb04100ea79343c65:
Merge tag 'tilcdc-4.22' of https://github.com/jsarha/linux into drm-next
(2019-01-11 06:29:31 +1000)
are available in the Git repository at:
git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2019-01-16
for you to fetch changes up to 94520db52fc0e931327bb77fe79a952a0e9dd2b0:
drm: fix alpha build after drm_util.h change (2019-01-16 11:16:17 +0100)
drm-misc-next for 5.1:
UAPI Changes:
- New fourcc identifier for ARM Framebuffer Compression v1.3
Cross-subsystem Changes:
Core Changes:
- Reorganisation of drm_device and drm_framebuffer headers
- Cleanup of the drmP inclusion
- Fix leaks in the fb-helpers
- Allow for depth different from bpp in fb-helper fbdev emulation
- Remove drm_mode_object from drm_display_mode
Driver Changes:
- Add reflection properties to rockchip
- a bunch of fixes for virtio
- a bunch of fixes for dp_mst and drivers using it, and introduction of a
new refcounting scheme
- Convertion of bochs to atomic and generic fbdev emulation
- Allow meson to remove the firmware framebuffers
Daniel Vetter (15):
drm/todo: Better defio support in the generic fbdev emulation
drm/crtc-helpers: WARN when used with atomic drivers
drm/ch7006: Stop using drm_crtc_force_disable
drm/nouveau: Stop using drm_crtc_force_disable
drm: Unexport drm_crtc_force_disable
drm/atomic: Add missing () to function ref in kerneldoc
drm: Move the legacy kms disable_all helper to crtc helpers
drm/arc: Don't set the dpms hook
drm/tda998x: Don't set dpms hook
drm/doc: Polish kerneldoc for drm_device.h
drm/docs: improve docs for drm_drv.c
drm/of: Fix kerneldoc
drm/panel: Small documentation polish
drm/doc: Move bridge link target to the right place
staging/vboxvideo: Don't set FBINFO_MISC_ALWAYS_SETPAR
Daniele Castagna (2):
drm/rockchip: Fix YUV buffers color rendering
drm/rockchip: Add reflection properties
Enric Balletbo i Serra (1):
drm/rockchip: update cursors asynchronously through atomic.
Ezequiel Garcia (5):
drm/virtio: Remove incorrect kfree()
drm/virtio: Add missing virtqueue reset
drm/virtio: Drop deprecated load/unload initialization
drm/rockchip: Fix typo in VOP macros argument
drm/rockchip: Separate RK3288 from RK3368 win01 registers
Gerd Hoffmann (19):
drm/virtio: log error responses
drm/virtio: fix pageflip flush
drm/virtio: drop virtio_gpu_fence_cleanup()
drm/bochs: encoder cleanup
drm/bochs: split bochs_hw_setmode
drm/bochs: atomic: add atomic_flush+atomic_enable callbacks.
drm/bochs: atomic: add mode_set_nofb callback.
drm/bochs: atomic: switch planes to atomic, wire up helpers.
drm/bochs: atomic: use atomic set_config helper
drm/bochs: atomic: use atomic page_flip helper
drm/bochs: atomic: use suspend/resume helpers
drm/bochs: atomic: set DRIVER_ATOMIC
drm/bochs: remove old bochs_crtc_* functions
drm/bochs: drop unused gpu_addr arg from bochs_bo_pin()
drm/bochs: move ttm_bo_(un)reserve calls into bochs_bo_{pin, unpin}
drm/bochs: add basic prime support
drm/bochs: switch to generic drm fbdev emulation
drm/bochs: drop old fbdev emulation code
drm/bochs: move remaining fb bits to kms
Gustavo A. R. Silva (1):
qxl: Use struct_size() in kzalloc()
Kuo-Hsin Yang (1):
drm/gem: Mark pinned pages as unevictable
Linus Walleij (2):
drm/fb-helper: Scale back depth to supported maximum
drm/panel: Add a driver for the TPO TPG110
Lyude Paul (21):
drm/dp_mst: Fix some formatting in drm_dp_add_port()
drm/dp_mst: Fix some formatting in drm_dp_payload_send_msg()
drm/dp_mst: Fix some formatting in drm_dp_mst_allocate_vcpi()
drm/dp_mst: Fix some formatting in drm_dp_mst_deallocate_vcpi()
drm/dp_mst: Rename drm_dp_mst_get_validated_(port|mstb)_ref and friends
drm/dp_mst: Introduce n
Re: Expose more EDID fields to userspace
Adam Jackson writes: > If the kernel wanted to expose its quirks db somehow the library could > certainly be taught to use it. However there are likely to be quirks > relevant only to userspace (see below), making the kernel carry that > doesn't make a ton of sense. We do expose some of the quirks to user space, but not as a database, and not consistently. Some of the quirks just match EDID data to drive some decision (like non-desktop). Other quirks override some EDID data that is 'wrong'. For these latter instances, I wonder if we shouldn't be re-writing the EDID data that user space gets? Or at least making sure the quirked values are available to user space? > Part of the problem with the idea is that EDID parsing is not > unambiguous. There are several fields for "physical output size" with > slightly different semantics, which one do you want? There are both > structured and free-form ways to encode monitor name and serial > number. Hrm. For places where the kernel *does* parse the data, it would sure be nice to have the kernel version to at least make that consistent. Model/serial data used to select quirks seems like something we should be exposing? I'm getting the feeling that either extreme approach (parse all of the EDID data, or parse none of it) is not going to work and that our current technique of picking some EDID-derived data to expose as separate parsed values, and some to leave for user space to discover for itself is where we'll be stuck for at least the near term. If we come to agreement that this approach is what we'll be doing, then I'd like to have a couple of suggestions: 1) Document what we've got today 2) Document basic guidelines of when to expose parsed EDID-derived data and when to just leave it in the EDID block for user space 3) Plan on exposing all values which the kernel *does* use internally as parsed-out properties. Especially values which get quirked, possibly exposing both the "raw" value and the "quirk" value. 4) Decide if we want to let user-space in on the quirking fun. I can imagine a user-space helper that gets run at hot-plug time, reads the EDID data and then pokes quirk values into the kernel. 5) Decide, as a matter of policy, whether the kernel will ever edit EDID data that gets exposed to user space. I can think of good reasons on both sides, but I think we need to hash out what the kernel will do so that user space knows what's going on. I think what I want is for kernel and user space to at least be operating on the same data, even if we end up re-implementing a pile of code up in user space. -- -keith signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/3] drm/dp_mst: Misc drive-by fixes
Two not terribly important fixes, and one actually important fix. Should be pretty easy to review :) Lyude Paul (3): drm/dp_mst: Remove lock check in drm_atomic_get_mst_topology_state() drm/dp_mst: Fix potential topology ref leak in drm_dp_atomic_find_vcpi_slots() drm/dp_mst: Fix topology ref leak in drm_dp_mst_allocate_vcpi() drivers/gpu/drm/drm_dp_mst_topology.c | 53 --- include/drm/drm_dp_mst_helper.h | 20 +- 2 files changed, 33 insertions(+), 40 deletions(-) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/dp_mst: Fix topology ref leak in drm_dp_mst_allocate_vcpi()
Another drive-by fix. If slots < 0, or drm_dp_init_vcpi() fails, we'll
end up returning from the function without dropping the topology ref
that we grabbed at the very start. This would result in an MST hub
and/or it's ports staying around even after the MST topology has been
removed from the system.
So, fix this by making sure that we always drop the topology ref to port
when returning from this function.
Additionally: it looks like this bug exists pre-topology & malloc krefs,
so let's also make sure this gets backported to stable.
Signed-off-by: Lyude Paul
Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper (v0.6)")
Cc: David Airlie
Cc: Daniel Vetter
Cc: # v3.17+
---
drivers/gpu/drm/drm_dp_mst_topology.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index d99560c5c693..403f035dc8b8 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3177,28 +3177,29 @@ EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots);
bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port, int pbn, int slots)
{
- int ret;
+ int rc;
+ bool ret = false;
- port = drm_dp_mst_topology_get_port_validated(mgr, port);
- if (!port)
+ if (slots < 0)
return false;
- if (slots < 0)
+ port = drm_dp_mst_topology_get_port_validated(mgr, port);
+ if (!port)
return false;
if (port->vcpi.vcpi > 0) {
DRM_DEBUG_KMS("payload: vcpi %d already allocated for pbn %d -
requested pbn %d\n",
port->vcpi.vcpi, port->vcpi.pbn, pbn);
if (pbn == port->vcpi.pbn) {
- drm_dp_mst_topology_put_port(port);
- return true;
+ ret = true;
+ goto out;
}
}
- ret = drm_dp_init_vcpi(mgr, &port->vcpi, pbn, slots);
- if (ret) {
+ rc = drm_dp_init_vcpi(mgr, &port->vcpi, pbn, slots);
+ if (rc) {
DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n",
- DIV_ROUND_UP(pbn, mgr->pbn_div), ret);
+ DIV_ROUND_UP(pbn, mgr->pbn_div), rc);
goto out;
}
DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
@@ -3206,10 +3207,10 @@ bool drm_dp_mst_allocate_vcpi(struct
drm_dp_mst_topology_mgr *mgr,
/* Keep port allocated until it's payload has been removed */
drm_dp_mst_get_port_malloc(port);
- drm_dp_mst_topology_put_port(port);
- return true;
+ ret = true;
out:
- return false;
+ drm_dp_mst_topology_put_port(port);
+ return ret;
}
EXPORT_SYMBOL(drm_dp_mst_allocate_vcpi);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/dp_mst: Remove lock check in drm_atomic_get_mst_topology_state()
Since modesetting locks by default were added to private objects in:
commit b962a12050a3 ("drm/atomic: integrate modeset lock with private
objects")
This means that the atomic state of a drm_dp_mst_topology_mgr is now
protected by drm_dp_mst_topology_mgr->base.lock as opposed to the main
connection_status mutex. This also means that locking isn't left up to
the caller anymore, and is handled automatically in
drm_atomic_get_private_obj_state().
So, remove the WARN_ON() in drm_atomic_get_mst_topology_state() since
that's now incorrect, and update the kernel docs for the function.
Additionally since all function is now is a simple wrapper around
drm_atomic_get_private_obj_state(), it seems more reasonable to move
this out of drm_dp_mst_topology.c and turn it into an inline function in
drm_dp_mst_helper.h
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 25 -
include/drm/drm_dp_mst_helper.h | 20 ++--
2 files changed, 18 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 196ebba8af5f..49575b80caeb 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3718,31 +3718,6 @@ const struct drm_private_state_funcs
drm_dp_mst_topology_state_funcs = {
};
EXPORT_SYMBOL(drm_dp_mst_topology_state_funcs);
-/**
- * drm_atomic_get_mst_topology_state: get MST topology state
- *
- * @state: global atomic state
- * @mgr: MST topology manager, also the private object in this case
- *
- * This function wraps drm_atomic_get_priv_obj_state() passing in the MST
atomic
- * state vtable so that the private object state returned is that of a MST
- * topology object. Also, drm_atomic_get_private_obj_state() expects the caller
- * to care of the locking, so warn if don't hold the connection_mutex.
- *
- * RETURNS:
- *
- * The MST topology state or error pointer.
- */
-struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct
drm_atomic_state *state,
- struct
drm_dp_mst_topology_mgr *mgr)
-{
- struct drm_device *dev = mgr->dev;
-
- WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
- return to_dp_mst_topology_state(drm_atomic_get_private_obj_state(state,
&mgr->base));
-}
-EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
-
/**
* drm_dp_mst_topology_mgr_init - initialise a topology manager
* @mgr: manager struct to initialise
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 8c97a5f92c47..263d82178ecd 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -643,8 +643,6 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr);
int __must_check
drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr);
-struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct
drm_atomic_state *state,
- struct
drm_dp_mst_topology_mgr *mgr);
int __must_check
drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
struct drm_dp_mst_topology_mgr *mgr,
@@ -756,4 +754,22 @@ __drm_dp_mst_state_iter_get(struct drm_atomic_state *state,
for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr),
NULL, &(new_state), (__i)))
+/**
+ * drm_atomic_get_mst_topology_state() - get MST topology state
+ * @state: global atomic state
+ * @mgr: MST topology manager, also the private object in this case
+ *
+ * This function retrieves the atomic topology state for @mgr using
+ * drm_atomic_get_priv_obj_state().
+ *
+ * Returns:
+ * The atomic MST topology state or an error pointer.
+ */
+static inline struct drm_dp_mst_topology_state *
+drm_atomic_get_mst_topology_state(struct drm_atomic_state *state,
+ struct drm_dp_mst_topology_mgr *mgr)
+{
+ return to_dp_mst_topology_state(drm_atomic_get_private_obj_state(
+ state, &mgr->base));
+}
#endif
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/dp_mst: Fix potential topology ref leak in drm_dp_atomic_find_vcpi_slots()
This is a very minute issue I introduced that I just noticed when
scrolling through drm_dp_mst_topology.c: if a driver uses
drm_dp_atomic_find_vcpi_slots() incorrectly, we'll forget to release the
topology reference we grabbed on port.
So, fix this by jumping to 'out' instead of returning -EINVAL
immediately.
Signed-off-by: Lyude Paul
Fixes: eceae1472467 ("drm/dp_mst: Start tracking per-port VCPI allocations")
Cc: Daniel Vetter
Cc: David Airlie
Cc: Harry Wentland
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 49575b80caeb..d99560c5c693 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3070,7 +3070,8 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state
*state,
if (WARN_ON(!prev_slots)) {
DRM_ERROR("cannot allocate and release VCPI on
[MST PORT:%p] in the same state\n",
port);
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
break;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109331] Empire Total War - Graphical Corruption
https://bugs.freedesktop.org/show_bug.cgi?id=109331 andrew.m.mcma...@gmail.com changed: What|Removed |Added Blocks||77449 Version|18.3|git Referenced Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=77449 [Bug 77449] Tracker bug for all bugs related to Steam titles -- 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
[Bug 108917] gamma adjustments cause stuttering with amdgpu.dc=1, especially problematic with RedShift etc.
https://bugs.freedesktop.org/show_bug.cgi?id=108917 --- Comment #7 from Hans D --- Can confirm that enabling redshift causes occasional stutters every 2-4 seconds with amdgpu.dc=1 with linux 5.0rc2. Without redshift everything (scrolling in browser, video playback)is buttery smooth. With amdgpu.dc=0 redshift doesn't introduce any hiccups. -- 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 v2 2/3] dt-bindings: drm/panel: simple: add support for PDA 91-00156-A0
On Mon, 14 Jan 2019 09:43:28 +, wrote: > From: Cristian Birsan > > PDA 91-00156-A0 5.0 is a 5.0" WVGA TFT LCD panel. > This panel with backlight is found in PDA 5" LCD screen (TM5000 series or > AC320005-5). > Adding Device Tree bindings for this panel. > > Signed-off-by: Cristian Birsan > [eugen.hris...@microchip.com]: specified backlight and supply bindings > Signed-off-by: Eugen Hristev > --- > Changes in v2: > - added binding explanations for required properties from simple-panel: > backlight and power supply which are mandatory > > .../devicetree/bindings/display/panel/pda,91-00156-a0.txt | 14 > ++ > 1 file changed, 14 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/pda,91-00156-a0.txt > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dt-bindings: drm/msm/a6xx: Document GMU bindings
Commit 24937c540917 ("dt-bindings: drm/msm/a6xx: Document GMU and update
GPU bindings") mistakenly omitted the GMU bindings as seen in [1].
Return them to their rightful place.
[1] https://patchwork.freedesktop.org/patch/268679/
Signed-off-by: Jordan Crouse
Reviewed-by: Rob Herring
---
.../devicetree/bindings/display/msm/gmu.txt| 59 ++
1 file changed, 59 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt
diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt
b/Documentation/devicetree/bindings/display/msm/gmu.txt
new file mode 100644
index 000..3439b38
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/gmu.txt
@@ -0,0 +1,59 @@
+Qualcomm adreno/snapdragon GMU (Graphics management unit)
+
+The GMU is a programmable power controller for the GPU. the CPU controls the
+GMU which in turn handles power controls for the GPU.
+
+Required properties:
+- compatible: "qcom,adreno-gmu-XYZ.W", "qcom,adreno-gmu"
+for example: "qcom,adreno-gmu-630.2", "qcom,adreno-gmu"
+ Note that you need to list the less specific "qcom,adreno-gmu"
+ for generic matches and the more specific identifier to identify
+ the specific device.
+- reg: Physical base address and length of the GMU registers.
+- reg-names: Matching names for the register regions
+ * "gmu"
+ * "gmu_pdc"
+ * "gmu_pdc_seg"
+- interrupts: The interrupt signals from the GMU.
+- interrupt-names: Matching names for the interrupts
+ * "hfi"
+ * "gmu"
+- clocks: phandles to the device clocks
+- clock-names: Matching names for the clocks
+ * "gmu"
+ * "cxo"
+ * "axi"
+ * "mnoc"
+- power-domains: should be <&clock_gpucc GPU_CX_GDSC>
+- iommus: phandle to the adreno iommu
+- operating-points-v2: phandle to the OPP operating points
+
+Example:
+
+/ {
+ ...
+
+ gmu: gmu@506a000 {
+ compatible="qcom,adreno-gmu-630.2", "qcom,adreno-gmu";
+
+ reg = <0x506a000 0x3>,
+ <0xb28 0x1>,
+ <0xb48 0x1>;
+ reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
+
+ interrupts = ,
+;
+ interrupt-names = "hfi", "gmu";
+
+ clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
+ <&gpucc GPU_CC_CXO_CLK>,
+ <&gcc GCC_DDRSS_GPU_AXI_CLK>,
+ <&gcc GCC_GPU_MEMNOC_GFX_CLK>;
+ clock-names = "gmu", "cxo", "axi", "memnoc";
+
+ power-domains = <&gpucc GPU_CX_GDSC>;
+ iommus = <&adreno_smmu 5>;
+
+ operating-points-v2 = <&gmu_opp_table>;
+ };
+};
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
linux-next: Fixes tag needs some work in the drm-misc tree
[I am experimenting with checking the Fixes tags in commits in linux-next.
Please let me know if you think I am being too strict.]
Hi all,
Commit
94520db52fc0 ("drm: fix alpha build after drm_util.h change")
has a malformed Fixes tag:
Fixes: 733748ac37b45 ("drm: move drm_can_sleep() to drm_util.h")
The SHA1 does not reference a commit currently in the tree. Maybe it
was meant to be e9eafcb58921.
--
Cheers,
Stephen Rothwell
pgpfwMrCl6QPr.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
