From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark yao Subject: Re: [PATCH] drm/rockchip: vop: Reset yrgb_mst when re-enabling Date: Sat, 19 Mar 2016 09:15:45 +0800 Message-ID: <56ECA841.40803@rock-chips.com> References: <1458300147-6472-1-git-send-email-tomeu.vizoso@collabora.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1458300147-6472-1-git-send-email-tomeu.vizoso@collabora.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Tomeu Vizoso , linux-kernel@vger.kernel.org Cc: linux-rockchip@lists.infradead.org, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org List-Id: linux-rockchip.vger.kernel.org T24gMjAxNuW5tDAz5pyIMTjml6UgMTk6MjIsIFRvbWV1IFZpem9zbyB3cm90ZToKPiBXaGVuIHRo ZSBWT1AgaXMgcmUtZW5hYmxlZCwgaXQgd2lsbCBzdGFydCBzY2FubmluZyByaWdodCBhd2F5IHRo ZQo+IGZyYW1lYnVmZmVycyB0aGF0IHdlcmUgY29uZmlndXJlZCBmcm9tIHRoZSBsYXN0IHRpbWUs IGV2ZW4gaWYgdGhvc2UgaGF2ZQo+IGJlZW4gZGVzdHJveWVkIGFscmVhZHkuIFRvIHByZXZlbnQg dGhlIFZPUCBmcm9tIHRyeWluZyB0byBhY2Nlc3MgZnJlZWQKPiBtZW1vcnksIHJlc2V0IHRoZSBy ZWdpc3RlcnMgdGhhdCBob2xkIHBvaW50ZXJzIHRvIGZyYW1lYnVmZmVycyByaWdodAo+IGFmdGVy IHdlIGNhbiB3cml0ZSB0byB0aGVtLCBidXQgYmVmb3JlIHRoZSBWT1AgaXMgYXdha2VuIGZyb20g c3RhbmRieS4KPgo+IFNpZ25lZC1vZmYtYnk6IFRvbWV1IFZpem9zbyA8dG9tZXUudml6b3NvQGNv bGxhYm9yYS5jb20+Cj4gTGluazogaHR0cDovL2xrbWwua2VybmVsLm9yZy9nL0NBQU9ic0tBdisw NWloNVUrPTRraWNfTnNqR01oZnhZaGVIUjh4WFhtYWNacytwNVNId0BtYWlsLmdtYWlsLmNvbQo+ IC0tLQo+ICAgZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL3JvY2tjaGlwX2RybV92b3AuYyB8IDEz ICsrKysrKysrKysrKysKPiAgIDEgZmlsZSBjaGFuZ2VkLCAxMyBpbnNlcnRpb25zKCspCj4KPiBk aWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL3JvY2tjaGlwX2RybV92b3AuYyBi L2RyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9yb2NrY2hpcF9kcm1fdm9wLmMKPiBpbmRleCA1ZTU3 ZjViMmU0YjAuLjBkZjkxYzI4NzQwYiAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vcm9j a2NoaXAvcm9ja2NoaXBfZHJtX3ZvcC5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlw L3JvY2tjaGlwX2RybV92b3AuYwo+IEBAIC00MjksNiArNDI5LDcgQEAgc3RhdGljIHZvaWQgdm9w X2RzcF9ob2xkX3ZhbGlkX2lycV9kaXNhYmxlKHN0cnVjdCB2b3AgKnZvcCkKPiAgIHN0YXRpYyB2 b2lkIHZvcF9lbmFibGUoc3RydWN0IGRybV9jcnRjICpjcnRjKQo+ICAgewo+ICAgCXN0cnVjdCB2 b3AgKnZvcCA9IHRvX3ZvcChjcnRjKTsKPiArCWludCBpOwo+ICAgCWludCByZXQ7Cj4gICAKPiAg IAlpZiAodm9wLT5pc19lbmFibGVkKQo+IEBAIC00NzYsNiArNDc3LDE4IEBAIHN0YXRpYyB2b2lk IHZvcF9lbmFibGUoc3RydWN0IGRybV9jcnRjICpjcnRjKQo+ICAgCSAqLwo+ICAgCXZvcC0+aXNf ZW5hYmxlZCA9IHRydWU7Cj4gICAKPiArCS8qCj4gKwkgKiBCZWZvcmUgdHVybmluZyB0aGUgVk9Q IGNvbXBsZXRlbHkgb24sIHVuc2V0IHRoZSByZWdpc3RlcnMKPiArCSAqIGNvbnRhaW5pbmcgRkIg YWRkcmVzc2VzIHRvIGF2b2lkIHRoZSBIVyBzdGFydCBzY2FubmluZyBvbGQgRkJzLgo+ICsJICov Cj4gKwlmb3IgKGkgPSAwOyBpIDwgdm9wLT5kYXRhLT53aW5fc2l6ZTsgaSsrKSB7Cj4gKwkJc3Ry dWN0IHZvcF93aW4gKnZvcF93aW4gPSAmdm9wLT53aW5baV07Cj4gKwkJY29uc3Qgc3RydWN0IHZv cF93aW5fZGF0YSAqd2luID0gdm9wX3dpbi0+ZGF0YTsKPiArCj4gKwkJVk9QX1dJTl9TRVQodm9w LCB3aW4sIHlyZ2JfbXN0LCAweDApOwo+ICsJCVZPUF9XSU5fU0VUKHZvcCwgd2luLCB1dl9tc3Qs IDB4MCk7Cj4gKwl9Cj4gKwpIaSBUb21ldQoKVGhhbmtzIGZvciB5b3VyIGZpeC4KClNldCB5cmdi X21zdCBhbmQgdXZfbXN0IGlzIG5vdCBhIGdvb2QgaWRlYSwgYmVjYXVzZSAweDAgYWxzbyBpcyBh IG1lbW9yeSAKYnVmZmVyIGFkZHJlc3MsIGRkciB3aWxsIGFjY2VzcyB0aGUgMHgwIGJ1ZmZlci4K SSB0aGluayB5b3UgbWF5IGVuYWJsZSBEUk1fRkJERVZfRU1VTEFUSU9OLCB0aGUgMHgwIGFkZHJl c3MgaXMgaW9tbXUgCm1tYXBlZCBhZGRyZXNzIGZvciBmYmRldiwgc28geW91ciB0ZXN0IGNhbiB3 b3Jrcy4KYnV0IGlmIERSTV9GQkRFVl9FTVVMQVRJT04gaXMgbm90IGRlZmluZSwgbWF5IGJlIDB4 MCBhZGRyZXNzIGlzIAp1bm1tYXBlZCwgd291bGQgZ2V0IHRoZSBpb21tdSBjcmFzaC4KCkkgdGhp bmsgd2UgY2FuIHVzZSBhdG9taWMgZGlzYWJsZSBmdW5jdGlvbiBsaWtlIHRoaXM6CmZvcl9lYWNo X3BsYW5lX2luX3N0YXRlKG9sZF9zdGF0ZSwgcGxhbmUsIG9sZF9wbGFuZV9zdGF0ZSwgaSkgewog ICAgIGNvbnN0IHN0cnVjdCBkcm1fcGxhbmVfaGVscGVyX2Z1bmNzICpmdW5jczsKICAgICBmdW5j cyA9IHBsYW5lLT5oZWxwZXJfcHJpdmF0ZTsKICAgICBmdW5jcy0+YXRvbWljX2Rpc2FibGUocGxh bmUsIG9sZF9wbGFuZV9zdGF0ZSk7Cn0KCkJ1dCBJIHRoaW5rIHdlJ2QgYmV0dGVyIGZpbmQgd2h5 IHdlIG5lZWQgZG8gdGhpcyBoYWNrIGhlcmUuCgpEb2VzIHRoZSBvbGQgRkIgYWRkcmVzcyBpcyB1 bW1hcGVkIHdoZW4gY3J0YyBkaXNhYmxpbmc/IHdoeSBwbGFuZSBpcyBub3QgCmRpc2FibGVkPwoK VGhhbmtzLgoKPiAgIAlzcGluX2xvY2soJnZvcC0+cmVnX2xvY2spOwo+ICAgCj4gICAJVk9QX0NU UkxfU0VUKHZvcCwgc3RhbmRieSwgMCk7CgoKLS0gCu+8rWFyayBZYW8KCgpfX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0 CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3Rv cC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: mark.yao@rock-chips.com (Mark yao) Date: Sat, 19 Mar 2016 09:15:45 +0800 Subject: [PATCH] drm/rockchip: vop: Reset yrgb_mst when re-enabling In-Reply-To: <1458300147-6472-1-git-send-email-tomeu.vizoso@collabora.com> References: <1458300147-6472-1-git-send-email-tomeu.vizoso@collabora.com> Message-ID: <56ECA841.40803@rock-chips.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2016?03?18? 19:22, Tomeu Vizoso wrote: > When the VOP is re-enabled, it will start scanning right away the > framebuffers that were configured from the last time, even if those have > been destroyed already. To prevent the VOP from trying to access freed > memory, reset the registers that hold pointers to framebuffers right > after we can write to them, but before the VOP is awaken from standby. > > Signed-off-by: Tomeu Vizoso > Link: http://lkml.kernel.org/g/CAAObsKAv+05ih5U+=4kic_NsjGMhfxYheHR8xXXmacZs+p5SHw at mail.gmail.com > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index 5e57f5b2e4b0..0df91c28740b 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -429,6 +429,7 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop) > static void vop_enable(struct drm_crtc *crtc) > { > struct vop *vop = to_vop(crtc); > + int i; > int ret; > > if (vop->is_enabled) > @@ -476,6 +477,18 @@ static void vop_enable(struct drm_crtc *crtc) > */ > vop->is_enabled = true; > > + /* > + * Before turning the VOP completely on, unset the registers > + * containing FB addresses to avoid the HW start scanning old FBs. > + */ > + for (i = 0; i < vop->data->win_size; i++) { > + struct vop_win *vop_win = &vop->win[i]; > + const struct vop_win_data *win = vop_win->data; > + > + VOP_WIN_SET(vop, win, yrgb_mst, 0x0); > + VOP_WIN_SET(vop, win, uv_mst, 0x0); > + } > + Hi Tomeu Thanks for your fix. Set yrgb_mst and uv_mst is not a good idea, because 0x0 also is a memory buffer address, ddr will access the 0x0 buffer. I think you may enable DRM_FBDEV_EMULATION, the 0x0 address is iommu mmaped address for fbdev, so your test can works. but if DRM_FBDEV_EMULATION is not define, may be 0x0 address is unmmaped, would get the iommu crash. I think we can use atomic disable function like this: for_each_plane_in_state(old_state, plane, old_plane_state, i) { const struct drm_plane_helper_funcs *funcs; funcs = plane->helper_private; funcs->atomic_disable(plane, old_plane_state); } But I think we'd better find why we need do this hack here. Does the old FB address is ummaped when crtc disabling? why plane is not disabled? Thanks. > spin_lock(&vop->reg_lock); > > VOP_CTRL_SET(vop, standby, 0); -- ?ark Yao From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754513AbcCSBQK (ORCPT ); Fri, 18 Mar 2016 21:16:10 -0400 Received: from regular1.263xmail.com ([211.150.99.140]:50471 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754144AbcCSBP6 (ORCPT ); Fri, 18 Mar 2016 21:15:58 -0400 X-263anti-spam: KSV:0;BIG:0;ABS:1;DNS:0;ATT:0;SPF:S; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 1 X-SKE-CHECKED: 1 X-ADDR-CHECKED: 0 X-RL-SENDER: mark.yao@rock-chips.com X-FST-TO: linux-rockchip@lists.infradead.org X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: mark.yao@rock-chips.com X-UNIQUE-TAG: X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Message-ID: <56ECA841.40803@rock-chips.com> Date: Sat, 19 Mar 2016 09:15:45 +0800 From: Mark yao User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Tomeu Vizoso , linux-kernel@vger.kernel.org CC: David Airlie , Heiko Stuebner , dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org Subject: Re: [PATCH] drm/rockchip: vop: Reset yrgb_mst when re-enabling References: <1458300147-6472-1-git-send-email-tomeu.vizoso@collabora.com> In-Reply-To: <1458300147-6472-1-git-send-email-tomeu.vizoso@collabora.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016年03月18日 19:22, Tomeu Vizoso wrote: > When the VOP is re-enabled, it will start scanning right away the > framebuffers that were configured from the last time, even if those have > been destroyed already. To prevent the VOP from trying to access freed > memory, reset the registers that hold pointers to framebuffers right > after we can write to them, but before the VOP is awaken from standby. > > Signed-off-by: Tomeu Vizoso > Link: http://lkml.kernel.org/g/CAAObsKAv+05ih5U+=4kic_NsjGMhfxYheHR8xXXmacZs+p5SHw@mail.gmail.com > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index 5e57f5b2e4b0..0df91c28740b 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -429,6 +429,7 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop) > static void vop_enable(struct drm_crtc *crtc) > { > struct vop *vop = to_vop(crtc); > + int i; > int ret; > > if (vop->is_enabled) > @@ -476,6 +477,18 @@ static void vop_enable(struct drm_crtc *crtc) > */ > vop->is_enabled = true; > > + /* > + * Before turning the VOP completely on, unset the registers > + * containing FB addresses to avoid the HW start scanning old FBs. > + */ > + for (i = 0; i < vop->data->win_size; i++) { > + struct vop_win *vop_win = &vop->win[i]; > + const struct vop_win_data *win = vop_win->data; > + > + VOP_WIN_SET(vop, win, yrgb_mst, 0x0); > + VOP_WIN_SET(vop, win, uv_mst, 0x0); > + } > + Hi Tomeu Thanks for your fix. Set yrgb_mst and uv_mst is not a good idea, because 0x0 also is a memory buffer address, ddr will access the 0x0 buffer. I think you may enable DRM_FBDEV_EMULATION, the 0x0 address is iommu mmaped address for fbdev, so your test can works. but if DRM_FBDEV_EMULATION is not define, may be 0x0 address is unmmaped, would get the iommu crash. I think we can use atomic disable function like this: for_each_plane_in_state(old_state, plane, old_plane_state, i) { const struct drm_plane_helper_funcs *funcs; funcs = plane->helper_private; funcs->atomic_disable(plane, old_plane_state); } But I think we'd better find why we need do this hack here. Does the old FB address is ummaped when crtc disabling? why plane is not disabled? Thanks. > spin_lock(&vop->reg_lock); > > VOP_CTRL_SET(vop, standby, 0); -- Ｍark Yao