[PATCH v2 06/22] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
Hi Marek,
2015ë
12ì 14ì¼ 18:15ì Marek Szyprowski ì´(ê°) ì´ ê¸:
> Hi Inki,
>
> On 2015-12-11 15:52, Inki Dae wrote:
>> 2015-12-11 20:27 GMT+09:00 Marek Szyprowski :
>>> On 2015-12-11 10:57, Inki Dae wrote:
2015ë
12ì 11ì¼ 18:26ì Marek Szyprowski ì´(ê°) ì´ ê¸:
> On 2015-12-11 10:02, Inki Dae wrote:
>> I found out why NULL point access happened. That was incurred by below
>> your patch,
>> [PATCH] drm/exynos: move dma_addr attribute from exynos plane to exynos
>> fb
>>
>> When another crtc driver is hotplugged in runtime such as HDMI or VIDI,
>> the drm frambuffer object of fb_helper is set to the plane state of the
>> new crtc driver
>> so the driver should get the drm framebuffer object from the plane's
>> state or
>> exynos_plane->pending_fb which is set by exynos_plane_atomic_update
>> function.
>>
>> After that, I think the drm framebuffer should be set to drm plane as a
>> current fb
>> which would be scanned out.
>>
>> Anyway, I can fix it like below if you are ok.
>>
>> Thanks,
>> Inki Dae
>>
>> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> @@ -137,7 +137,7 @@ static void vidi_update_plane(struct exynos_drm_crtc
>> *crtc,
>>if (ctx->suspended)
>>return;
>> - addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>> + addr = exynos_drm_fb_dma_addr(plane->pending_fb, 0);
>>DRM_DEBUG_KMS("dma_addr = %pad\n", );
>>
>> +plane->base.fb = plane->pending_fb;
> plane->base.fb should not be modified. I think that the following fix is
> more
Could you explain why plane->base.fb shouldn't be modified?
>>>
>>> All 'base' classes are modified by DRM core and there should be no need
>>> to modify them from the drivers.
>> Ok, If so - drm core updates plane->fb - then it's reasonable. But I
>> couldn't find the exact location where plane->fb is set to a fb to be
>> scanned out.
>> So could you let me know the exact location? it's not clear to me yet.
>
> Setting plane->fb is wired somewhere in the atomic update logic, see
> __setplane_internal() function in drivers/gpu/drm/drm_crtc.c. Some more
> comments
This function wouldn't be related to what we are talking about. But...
> are also in the drm_atomic_clean_old_fb() function in
> drivers/gpu/drm/drm_atomic.c
Right. Seems that this function is called after exynos_plane_atomic_update
function
call by atomic_update callback. So drm core updates plane->fb appropriately.
Thanks,
Inki Dae.
> However I don't know the exact call path.
>
>>> I've checked this issue and the proper fix for is the following code:
>>>
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>>> @@ -131,13 +131,14 @@ static void vidi_disable_vblank(struct exynos_drm_crtc
>>> *crtc)
>>> static void vidi_update_plane(struct exynos_drm_crtc *crtc,
>>>struct exynos_drm_plane *plane)
>>> {
>>> + struct drm_plane_state *state = plane->base.state;
>>> struct vidi_context *ctx = crtc->ctx;
>>> dma_addr_t addr;
>>>
>>> if (ctx->suspended)
>>> return;
>>>
>>> - addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>>> + addr = exynos_drm_fb_dma_addr(state->fb, 0);
>>> DRM_DEBUG_KMS("dma_addr = %pad\n", );
>>>
>>> if (ctx->vblank_on)
>>>
>>>
>>> plane->base.fb is updated from the core once all crtc/plane atomic update
>>> calls finishes. The drivers should use the fb stored in plane->base.state.
>>> I've missed that VIDI driver doesn't get the fb and incorrectly used
>>> plane->base.fb instad of state->fb while updating the code.
>>>
>> Actually, I used state->fb instead of plane->pending_fb but in
>> exynos_plane_atomic_update function, plane->pending_fb is set to
>> state->fb.
>> That is why I commented like below,
>> " so the driver should get the drm framebuffer object from the plane's
>> state or exynos_plane->pending_fb which is set by
>> exynos_plane_atomic_update function."
>>
>> Anyway, using state->fb looks like more consistent with other drivers,
>> fimd, decon and mixer.
>
> Thanks for applying my fix and merging this patch.
>
In case that userspace requests setplane, plane->base.fb would be updated
after update_plane callback.
However, in other cases, I don't see how plane->base.fb could be updated.
In this case, I think vendor specific drivers would need to update it as a
current fb to be scanned out like other some drivers did.
Of course, it may be possible for drm core part to take care of it
appropriately later.
Thanks,
Inki Dae
> appropriate:
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -132,12 +132,13 @@
[PATCH v2 06/22] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
Hi Inki,
On 2015-12-11 15:52, Inki Dae wrote:
> 2015-12-11 20:27 GMT+09:00 Marek Szyprowski :
>> On 2015-12-11 10:57, Inki Dae wrote:
>>> 2015ë
12ì 11ì¼ 18:26ì Marek Szyprowski ì´(ê°) ì´ ê¸:
On 2015-12-11 10:02, Inki Dae wrote:
> I found out why NULL point access happened. That was incurred by below
> your patch,
> [PATCH] drm/exynos: move dma_addr attribute from exynos plane to exynos
> fb
>
> When another crtc driver is hotplugged in runtime such as HDMI or VIDI,
> the drm frambuffer object of fb_helper is set to the plane state of the
> new crtc driver
> so the driver should get the drm framebuffer object from the plane's
> state or
> exynos_plane->pending_fb which is set by exynos_plane_atomic_update
> function.
>
> After that, I think the drm framebuffer should be set to drm plane as a
> current fb
> which would be scanned out.
>
> Anyway, I can fix it like below if you are ok.
>
> Thanks,
> Inki Dae
>
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -137,7 +137,7 @@ static void vidi_update_plane(struct exynos_drm_crtc
> *crtc,
>if (ctx->suspended)
>return;
> - addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
> + addr = exynos_drm_fb_dma_addr(plane->pending_fb, 0);
>DRM_DEBUG_KMS("dma_addr = %pad\n", );
>
> +plane->base.fb = plane->pending_fb;
plane->base.fb should not be modified. I think that the following fix is
more
>>> Could you explain why plane->base.fb shouldn't be modified?
>>
>> All 'base' classes are modified by DRM core and there should be no need
>> to modify them from the drivers.
> Ok, If so - drm core updates plane->fb - then it's reasonable. But I
> couldn't find the exact location where plane->fb is set to a fb to be
> scanned out.
> So could you let me know the exact location? it's not clear to me yet.
Setting plane->fb is wired somewhere in the atomic update logic, see
__setplane_internal() function in drivers/gpu/drm/drm_crtc.c. Some more
comments
are also in the drm_atomic_clean_old_fb() function in
drivers/gpu/drm/drm_atomic.c
However I don't know the exact call path.
>> I've checked this issue and the proper fix for is the following code:
>>
>> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> @@ -131,13 +131,14 @@ static void vidi_disable_vblank(struct exynos_drm_crtc
>> *crtc)
>> static void vidi_update_plane(struct exynos_drm_crtc *crtc,
>>struct exynos_drm_plane *plane)
>> {
>> + struct drm_plane_state *state = plane->base.state;
>> struct vidi_context *ctx = crtc->ctx;
>> dma_addr_t addr;
>>
>> if (ctx->suspended)
>> return;
>>
>> - addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>> + addr = exynos_drm_fb_dma_addr(state->fb, 0);
>> DRM_DEBUG_KMS("dma_addr = %pad\n", );
>>
>> if (ctx->vblank_on)
>>
>>
>> plane->base.fb is updated from the core once all crtc/plane atomic update
>> calls finishes. The drivers should use the fb stored in plane->base.state.
>> I've missed that VIDI driver doesn't get the fb and incorrectly used
>> plane->base.fb instad of state->fb while updating the code.
>>
> Actually, I used state->fb instead of plane->pending_fb but in
> exynos_plane_atomic_update function, plane->pending_fb is set to
> state->fb.
> That is why I commented like below,
> " so the driver should get the drm framebuffer object from the plane's
> state or exynos_plane->pending_fb which is set by
> exynos_plane_atomic_update function."
>
> Anyway, using state->fb looks like more consistent with other drivers,
> fimd, decon and mixer.
Thanks for applying my fix and merging this patch.
>>> In case that userspace requests setplane, plane->base.fb would be updated
>>> after update_plane callback.
>>> However, in other cases, I don't see how plane->base.fb could be updated.
>>> In this case, I think vendor specific drivers would need to update it as a
>>> current fb to be scanned out like other some drivers did.
>>> Of course, it may be possible for drm core part to take care of it
>>> appropriately later.
>>>
>>> Thanks,
>>> Inki Dae
>>>
appropriate:
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -132,12 +132,13 @@ static void vidi_update_plane(struct
exynos_drm_crtc *crtc,
struct exynos_drm_plane *plane)
{
struct vidi_context *ctx = crtc->ctx;
- dma_addr_t addr;
+ dma_addr_t addr = 0;
if (ctx->suspended)
return;
- addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
+ if (plane->base.fb)
+ addr =
[PATCH v2 06/22] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
Hi Marek,
2015-12-11 20:27 GMT+09:00 Marek Szyprowski :
> Hi Inki,
>
>
> On 2015-12-11 10:57, Inki Dae wrote:
>>
>> Hi Marek,
>>
>> 2015ë
12ì 11ì¼ 18:26ì Marek Szyprowski ì´(ê°) ì´ ê¸:
>>>
>>> Hi Inki,
>>>
>>> On 2015-12-11 10:02, Inki Dae wrote:
Hi Marek,
I found out why NULL point access happened. That was incurred by below
your patch,
[PATCH] drm/exynos: move dma_addr attribute from exynos plane to exynos
fb
When another crtc driver is hotplugged in runtime such as HDMI or VIDI,
the drm frambuffer object of fb_helper is set to the plane state of the
new crtc driver
so the driver should get the drm framebuffer object from the plane's
state or
exynos_plane->pending_fb which is set by exynos_plane_atomic_update
function.
After that, I think the drm framebuffer should be set to drm plane as a
current fb
which would be scanned out.
Anyway, I can fix it like below if you are ok.
Thanks,
Inki Dae
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -137,7 +137,7 @@ static void vidi_update_plane(struct exynos_drm_crtc
*crtc,
if (ctx->suspended)
return;
- addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
+ addr = exynos_drm_fb_dma_addr(plane->pending_fb, 0);
DRM_DEBUG_KMS("dma_addr = %pad\n", );
+plane->base.fb = plane->pending_fb;
>>>
>>> plane->base.fb should not be modified. I think that the following fix is
>>> more
>>
>> Could you explain why plane->base.fb shouldn't be modified?
>
>
> All 'base' classes are modified by DRM core and there should be no need
> to modify them from the drivers.
Ok, If so - drm core updates plane->fb - then it's reasonable. But I
couldn't find the exact location where plane->fb is set to a fb to be
scanned out.
So could you let me know the exact location? it's not clear to me yet.
>
> I've checked this issue and the proper fix for is the following code:
>
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -131,13 +131,14 @@ static void vidi_disable_vblank(struct exynos_drm_crtc
> *crtc)
> static void vidi_update_plane(struct exynos_drm_crtc *crtc,
> struct exynos_drm_plane *plane)
> {
> + struct drm_plane_state *state = plane->base.state;
> struct vidi_context *ctx = crtc->ctx;
> dma_addr_t addr;
>
> if (ctx->suspended)
> return;
>
> - addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
> + addr = exynos_drm_fb_dma_addr(state->fb, 0);
> DRM_DEBUG_KMS("dma_addr = %pad\n", );
>
> if (ctx->vblank_on)
>
>
> plane->base.fb is updated from the core once all crtc/plane atomic update
> calls finishes. The drivers should use the fb stored in plane->base.state.
> I've missed that VIDI driver doesn't get the fb and incorrectly used
> plane->base.fb instad of state->fb while updating the code.
>
Actually, I used state->fb instead of plane->pending_fb but in
exynos_plane_atomic_update function, plane->pending_fb is set to
state->fb.
That is why I commented like below,
" so the driver should get the drm framebuffer object from the plane's
state or exynos_plane->pending_fb which is set by
exynos_plane_atomic_update function."
Anyway, using state->fb looks like more consistent with other drivers,
fimd, decon and mixer.
Thanks,
Inki Dae
>
>
>> In case that userspace requests setplane, plane->base.fb would be updated
>> after update_plane callback.
>> However, in other cases, I don't see how plane->base.fb could be updated.
>> In this case, I think vendor specific drivers would need to update it as a
>> current fb to be scanned out like other some drivers did.
>> Of course, it may be possible for drm core part to take care of it
>> appropriately later.
>>
>> Thanks,
>> Inki Dae
>>
>>> appropriate:
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>>> @@ -132,12 +132,13 @@ static void vidi_update_plane(struct
>>> exynos_drm_crtc *crtc,
>>>struct exynos_drm_plane *plane)
>>> {
>>> struct vidi_context *ctx = crtc->ctx;
>>> - dma_addr_t addr;
>>> + dma_addr_t addr = 0;
>>>
>>> if (ctx->suspended)
>>> return;
>>>
>>> - addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>>> + if (plane->base.fb)
>>> + addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>>> DRM_DEBUG_KMS("dma_addr = %pad\n", );
>>>
>>> if (ctx->vblank_on)
>>>
>>> I will investigate the case of NULL plane->state.fb, because it might be
>>> relevant
>>> to other crtc drivers as well.
>>>
>>>
if (ctx->vblank_on)
2015ë
12ì 10ì¼ 22:05ì Inki Dae ì´(ê°) ì´ ê¸:
>
> 2015ë
[PATCH v2 06/22] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
Hi Marek,
2015ë
12ì 11ì¼ 18:26ì Marek Szyprowski ì´(ê°) ì´ ê¸:
> Hi Inki,
>
> On 2015-12-11 10:02, Inki Dae wrote:
>> Hi Marek,
>>
>> I found out why NULL point access happened. That was incurred by below your
>> patch,
>> [PATCH] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
>>
>> When another crtc driver is hotplugged in runtime such as HDMI or VIDI,
>> the drm frambuffer object of fb_helper is set to the plane state of the new
>> crtc driver
>> so the driver should get the drm framebuffer object from the plane's state or
>> exynos_plane->pending_fb which is set by exynos_plane_atomic_update function.
>>
>> After that, I think the drm framebuffer should be set to drm plane as a
>> current fb
>> which would be scanned out.
>>
>> Anyway, I can fix it like below if you are ok.
>>
>> Thanks,
>> Inki Dae
>>
>> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> @@ -137,7 +137,7 @@ static void vidi_update_plane(struct exynos_drm_crtc
>> *crtc,
>> if (ctx->suspended)
>> return;
>> - addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>> + addr = exynos_drm_fb_dma_addr(plane->pending_fb, 0);
>> DRM_DEBUG_KMS("dma_addr = %pad\n", );
>>
>> +plane->base.fb = plane->pending_fb;
>
> plane->base.fb should not be modified. I think that the following fix is more
Could you explain why plane->base.fb shouldn't be modified?
In case that userspace requests setplane, plane->base.fb would be updated after
update_plane callback.
However, in other cases, I don't see how plane->base.fb could be updated.
In this case, I think vendor specific drivers would need to update it as a
current fb to be scanned out like other some drivers did.
Of course, it may be possible for drm core part to take care of it
appropriately later.
Thanks,
Inki Dae
> appropriate:
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -132,12 +132,13 @@ static void vidi_update_plane(struct exynos_drm_crtc
> *crtc,
> struct exynos_drm_plane *plane)
> {
> struct vidi_context *ctx = crtc->ctx;
> - dma_addr_t addr;
> + dma_addr_t addr = 0;
>
> if (ctx->suspended)
> return;
>
> - addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
> + if (plane->base.fb)
> + addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
> DRM_DEBUG_KMS("dma_addr = %pad\n", );
>
> if (ctx->vblank_on)
>
> I will investigate the case of NULL plane->state.fb, because it might be
> relevant
> to other crtc drivers as well.
>
>
>>if (ctx->vblank_on)
>>
>>
>> 2015ë
12ì 10ì¼ 22:05ì Inki Dae ì´(ê°) ì´ ê¸:
>>>
>>> 2015ë
11ì 30ì¼ 22:53ì Marek Szyprowski ì´(ê°) ì´ ê¸:
DMA address is a framebuffer attribute and the right place for it is
exynos_drm_framebuffer not exynos_drm_plane. This patch also introduces
helper function for getting dma address of the given framebuffer.
Signed-off-by: Marek Szyprowski
Reviewed-by: Gustavo Padovan
---
drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 13 -
drivers/gpu/drm/exynos/exynos7_drm_decon.c| 16 +---
drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 ---
drivers/gpu/drm/exynos/exynos_drm_fb.c| 16 ++--
drivers/gpu/drm/exynos/exynos_drm_fb.h| 3 +--
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 10 ++
drivers/gpu/drm/exynos/exynos_drm_plane.c | 18 --
drivers/gpu/drm/exynos/exynos_drm_vidi.c | 5 -
drivers/gpu/drm/exynos/exynos_mixer.c | 7 ---
9 files changed, 38 insertions(+), 53 deletions(-)
>>> <--snip-->
>>>
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index 669362c53f49..3ce141236fad 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -24,6 +24,7 @@
#include "exynos_drm_drv.h"
#include "exynos_drm_crtc.h"
+#include "exynos_drm_fb.h"
#include "exynos_drm_plane.h"
#include "exynos_drm_vidi.h"
@@ -126,11 +127,13 @@ static void vidi_update_plane(struct
exynos_drm_crtc *crtc,
struct exynos_drm_plane *plane)
{
struct vidi_context *ctx = crtc->ctx;
+dma_addr_t addr;
if (ctx->suspended)
return;
-DRM_DEBUG_KMS("dma_addr = %pad\n", plane->dma_addr);
+addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>>> At this point, plane->base.fb is NULL so null pointer access happens like
>>> below,
>>>
>>> [5.969422] Unable to handle kernel NULL pointer dereference at virtual
>>> address 0090
>>> [5.977481] pgd = ee59
>>> [
[PATCH v2 06/22] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
Hi Marek,
I found out why NULL point access happened. That was incurred by below your
patch,
[PATCH] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
When another crtc driver is hotplugged in runtime such as HDMI or VIDI,
the drm frambuffer object of fb_helper is set to the plane state of the new
crtc driver
so the driver should get the drm framebuffer object from the plane's state or
exynos_plane->pending_fb which is set by exynos_plane_atomic_update function.
After that, I think the drm framebuffer should be set to drm plane as a current
fb
which would be scanned out.
Anyway, I can fix it like below if you are ok.
Thanks,
Inki Dae
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -137,7 +137,7 @@ static void vidi_update_plane(struct exynos_drm_crtc *crtc,
if (ctx->suspended)
return;
- addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
+ addr = exynos_drm_fb_dma_addr(plane->pending_fb, 0);
DRM_DEBUG_KMS("dma_addr = %pad\n", );
+ plane->base.fb = plane->pending_fb;
if (ctx->vblank_on)
2015ë
12ì 10ì¼ 22:05ì Inki Dae ì´(ê°) ì´ ê¸:
>
>
> 2015ë
11ì 30ì¼ 22:53ì Marek Szyprowski ì´(ê°) ì´ ê¸:
>> DMA address is a framebuffer attribute and the right place for it is
>> exynos_drm_framebuffer not exynos_drm_plane. This patch also introduces
>> helper function for getting dma address of the given framebuffer.
>>
>> Signed-off-by: Marek Szyprowski
>> Reviewed-by: Gustavo Padovan
>> ---
>> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 13 -
>> drivers/gpu/drm/exynos/exynos7_drm_decon.c| 16 +---
>> drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 ---
>> drivers/gpu/drm/exynos/exynos_drm_fb.c| 16 ++--
>> drivers/gpu/drm/exynos/exynos_drm_fb.h| 3 +--
>> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 10 ++
>> drivers/gpu/drm/exynos/exynos_drm_plane.c | 18 --
>> drivers/gpu/drm/exynos/exynos_drm_vidi.c | 5 -
>> drivers/gpu/drm/exynos/exynos_mixer.c | 7 ---
>> 9 files changed, 38 insertions(+), 53 deletions(-)
>>
>
> <--snip-->
>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> index 669362c53f49..3ce141236fad 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> @@ -24,6 +24,7 @@
>>
>> #include "exynos_drm_drv.h"
>> #include "exynos_drm_crtc.h"
>> +#include "exynos_drm_fb.h"
>> #include "exynos_drm_plane.h"
>> #include "exynos_drm_vidi.h"
>>
>> @@ -126,11 +127,13 @@ static void vidi_update_plane(struct exynos_drm_crtc
>> *crtc,
>>struct exynos_drm_plane *plane)
>> {
>> struct vidi_context *ctx = crtc->ctx;
>> +dma_addr_t addr;
>>
>> if (ctx->suspended)
>> return;
>>
>> -DRM_DEBUG_KMS("dma_addr = %pad\n", plane->dma_addr);
>> +addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>
> At this point, plane->base.fb is NULL so null pointer access happens like
> below,
>
> [5.969422] Unable to handle kernel NULL pointer dereference at virtual
> address 0090
> [5.977481] pgd = ee59
> [5.980142] [0090] *pgd=6e526831, *pte=, *ppte=
> [5.986347] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
> [5.991712] Modules linked in:
> [5.994770] CPU: 3 PID: 1598 Comm: sh Not tainted
> 4.4.0-rc3-00052-gc60d7e2-dirty #199
> [6.002565] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [6.008647] task: ef328000 ti: ee4d4000 task.ti: ee4d4000
> [6.014053] PC is at exynos_drm_fb_dma_addr+0x8/0x14
> [6.018990] LR is at vidi_update_plane+0x4c/0xc4
> [6.023581] pc : []lr : []psr: 8013
> [6.023581] sp : ee4d5d90 ip : 0001 fp :
> [6.035029] r10: r9 : c05b965c r8 : ee813e00
> [6.040241] r7 : r6 : ee8e3330 r5 : r4 : ee8e3010
> [6.046749] r3 : r2 : r1 : 0024 r0 :
> [6.053264] Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment
> none
> [6.060379] Control: 10c5387d Table: 6e59004a DAC: 0051
> [6.066107] Process sh (pid: 1598, stack limit = 0xee4d4210)
> [6.071748] Stack: (0xee4d5d90 to 0xee4d6000)
> [6.076100] 5d80: ee813300
> ee476e40 0005
> [6.084236] 5da0: ee8e3330 c028ac14 0008 ee476e40 ee476fc0 ef3b3800
> ee476fc0 eeb3e380
> [6.092395] 5dc0: 0002 c02b08e4 eeb3e3a4 ee476fc0 ee476e40
> ef3b3800 eeb3e380
> [6.100554] 5de0: 0002 c02b12b8 ee854400 0001 ee8501a8
> ee476e40
> [6.108714] 5e00: ef3b3800 0001 ee476e40 0050 ee850c80 0001
> ee476e40 ef3b3aac
> [6.116873] 5e20: 0002 00ff c028e0ec 000a82b4 c02acc50
> ee8e36a0 ee476c80
[PATCH v2 06/22] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
Hi Inki,
On 2015-12-11 10:57, Inki Dae wrote:
> Hi Marek,
>
> 2015ë
12ì 11ì¼ 18:26ì Marek Szyprowski ì´(ê°) ì´ ê¸:
>> Hi Inki,
>>
>> On 2015-12-11 10:02, Inki Dae wrote:
>>> Hi Marek,
>>>
>>> I found out why NULL point access happened. That was incurred by below your
>>> patch,
>>> [PATCH] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
>>>
>>> When another crtc driver is hotplugged in runtime such as HDMI or VIDI,
>>> the drm frambuffer object of fb_helper is set to the plane state of the new
>>> crtc driver
>>> so the driver should get the drm framebuffer object from the plane's state
>>> or
>>> exynos_plane->pending_fb which is set by exynos_plane_atomic_update
>>> function.
>>>
>>> After that, I think the drm framebuffer should be set to drm plane as a
>>> current fb
>>> which would be scanned out.
>>>
>>> Anyway, I can fix it like below if you are ok.
>>>
>>> Thanks,
>>> Inki Dae
>>>
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>>> @@ -137,7 +137,7 @@ static void vidi_update_plane(struct exynos_drm_crtc
>>> *crtc,
>>> if (ctx->suspended)
>>> return;
>>>- addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>>> + addr = exynos_drm_fb_dma_addr(plane->pending_fb, 0);
>>> DRM_DEBUG_KMS("dma_addr = %pad\n", );
>>>
>>> +plane->base.fb = plane->pending_fb;
>> plane->base.fb should not be modified. I think that the following fix is more
> Could you explain why plane->base.fb shouldn't be modified?
All 'base' classes are modified by DRM core and there should be no need
to modify them from the drivers.
I've checked this issue and the proper fix for is the following code:
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -131,13 +131,14 @@ static void vidi_disable_vblank(struct
exynos_drm_crtc *crtc)
static void vidi_update_plane(struct exynos_drm_crtc *crtc,
struct exynos_drm_plane *plane)
{
+ struct drm_plane_state *state = plane->base.state;
struct vidi_context *ctx = crtc->ctx;
dma_addr_t addr;
if (ctx->suspended)
return;
- addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
+ addr = exynos_drm_fb_dma_addr(state->fb, 0);
DRM_DEBUG_KMS("dma_addr = %pad\n", );
if (ctx->vblank_on)
plane->base.fb is updated from the core once all crtc/plane atomic update
calls finishes. The drivers should use the fb stored in plane->base.state.
I've missed that VIDI driver doesn't get the fb and incorrectly used
plane->base.fb instad of state->fb while updating the code.
> In case that userspace requests setplane, plane->base.fb would be updated
> after update_plane callback.
> However, in other cases, I don't see how plane->base.fb could be updated.
> In this case, I think vendor specific drivers would need to update it as a
> current fb to be scanned out like other some drivers did.
> Of course, it may be possible for drm core part to take care of it
> appropriately later.
>
> Thanks,
> Inki Dae
>
>> appropriate:
>> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>> @@ -132,12 +132,13 @@ static void vidi_update_plane(struct exynos_drm_crtc
>> *crtc,
>>struct exynos_drm_plane *plane)
>> {
>> struct vidi_context *ctx = crtc->ctx;
>> - dma_addr_t addr;
>> + dma_addr_t addr = 0;
>>
>> if (ctx->suspended)
>> return;
>>
>> - addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>> + if (plane->base.fb)
>> + addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>> DRM_DEBUG_KMS("dma_addr = %pad\n", );
>>
>> if (ctx->vblank_on)
>>
>> I will investigate the case of NULL plane->state.fb, because it might be
>> relevant
>> to other crtc drivers as well.
>>
>>
>>> if (ctx->vblank_on)
>>>
>>>
>>> 2015ë
12ì 10ì¼ 22:05ì Inki Dae ì´(ê°) ì´ ê¸:
2015ë
11ì 30ì¼ 22:53ì Marek Szyprowski ì´(ê°) ì´ ê¸:
> DMA address is a framebuffer attribute and the right place for it is
> exynos_drm_framebuffer not exynos_drm_plane. This patch also introduces
> helper function for getting dma address of the given framebuffer.
>
> Signed-off-by: Marek Szyprowski
> Reviewed-by: Gustavo Padovan
> ---
>drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 13 -
>drivers/gpu/drm/exynos/exynos7_drm_decon.c| 16 +---
>drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 ---
>drivers/gpu/drm/exynos/exynos_drm_fb.c| 16 ++--
>drivers/gpu/drm/exynos/exynos_drm_fb.h| 3 +--
>drivers/gpu/drm/exynos/exynos_drm_fimd.c | 10 ++
>drivers/gpu/drm/exynos/exynos_drm_plane.c | 18 --
>
[PATCH v2 06/22] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
Hi Inki,
On 2015-12-11 10:02, Inki Dae wrote:
> Hi Marek,
>
> I found out why NULL point access happened. That was incurred by below your
> patch,
> [PATCH] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
>
> When another crtc driver is hotplugged in runtime such as HDMI or VIDI,
> the drm frambuffer object of fb_helper is set to the plane state of the new
> crtc driver
> so the driver should get the drm framebuffer object from the plane's state or
> exynos_plane->pending_fb which is set by exynos_plane_atomic_update function.
>
> After that, I think the drm framebuffer should be set to drm plane as a
> current fb
> which would be scanned out.
>
> Anyway, I can fix it like below if you are ok.
>
> Thanks,
> Inki Dae
>
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -137,7 +137,7 @@ static void vidi_update_plane(struct exynos_drm_crtc
> *crtc,
> if (ctx->suspended)
> return;
>
> - addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
> + addr = exynos_drm_fb_dma_addr(plane->pending_fb, 0);
> DRM_DEBUG_KMS("dma_addr = %pad\n", );
>
> + plane->base.fb = plane->pending_fb;
plane->base.fb should not be modified. I think that the following fix is
more
appropriate:
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -132,12 +132,13 @@ static void vidi_update_plane(struct
exynos_drm_crtc *crtc,
struct exynos_drm_plane *plane)
{
struct vidi_context *ctx = crtc->ctx;
- dma_addr_t addr;
+ dma_addr_t addr = 0;
if (ctx->suspended)
return;
- addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
+ if (plane->base.fb)
+ addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
DRM_DEBUG_KMS("dma_addr = %pad\n", );
if (ctx->vblank_on)
I will investigate the case of NULL plane->state.fb, because it might be
relevant
to other crtc drivers as well.
>
> if (ctx->vblank_on)
>
>
> 2015ë
12ì 10ì¼ 22:05ì Inki Dae ì´(ê°) ì´ ê¸:
>>
>> 2015ë
11ì 30ì¼ 22:53ì Marek Szyprowski ì´(ê°) ì´ ê¸:
>>> DMA address is a framebuffer attribute and the right place for it is
>>> exynos_drm_framebuffer not exynos_drm_plane. This patch also introduces
>>> helper function for getting dma address of the given framebuffer.
>>>
>>> Signed-off-by: Marek Szyprowski
>>> Reviewed-by: Gustavo Padovan
>>> ---
>>> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 13 -
>>> drivers/gpu/drm/exynos/exynos7_drm_decon.c| 16 +---
>>> drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 ---
>>> drivers/gpu/drm/exynos/exynos_drm_fb.c| 16 ++--
>>> drivers/gpu/drm/exynos/exynos_drm_fb.h| 3 +--
>>> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 10 ++
>>> drivers/gpu/drm/exynos/exynos_drm_plane.c | 18 --
>>> drivers/gpu/drm/exynos/exynos_drm_vidi.c | 5 -
>>> drivers/gpu/drm/exynos/exynos_mixer.c | 7 ---
>>> 9 files changed, 38 insertions(+), 53 deletions(-)
>>>
>> <--snip-->
>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>>> index 669362c53f49..3ce141236fad 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
>>> @@ -24,6 +24,7 @@
>>>
>>> #include "exynos_drm_drv.h"
>>> #include "exynos_drm_crtc.h"
>>> +#include "exynos_drm_fb.h"
>>> #include "exynos_drm_plane.h"
>>> #include "exynos_drm_vidi.h"
>>>
>>> @@ -126,11 +127,13 @@ static void vidi_update_plane(struct exynos_drm_crtc
>>> *crtc,
>>> struct exynos_drm_plane *plane)
>>> {
>>> struct vidi_context *ctx = crtc->ctx;
>>> + dma_addr_t addr;
>>>
>>> if (ctx->suspended)
>>> return;
>>>
>>> - DRM_DEBUG_KMS("dma_addr = %pad\n", plane->dma_addr);
>>> + addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
>> At this point, plane->base.fb is NULL so null pointer access happens like
>> below,
>>
>> [5.969422] Unable to handle kernel NULL pointer dereference at virtual
>> address 0090
>> [5.977481] pgd = ee59
>> [5.980142] [0090] *pgd=6e526831, *pte=, *ppte=
>> [5.986347] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
>> [5.991712] Modules linked in:
>> [5.994770] CPU: 3 PID: 1598 Comm: sh Not tainted
>> 4.4.0-rc3-00052-gc60d7e2-dirty #199
>> [6.002565] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>> [6.008647] task: ef328000 ti: ee4d4000 task.ti: ee4d4000
>> [6.014053] PC is at exynos_drm_fb_dma_addr+0x8/0x14
>> [6.018990] LR is at vidi_update_plane+0x4c/0xc4
>> [6.023581] pc : []lr : []psr: 8013
>> [6.023581] sp : ee4d5d90 ip : 0001 fp :
>> [6.035029] r10: r9 :
[PATCH v2 06/22] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
2015ë
11ì 30ì¼ 22:53ì Marek Szyprowski ì´(ê°) ì´ ê¸:
> DMA address is a framebuffer attribute and the right place for it is
> exynos_drm_framebuffer not exynos_drm_plane. This patch also introduces
> helper function for getting dma address of the given framebuffer.
>
> Signed-off-by: Marek Szyprowski
> Reviewed-by: Gustavo Padovan
> ---
> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 13 -
> drivers/gpu/drm/exynos/exynos7_drm_decon.c| 16 +---
> drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 ---
> drivers/gpu/drm/exynos/exynos_drm_fb.c| 16 ++--
> drivers/gpu/drm/exynos/exynos_drm_fb.h| 3 +--
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 10 ++
> drivers/gpu/drm/exynos/exynos_drm_plane.c | 18 --
> drivers/gpu/drm/exynos/exynos_drm_vidi.c | 5 -
> drivers/gpu/drm/exynos/exynos_mixer.c | 7 ---
> 9 files changed, 38 insertions(+), 53 deletions(-)
>
<--snip-->
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> index 669362c53f49..3ce141236fad 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -24,6 +24,7 @@
>
> #include "exynos_drm_drv.h"
> #include "exynos_drm_crtc.h"
> +#include "exynos_drm_fb.h"
> #include "exynos_drm_plane.h"
> #include "exynos_drm_vidi.h"
>
> @@ -126,11 +127,13 @@ static void vidi_update_plane(struct exynos_drm_crtc
> *crtc,
> struct exynos_drm_plane *plane)
> {
> struct vidi_context *ctx = crtc->ctx;
> + dma_addr_t addr;
>
> if (ctx->suspended)
> return;
>
> - DRM_DEBUG_KMS("dma_addr = %pad\n", plane->dma_addr);
> + addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
At this point, plane->base.fb is NULL so null pointer access happens like below,
[5.969422] Unable to handle kernel NULL pointer dereference at virtual
address 0090
[5.977481] pgd = ee59
[5.980142] [0090] *pgd=6e526831, *pte=, *ppte=
[5.986347] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
[5.991712] Modules linked in:
[5.994770] CPU: 3 PID: 1598 Comm: sh Not tainted
4.4.0-rc3-00052-gc60d7e2-dirty #199
[6.002565] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[6.008647] task: ef328000 ti: ee4d4000 task.ti: ee4d4000
[6.014053] PC is at exynos_drm_fb_dma_addr+0x8/0x14
[6.018990] LR is at vidi_update_plane+0x4c/0xc4
[6.023581] pc : []lr : []psr: 8013
[6.023581] sp : ee4d5d90 ip : 0001 fp :
[6.035029] r10: r9 : c05b965c r8 : ee813e00
[6.040241] r7 : r6 : ee8e3330 r5 : r4 : ee8e3010
[6.046749] r3 : r2 : r1 : 0024 r0 :
[6.053264] Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[6.060379] Control: 10c5387d Table: 6e59004a DAC: 0051
[6.066107] Process sh (pid: 1598, stack limit = 0xee4d4210)
[6.071748] Stack: (0xee4d5d90 to 0xee4d6000)
[6.076100] 5d80: ee813300
ee476e40 0005
[6.084236] 5da0: ee8e3330 c028ac14 0008 ee476e40 ee476fc0 ef3b3800
ee476fc0 eeb3e380
[6.092395] 5dc0: 0002 c02b08e4 eeb3e3a4 ee476fc0 ee476e40
ef3b3800 eeb3e380
[6.100554] 5de0: 0002 c02b12b8 ee854400 0001 ee8501a8
ee476e40
[6.108714] 5e00: ef3b3800 0001 ee476e40 0050 ee850c80 0001
ee476e40 ef3b3aac
[6.116873] 5e20: 0002 00ff c028e0ec 000a82b4 c02acc50
ee8e36a0 ee476c80
[6.125032] 5e40: ef3b3aac ef3b3800 ee476c9c ee850c80 ef3b3800 ef3b3800
ef3b3800 ef3b398c
[6.133191] 5e60: c088c390 0002 000a82b4 c028f8d4 ef3b3800
ef0f4300 c028f948
[6.141350] 5e80: ee850c80 c028f864 ef3b3a84 0001 ef3b3a90 c02853e4
0001
[6.149509] 5ea0: 000a82b4 ee4d5ec0 0002 ee8e3010 0002 0002
ee4d5f88
[6.157669] 5ec0: eeb8df00 000a82b4 c02c4278 0002 ee476b00
eeb8df0c c01390ac
[6.165828] 5ee0: ee4e1f00 0002 000a9540 ee4d5f88
c000f844 ee4d4000
[6.173987] 5f00: c00dbf70 000a82b4 c00093dc ee4d4000 ee4d5f78
ef328234 c0579bec
[6.182146] 5f20: 0001 0001 ee4d5f3c 0001 ee45e9c4 0001
000a82b4 c005ca74
[6.190306] 5f40: ee45e9c4 0002 000a9540 c005cad4 ee4e1f00 0002
000a9540 ee4d5f88
[6.198465] 5f60: c000f844 c00dc770 ee4e1f00 ee4e1f00
0002 000a9540
[6.206624] 5f80: c000f844 c00dcf98 0003 000a7c40
0001 000a9540
[6.214783] 5fa0: 0004 c000f680 000a7c40 0001 0001 000a9540
0002
[6.222942] 5fc0: 000a7c40 0001 000a9540 0004 0020 000a82c8
000a8294 000a82b4
[6.231102] 5fe0: be8b1624 00012345 b6e94166 4030 0001
[PATCH v2 06/22] drm/exynos: move dma_addr attribute from exynos plane to exynos fb
DMA address is a framebuffer attribute and the right place for it is
exynos_drm_framebuffer not exynos_drm_plane. This patch also introduces
helper function for getting dma address of the given framebuffer.
Signed-off-by: Marek Szyprowski
Reviewed-by: Gustavo Padovan
---
drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 13 -
drivers/gpu/drm/exynos/exynos7_drm_decon.c| 16 +---
drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 ---
drivers/gpu/drm/exynos/exynos_drm_fb.c| 16 ++--
drivers/gpu/drm/exynos/exynos_drm_fb.h| 3 +--
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 10 ++
drivers/gpu/drm/exynos/exynos_drm_plane.c | 18 --
drivers/gpu/drm/exynos/exynos_drm_vidi.c | 5 -
drivers/gpu/drm/exynos/exynos_mixer.c | 7 ---
9 files changed, 38 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index edfd6e390ef7..320efc3d0659 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -21,6 +21,7 @@
#include "exynos_drm_drv.h"
#include "exynos_drm_crtc.h"
+#include "exynos_drm_fb.h"
#include "exynos_drm_plane.h"
#include "exynos_drm_iommu.h"
@@ -261,9 +262,11 @@ static void decon_update_plane(struct exynos_drm_crtc
*crtc,
{
struct decon_context *ctx = crtc->ctx;
struct drm_plane_state *state = plane->base.state;
+ struct drm_framebuffer *fb = state->fb;
unsigned int win = plane->zpos;
- unsigned int bpp = state->fb->bits_per_pixel >> 3;
- unsigned int pitch = state->fb->pitches[0];
+ unsigned int bpp = fb->bits_per_pixel >> 3;
+ unsigned int pitch = fb->pitches[0];
+ dma_addr_t dma_addr = exynos_drm_fb_dma_addr(fb, 0);
u32 val;
if (test_bit(BIT_SUSPENDED, >flags))
@@ -284,9 +287,9 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
VIDOSD_Wx_ALPHA_B_F(0x0);
writel(val, ctx->addr + DECON_VIDOSDxD(win));
- writel(plane->dma_addr[0], ctx->addr + DECON_VIDW0xADD0B0(win));
+ writel(dma_addr, ctx->addr + DECON_VIDW0xADD0B0(win));
- val = plane->dma_addr[0] + pitch * plane->crtc_h;
+ val = dma_addr + pitch * plane->crtc_h;
writel(val, ctx->addr + DECON_VIDW0xADD1B0(win));
if (ctx->out_type != IFTYPE_HDMI)
@@ -297,7 +300,7 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
| BIT_VAL(plane->crtc_w * bpp, 14, 0);
writel(val, ctx->addr + DECON_VIDW0xADD2(win));
- decon_win_set_pixfmt(ctx, win, state->fb);
+ decon_win_set_pixfmt(ctx, win, fb);
/* window enable */
decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, ~0);
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 4db04f244c17..1629732574e0 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -30,6 +30,7 @@
#include "exynos_drm_crtc.h"
#include "exynos_drm_plane.h"
#include "exynos_drm_drv.h"
+#include "exynos_drm_fb.h"
#include "exynos_drm_fbdev.h"
#include "exynos_drm_iommu.h"
@@ -395,13 +396,14 @@ static void decon_update_plane(struct exynos_drm_crtc
*crtc,
{
struct decon_context *ctx = crtc->ctx;
struct drm_plane_state *state = plane->base.state;
+ struct drm_framebuffer *fb = state->fb;
int padding;
unsigned long val, alpha;
unsigned int last_x;
unsigned int last_y;
unsigned int win = plane->zpos;
- unsigned int bpp = state->fb->bits_per_pixel >> 3;
- unsigned int pitch = state->fb->pitches[0];
+ unsigned int bpp = fb->bits_per_pixel >> 3;
+ unsigned int pitch = fb->pitches[0];
if (ctx->suspended)
return;
@@ -417,14 +419,14 @@ static void decon_update_plane(struct exynos_drm_crtc
*crtc,
*/
/* buffer start address */
- val = (unsigned long)plane->dma_addr[0];
+ val = (unsigned long)exynos_drm_fb_dma_addr(fb, 0);
writel(val, ctx->regs + VIDW_BUF_START(win));
- padding = (pitch / bpp) - state->fb->width;
+ padding = (pitch / bpp) - fb->width;
/* buffer size */
- writel(state->fb->width + padding, ctx->regs + VIDW_WHOLE_X(win));
- writel(state->fb->height, ctx->regs + VIDW_WHOLE_Y(win));
+ writel(fb->width + padding, ctx->regs + VIDW_WHOLE_X(win));
+ writel(fb->height, ctx->regs + VIDW_WHOLE_Y(win));
/* offset from the start of the buffer to read */
writel(plane->src_x, ctx->regs + VIDW_OFFSET_X(win));
@@ -466,7 +468,7 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
writel(alpha, ctx->regs + VIDOSD_D(win));
- decon_win_set_pixfmt(ctx, win, state->fb);
+ decon_win_set_pixfmt(ctx, win, fb);
/* hardware
