Re: [PATCH v15 1/3] drm: rockchip: Add basic drm driver

Re: [PATCH v15 1/3] drm: rockchip: Add basic drm driver

[Tomeu Vizoso]

On 2 December 2014 at 10:15, Mark Yao <mark.yao-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> new file mode 100644
> index 0000000..e7ca25b
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -0,0 +1,1455 @@
...
> +static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
> +                               const struct drm_display_mode *mode,
> +                               struct drm_display_mode *adjusted_mode)
> +{
> +       if (adjusted_mode->htotal == 0 || adjusted_mode->vtotal == 0)
> +               return false;

Hi Mark,

what's the rationale for this?

Disabling a CRTC as in [0] will cause mode_fixup() to be called with
an empty mode, failing that test.

Removing the check seems to get things working fine for a short while,
but a later modeset invariably gets the VOP to hang (as reported by
[1]).

Do you know why that check was put in place and what exactly could be
causing the hw to hang?

[0] https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/lib/igt_kms.c#n1616
[1] https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rockchip/rockchip_drm_vop.c#n873

Thanks,

Tomeu

> +
> +       return true;
> +}

Re: [PATCH v15 1/3] drm: rockchip: Add basic drm driver

[Mark yao]

On 2016年03月14日 21:35, Tomeu Vizoso wrote:
> On 2 December 2014 at 10:15, Mark Yao <mark.yao-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> new file mode 100644
>> index 0000000..e7ca25b
>> --- /dev/null
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -0,0 +1,1455 @@
> ...
>> +static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
>> +                               const struct drm_display_mode *mode,
>> +                               struct drm_display_mode *adjusted_mode)
>> +{
>> +       if (adjusted_mode->htotal == 0 || adjusted_mode->vtotal == 0)
>> +               return false;
> Hi Mark,
>
> what's the rationale for this?
>
> Disabling a CRTC as in [0] will cause mode_fixup() to be called with
> an empty mode, failing that test.
>
> Removing the check seems to get things working fine for a short while,
> but a later modeset invariably gets the VOP to hang (as reported by
> [1]).
>
> Do you know why that check was put in place and what exactly could be
> causing the hw to hang?
>
> [0] https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/lib/igt_kms.c#n1616
> [1] https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rockchip/rockchip_drm_vop.c#n873
>
> Thanks,
>
> Tomeu
>
Hi Tomeu

Just thinking that "adjusted_mode->htotal == 0 || adjusted_mode->vtotal 
== 0" is not a good mode for vop.

And you said VOP hang, only WARN_ON error message? or system hang, die?

I think maybe crtc disable too fast, vblank is off, then no one can feed the wait_update_complete.
Can you test it again with following patch?
drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -503,6 +503,8 @@ static void vop_crtc_disable(struct drm_crtc *crtc)
         if (!vop->is_enabled)
                 return;
  
+       vop_crtc_wait_for_update(crtc);
+
         drm_crtc_vblank_off(crtc);

Thanks.

-- 
Ｍark Yao


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v15 1/3] drm: rockchip: Add basic drm driver

[Tomeu Vizoso]

[-- Attachment #1: Type: text/plain, Size: 3666 bytes --]

On 15 March 2016 at 02:30, Mark yao <mark.yao-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> On 2016年03月14日 21:35, Tomeu Vizoso wrote:
>>
>> On 2 December 2014 at 10:15, Mark Yao <mark.yao-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>> new file mode 100644
>>> index 0000000..e7ca25b
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>> @@ -0,0 +1,1455 @@
>>
>> ...
>>>
>>> +static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
>>> +                               const struct drm_display_mode *mode,
>>> +                               struct drm_display_mode *adjusted_mode)
>>> +{
>>> +       if (adjusted_mode->htotal == 0 || adjusted_mode->vtotal == 0)
>>> +               return false;
>>
>> Hi Mark,
>>
>> what's the rationale for this?
>>
>> Disabling a CRTC as in [0] will cause mode_fixup() to be called with
>> an empty mode, failing that test.
>>
>> Removing the check seems to get things working fine for a short while,
>> but a later modeset invariably gets the VOP to hang (as reported by
>> [1]).
>>
>> Do you know why that check was put in place and what exactly could be
>> causing the hw to hang?
>>
>> [0]
>> https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/lib/igt_kms.c#n1616
>> [1]
>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rockchip/rockchip_drm_vop.c#n873
>>
>> Thanks,
>>
>> Tomeu
>>
> Hi Tomeu
>
> Just thinking that "adjusted_mode->htotal == 0 || adjusted_mode->vtotal ==
> 0" is not a good mode for vop.

Ah, ok. Guess it should be removed then so we don't break userspace?

> And you said VOP hang, only WARN_ON error message? or system hang, die?

Sorry, the symptom was only the warning, I just went a bit too far and
assumed the VOP had stopped working at all.

> I think maybe crtc disable too fast, vblank is off, then no one can feed the
> wait_update_complete.
> Can you test it again with following patch?

Actually, in today's testing I don't see that happening any more,
sorry about that :/

What I have been looking at today is a related issue when running the
kms_flip_event_leak test from intel-gpu-tools. If I remove the check
mentioned above so CRTCs can be disabled with the SetCRTC IOCTL, I see
this page fault the second and subsequent times I run the test.

[   75.809031] rk_iommu ff930300.iommu: Page fault at 0x01000000 of type read
[   75.809035] rk_iommu ff930300.iommu: iova = 0x01000000: dte_index:
0x4 pte_index: 0x0 page_offset: 0x0
[   75.809040] rk_iommu ff930300.iommu: mmu_dte_addr: 0x2c258000
dte@0x2c258010: 0x2c561001 valid: 1 pte@0x2c561000: 0x2a000006 valid:
0 page@0x00000000 flags: 0x0
[   76.951288] rk_iommu ff930300.iommu: Enable stall request timed
out, status: 0x00004b

I have written a smaller standalone test that is attached in case you
want to check it out, but I haven't been able to find out why it only
happens when the test is rerun.

Apparently the VOP is still trying to read a BO (0x01000000) right
when the kernel frees it, but from what I can see, it should be
scanning another BO at that point.

Do you have any ideas on what could be happening?

Thanks,

Tomeu

> drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -503,6 +503,8 @@ static void vop_crtc_disable(struct drm_crtc *crtc)
>         if (!vop->is_enabled)
>                 return;
>  +       vop_crtc_wait_for_update(crtc);
> +
>         drm_crtc_vblank_off(crtc);
>
> Thanks.
>
> --
> Ｍark Yao
>
>

[-- Attachment #2: page_fault.c --]
[-- Type: text/x-csrc, Size: 3368 bytes --]

#include <fcntl.h>
#include <sys/stat.h>
#include <unistd.h>
#include <string.h>
#include <stdio.h>
#include <drm.h>
#include <drm_fourcc.h>
#include <xf86drm.h>
#include <xf86drmMode.h>

__attribute__((format(printf, 1, 2)))
static void kmsg(const char *format, ...)
#define KERN_EMER	"<0>"
#define KERN_ALERT	"<1>"
#define KERN_CRIT	"<2>"
#define KERN_ERR	"<3>"
#define KERN_WARNING	"<4>"
#define KERN_NOTICE	"<5>"
#define KERN_INFO	"<6>"
#define KERN_DEBUG	"<7>"
{
	va_list ap;
	FILE *file;

	file = fopen("/dev/kmsg", "w");
	if (file == NULL)
		return;

	va_start(ap, format);
	fprintf(file, "userspace: ");
	vfprintf(file, format, ap);
	va_end(ap);

	fclose(file);
}

static uint32_t dumb_create(int fd, int width, int height, int bpp)
{
	struct drm_mode_create_dumb create = {};

	create.width = width;
	create.height = height;
	create.bpp = bpp;

	create.handle = 0;
	drmIoctl(fd, DRM_IOCTL_MODE_CREATE_DUMB, &create);

	return create.handle;
}

static uint32_t fb_create(int fd, int width, int height, int format, int bo, int pitch)
{
	struct drm_mode_fb_cmd2 f = {};

	f.width = width;
	f.height = height;
	f.pixel_format = format;
	f.handles[0] = bo;
	f.pitches[0] = pitch;
	f.fb_id = 0;

	drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f);

	return f.fb_id;
}

drmModeModeInfo *connector_get_default_mode(int fd, int conn_id)
{
	drmModeConnectorPtr conn;
	int i;
 
	conn = drmModeGetConnectorCurrent(fd, conn_id);

	for (i = 0; i < conn->count_modes; i++) {
		if (conn->modes[i].type & DRM_MODE_TYPE_PREFERRED)
			return &conn->modes[i];
	}

}

static int dumb_destroy(int fd, uint32_t bo)
{
	struct drm_mode_destroy_dumb arg = {};

	arg.handle = bo;

	return drmIoctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, &arg);
}

void test_pipe(int fd, int pipe, int conn, int crtc)
{
	int ret, bo1, fb1, bo2, fb2;
	drmModeModeInfo *mode;
	drmVBlank wait_vbl = {};

	kmsg("%s: pipe %d conn %d crtc %d\n", __func__, pipe, conn, crtc);

	bo1 = dumb_create(fd, 1920, 1080, 4 * 8);
	kmsg("dumb_create: %d\n", bo1);

	fb1 = fb_create(fd, 1920, 1080, DRM_FORMAT_XRGB8888, bo1, 1920 * 4);
	kmsg("fb_create: %d\n", fb1);

	mode = connector_get_default_mode(fd, conn);
	kmsg("connector_get_default_mode: %s\n", mode->name);

	ret = drmModeSetCrtc(fd,
			     crtc,
			     fb1,
			     0, 0, /* x, y */
			     &conn, /* connectors */
			     1,    /* n_connectors */
			     mode  /* mode */);
	kmsg("drmModeSetCrtc: %d\n", ret);

	wait_vbl.request.type = pipe << DRM_VBLANK_HIGH_CRTC_SHIFT |
				DRM_VBLANK_RELATIVE;
	wait_vbl.request.sequence = 1;

	ret = drmWaitVBlank(fd, &wait_vbl);
	kmsg("drmWaitVBlank: %d\n", ret);

	bo2 = dumb_create(fd, 1920, 1080, 4 * 8);
	kmsg("dumb_create: %d\n", bo2);

	fb2 = fb_create(fd, 1920, 1080, DRM_FORMAT_XRGB8888, bo2, 1920 * 4);
	kmsg("fb_create: %d\n", fb2);

	ret = drmModePageFlip(fd, crtc, fb2, DRM_MODE_PAGE_FLIP_EVENT, NULL);
	kmsg("drmModePageFlip: %d\n", ret);
	
	ret = dumb_destroy(fd, bo2);
	kmsg("dumb_destroy: %d\n", ret);
	
	ret = drmModeRmFB(fd, fb2);
	kmsg("drmModeRmFB: %d\n", ret);

	ret = dumb_destroy(fd, bo1);
	kmsg("dumb_destroy: %d\n", ret);

	ret = drmModeRmFB(fd, fb1);
	kmsg("drmModeRmFB: %d\n", ret);
}

int main(int argc, char **argv)
{
	int ret;
	int fd = open("/dev/dri/card0", O_RDWR);

	ret = drmSetMaster(fd);

	test_pipe(fd, 0, 29, 20);
	test_pipe(fd, 1, 29, 25);

	ret = drmDropMaster(fd);

	close(fd);

	return 0;
}

Re: [PATCH v15 1/3] drm: rockchip: Add basic drm driver

[Tomeu Vizoso]

[-- Attachment #1: Type: text/plain, Size: 4284 bytes --]

On 16 March 2016 at 16:23, Tomeu Vizoso <tomeu-XCtybt49RKsYaV1qd6yewg@public.gmane.org> wrote:
> On 15 March 2016 at 02:30, Mark yao <mark.yao-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
>> On 2016年03月14日 21:35, Tomeu Vizoso wrote:
>>>
>>> On 2 December 2014 at 10:15, Mark Yao <mark.yao-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>> new file mode 100644
>>>> index 0000000..e7ca25b
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>> @@ -0,0 +1,1455 @@
>>>
>>> ...
>>>>
>>>> +static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
>>>> +                               const struct drm_display_mode *mode,
>>>> +                               struct drm_display_mode *adjusted_mode)
>>>> +{
>>>> +       if (adjusted_mode->htotal == 0 || adjusted_mode->vtotal == 0)
>>>> +               return false;
>>>
>>> Hi Mark,
>>>
>>> what's the rationale for this?
>>>
>>> Disabling a CRTC as in [0] will cause mode_fixup() to be called with
>>> an empty mode, failing that test.
>>>
>>> Removing the check seems to get things working fine for a short while,
>>> but a later modeset invariably gets the VOP to hang (as reported by
>>> [1]).
>>>
>>> Do you know why that check was put in place and what exactly could be
>>> causing the hw to hang?
>>>
>>> [0]
>>> https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/lib/igt_kms.c#n1616
>>> [1]
>>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rockchip/rockchip_drm_vop.c#n873
>>>
>>> Thanks,
>>>
>>> Tomeu
>>>
>> Hi Tomeu
>>
>> Just thinking that "adjusted_mode->htotal == 0 || adjusted_mode->vtotal ==
>> 0" is not a good mode for vop.
>
> Ah, ok. Guess it should be removed then so we don't break userspace?
>
>> And you said VOP hang, only WARN_ON error message? or system hang, die?
>
> Sorry, the symptom was only the warning, I just went a bit too far and
> assumed the VOP had stopped working at all.
>
>> I think maybe crtc disable too fast, vblank is off, then no one can feed the
>> wait_update_complete.
>> Can you test it again with following patch?
>
> Actually, in today's testing I don't see that happening any more,
> sorry about that :/
>
> What I have been looking at today is a related issue when running the
> kms_flip_event_leak test from intel-gpu-tools. If I remove the check
> mentioned above so CRTCs can be disabled with the SetCRTC IOCTL, I see
> this page fault the second and subsequent times I run the test.
>
> [   75.809031] rk_iommu ff930300.iommu: Page fault at 0x01000000 of type read
> [   75.809035] rk_iommu ff930300.iommu: iova = 0x01000000: dte_index:
> 0x4 pte_index: 0x0 page_offset: 0x0
> [   75.809040] rk_iommu ff930300.iommu: mmu_dte_addr: 0x2c258000
> dte@0x2c258010: 0x2c561001 valid: 1 pte@0x2c561000: 0x2a000006 valid:
> 0 page@0x00000000 flags: 0x0
> [   76.951288] rk_iommu ff930300.iommu: Enable stall request timed
> out, status: 0x00004b
>
> I have written a smaller standalone test that is attached in case you
> want to check it out, but I haven't been able to find out why it only
> happens when the test is rerun.
>
> Apparently the VOP is still trying to read a BO (0x01000000) right
> when the kernel frees it, but from what I can see, it should be
> scanning another BO at that point.
>
> Do you have any ideas on what could be happening?

Apparently, when the VOP is re-enabled, it will start scanning from
the framebuffer address that had been set last.

Because DMA addresses are recycled and there's going to be a low
number of framebuffers, this isn't going to be obvious unless we make
sure that there isn't a FB allocated at that DMA address any more.

The attached test case does just that.

Regards,

Tomeu

> Thanks,
>
> Tomeu
>
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -503,6 +503,8 @@ static void vop_crtc_disable(struct drm_crtc *crtc)
>>         if (!vop->is_enabled)
>>                 return;
>>  +       vop_crtc_wait_for_update(crtc);
>> +
>>         drm_crtc_vblank_off(crtc);
>>
>> Thanks.
>>
>> --
>> Ｍark Yao
>>
>>

[-- Attachment #2: page_fault.c --]
[-- Type: text/x-csrc, Size: 3588 bytes --]

#include <fcntl.h>
#include <sys/stat.h>
#include <unistd.h>
#include <string.h>
#include <stdio.h>
#include <drm.h>
#include <drm_fourcc.h>
#include <xf86drm.h>
#include <xf86drmMode.h>

__attribute__((format(printf, 1, 2)))
static void kmsg(const char *format, ...)
#define KERN_EMER	"<0>"
#define KERN_ALERT	"<1>"
#define KERN_CRIT	"<2>"
#define KERN_ERR	"<3>"
#define KERN_WARNING	"<4>"
#define KERN_NOTICE	"<5>"
#define KERN_INFO	"<6>"
#define KERN_DEBUG	"<7>"
{
	va_list ap;
	FILE *file;

	file = fopen("/dev/kmsg", "w");
	if (file == NULL)
		return;

	va_start(ap, format);
	fprintf(file, "userspace: ");
	vfprintf(file, format, ap);
	va_end(ap);

	fclose(file);
}

static uint32_t dumb_create(int fd, int width, int height, int bpp)
{
	struct drm_mode_create_dumb create = {};

	create.width = width;
	create.height = height;
	create.bpp = bpp;

	create.handle = 0;
	drmIoctl(fd, DRM_IOCTL_MODE_CREATE_DUMB, &create);

	return create.handle;
}

static uint32_t fb_create(int fd, int width, int height, int format, int bo, int pitch)
{
	struct drm_mode_fb_cmd2 f = {};

	f.width = width;
	f.height = height;
	f.pixel_format = format;
	f.handles[0] = bo;
	f.pitches[0] = pitch;
	f.fb_id = 0;

	drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f);

	return f.fb_id;
}

drmModeModeInfo *connector_get_default_mode(int fd, int conn_id)
{
	drmModeConnectorPtr conn;
	int i;
 
	conn = drmModeGetConnectorCurrent(fd, conn_id);

	for (i = 0; i < conn->count_modes; i++) {
		if (conn->modes[i].type & DRM_MODE_TYPE_PREFERRED)
			return &conn->modes[i];
	}

}

static int dumb_destroy(int fd, uint32_t bo)
{
	struct drm_mode_destroy_dumb arg = {};

	arg.handle = bo;

	return drmIoctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, &arg);
}

int main(int argc, char **argv)
{
	int ret;
	int fd;
	int crtc = 25;
	int conn = 29;
	int bo1, fb1, bo2, fb2;
	drmModeModeInfo *mode;

	kmsg("------------- start ----------------\n");

	fd = open("/dev/dri/card0", O_RDWR);

	ret = drmSetMaster(fd);

	kmsg("%s: pipe %d conn %d crtc %d\n", __func__, pipe, conn, crtc);

	bo1 = dumb_create(fd, 1920, 1080, 4 * 8);
	kmsg("dumb_create 1: %d\n", bo1);

	fb1 = fb_create(fd, 1920, 1080, DRM_FORMAT_XRGB8888, bo1, 1920 * 4);
	kmsg("fb_create 1: %d\n", fb1);

	mode = connector_get_default_mode(fd, conn);
	kmsg("connector_get_default_mode: %s\n", mode->name);

	ret = drmModeSetCrtc(fd,
			     crtc,
			     fb1,
			     0, 0, /* x, y */
			     &conn, /* connectors */
			     1,    /* n_connectors */
			     mode  /* mode */);
	kmsg("drmModeSetCrtc: %d\n", ret);

	ret = drmModeSetCrtc(fd,
			     crtc,
			     fb1,
			     0, 0, /* x, y */
			     NULL, /* connectors */
			     0,    /* n_connectors */
			     NULL  /* mode */);

	/* Create a second BO before destroying the first one, so it has a
	 * different DMA address
	 */
	bo2 = dumb_create(fd, 1920, 1080, 4 * 8);
	kmsg("dumb_create 2: %d\n", bo2);

	fb2 = fb_create(fd, 1920, 1080, DRM_FORMAT_XRGB8888, bo2, 1920 * 4);
	kmsg("fb_create 2: %d\n", fb2);

	/* Make sure we don't hold a reference to the first BO */
	ret = drmModeRmFB(fd, fb1);
	kmsg("drmModeRmFB: %d\n", ret);
	
	ret = dumb_destroy(fd, bo1);
	kmsg("dumb_destroy: %d\n", ret);

	/* The CRTC will be enabled with the old configuration and the VOP
	 * will try to read from the first BO.
	 */
	ret = drmModeSetCrtc(fd,
			     crtc,
			     fb2,
			     0, 0, /* x, y */
			     &conn, /* connectors */
			     1,    /* n_connectors */
			     mode  /* mode */);
	kmsg("drmModeSetCrtc: %d\n", ret);
	
	ret = drmDropMaster(fd);

	close(fd);

	kmsg("-------------- end -----------------\n");

	return 0;
}