Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-03 Thread Liviu Dudau
On Thu, Dec 03, 2015 at 02:20:26PM +, Daniel Stone wrote:
> Hi Liviu,
> 
> On 3 December 2015 at 10:00, Liviu Dudau  wrote:
> > On Wed, Dec 02, 2015 at 05:21:44PM +, Daniel Stone wrote:
> >> On 2 December 2015 at 12:23, Liviu Dudau  wrote:
> >> > +   if (irq_status & HDLCD_INTERRUPT_VSYNC) {
> >> > +   unsigned long flags;
> >> > +
> >> > +   drm_handle_vblank(drm, 0);
> >> > +
> >> > +   spin_lock_irqsave(>event_lock, flags);
> >> > +   if (hdlcd->event) {
> >> > +   drm_send_vblank_event(drm, hdlcd->event->pipe, 
> >> > hdlcd->event);
> >> > +   drm_crtc_vblank_put(>crtc);
> >> > +   hdlcd->event = NULL;
> >> > +   }
> >> > +   spin_unlock_irqrestore(>event_lock, flags);
> >> > +   }
> >>
> >> As with VC4 and Rockchip, you're missing a ->preclose handler in your
> >> drm_drv, to make sure that you don't try to send events to a dead
> >> client (which causes an OOPS):
> >> https://git.collabora.com/cgit/user/daniels/linux.git/commit/?h=wip/4.4.x/rockchip-drm-fixes=d14f21bcd7
> >> (and its parent)
> >
> > Thanks for reviewing this!
> >
> > I do acknowledge your concerns and you might correct my understanding on
> > how atomic DRM works, but I believe in this case we should be safe. The
> > event stored in hdlcd->event is taken out of the crtc->state->event during
> > the crtc->atomic_begin() callback. If the client is dead the callback should
> > not be called, so that's how we avoid the OOPS.
> 
> Right, it's taken out of the CRTC state and put into the overall HDLCD
> structure. So the OOPS happens when:
>   - client submits state with event requested, async flag set
>   - atomic_begin moves crtc->state->event into hdlcd->event
>   - control returns to client, who exits immediately
>   - vblank happens
>   - hdlcd->event->base.file_priv now points to a dead client
>   - OOPS

I will try to construct an userspace test to see if I can trigger this scenario.

Thanks for explaining it to me.

Best regards,
Liviu

> 
> >> Also, is there anything preventing clients from submitting multiple
> >> pageflips before the event is sent? I couldn't see anything from a
> >> quick look, so you could have the situation of:
> >>   - client submits pageflip, event 1 stored to hdlcd->event
> >>   - client submits pageflip, event 2 stored to hdlcd->event
> >>   - vblank arrives, event 2 is sent
> >>   - event 1 has disappeared and been leaked
> >
> > As for multiple events being submitted before vsync interrupt happening: 
> > given
> > that the event is plucked out of the crtc->state, is that possible? And if 
> > so,
> > is there a case where having the later event overwrite the earlier one a 
> > desired
> > outcome?
> 
> Having events being overwritten is extremely undesirable; it could
> cause a client to stall in the right scenarios (e.g. requests
> submitted separately for different planes). It would be far better to
> turn hdlcd->event into a list of events which are sent per-vblank,
> probably just by retaining a list of pending states to apply; this
> also makes it easier to handle async commits in the future (which
> Weston in particular relies on).
> 
> Cheers,
> Daniel
> 

-- 

| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---
¯\_(ツ)_/¯
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-03 Thread Daniel Stone
Hi Liviu,

On 3 December 2015 at 10:00, Liviu Dudau  wrote:
> On Wed, Dec 02, 2015 at 05:21:44PM +, Daniel Stone wrote:
>> On 2 December 2015 at 12:23, Liviu Dudau  wrote:
>> > +   if (irq_status & HDLCD_INTERRUPT_VSYNC) {
>> > +   unsigned long flags;
>> > +
>> > +   drm_handle_vblank(drm, 0);
>> > +
>> > +   spin_lock_irqsave(>event_lock, flags);
>> > +   if (hdlcd->event) {
>> > +   drm_send_vblank_event(drm, hdlcd->event->pipe, 
>> > hdlcd->event);
>> > +   drm_crtc_vblank_put(>crtc);
>> > +   hdlcd->event = NULL;
>> > +   }
>> > +   spin_unlock_irqrestore(>event_lock, flags);
>> > +   }
>>
>> As with VC4 and Rockchip, you're missing a ->preclose handler in your
>> drm_drv, to make sure that you don't try to send events to a dead
>> client (which causes an OOPS):
>> https://git.collabora.com/cgit/user/daniels/linux.git/commit/?h=wip/4.4.x/rockchip-drm-fixes=d14f21bcd7
>> (and its parent)
>
> Thanks for reviewing this!
>
> I do acknowledge your concerns and you might correct my understanding on
> how atomic DRM works, but I believe in this case we should be safe. The
> event stored in hdlcd->event is taken out of the crtc->state->event during
> the crtc->atomic_begin() callback. If the client is dead the callback should
> not be called, so that's how we avoid the OOPS.

Right, it's taken out of the CRTC state and put into the overall HDLCD
structure. So the OOPS happens when:
  - client submits state with event requested, async flag set
  - atomic_begin moves crtc->state->event into hdlcd->event
  - control returns to client, who exits immediately
  - vblank happens
  - hdlcd->event->base.file_priv now points to a dead client
  - OOPS

>> Also, is there anything preventing clients from submitting multiple
>> pageflips before the event is sent? I couldn't see anything from a
>> quick look, so you could have the situation of:
>>   - client submits pageflip, event 1 stored to hdlcd->event
>>   - client submits pageflip, event 2 stored to hdlcd->event
>>   - vblank arrives, event 2 is sent
>>   - event 1 has disappeared and been leaked
>
> As for multiple events being submitted before vsync interrupt happening: given
> that the event is plucked out of the crtc->state, is that possible? And if so,
> is there a case where having the later event overwrite the earlier one a 
> desired
> outcome?

Having events being overwritten is extremely undesirable; it could
cause a client to stall in the right scenarios (e.g. requests
submitted separately for different planes). It would be far better to
turn hdlcd->event into a list of events which are sent per-vblank,
probably just by retaining a list of pending states to apply; this
also makes it easier to handle async commits in the future (which
Weston in particular relies on).

Cheers,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-03 Thread Liviu Dudau
On Wed, Dec 02, 2015 at 05:21:44PM +, Daniel Stone wrote:
> Hi Liviu,

Hi Daniel,

> 
> On 2 December 2015 at 12:23, Liviu Dudau  wrote:
> > +   if (irq_status & HDLCD_INTERRUPT_VSYNC) {
> > +   unsigned long flags;
> > +
> > +   drm_handle_vblank(drm, 0);
> > +
> > +   spin_lock_irqsave(>event_lock, flags);
> > +   if (hdlcd->event) {
> > +   drm_send_vblank_event(drm, hdlcd->event->pipe, 
> > hdlcd->event);
> > +   drm_crtc_vblank_put(>crtc);
> > +   hdlcd->event = NULL;
> > +   }
> > +   spin_unlock_irqrestore(>event_lock, flags);
> > +   }
> 
> As with VC4 and Rockchip, you're missing a ->preclose handler in your
> drm_drv, to make sure that you don't try to send events to a dead
> client (which causes an OOPS):
> https://git.collabora.com/cgit/user/daniels/linux.git/commit/?h=wip/4.4.x/rockchip-drm-fixes=d14f21bcd7
> (and its parent)
 
Thanks for reviewing this!

I do acknowledge your concerns and you might correct my understanding on
how atomic DRM works, but I believe in this case we should be safe. The
event stored in hdlcd->event is taken out of the crtc->state->event during
the crtc->atomic_begin() callback. If the client is dead the callback should
not be called, so that's how we avoid the OOPS.

> Also, is there anything preventing clients from submitting multiple
> pageflips before the event is sent? I couldn't see anything from a
> quick look, so you could have the situation of:
>   - client submits pageflip, event 1 stored to hdlcd->event
>   - client submits pageflip, event 2 stored to hdlcd->event
>   - vblank arrives, event 2 is sent
>   - event 1 has disappeared and been leaked

As for multiple events being submitted before vsync interrupt happening: given
that the event is plucked out of the crtc->state, is that possible? And if so,
is there a case where having the later event overwrite the earlier one a desired
outcome?

> 
> Cheers,
> Daniel
> 

Best regards,
Liviu

-- 

| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---
¯\_(ツ)_/¯
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-03 Thread Liviu Dudau
On Wed, Dec 02, 2015 at 05:21:44PM +, Daniel Stone wrote:
> Hi Liviu,

Hi Daniel,

> 
> On 2 December 2015 at 12:23, Liviu Dudau  wrote:
> > +   if (irq_status & HDLCD_INTERRUPT_VSYNC) {
> > +   unsigned long flags;
> > +
> > +   drm_handle_vblank(drm, 0);
> > +
> > +   spin_lock_irqsave(>event_lock, flags);
> > +   if (hdlcd->event) {
> > +   drm_send_vblank_event(drm, hdlcd->event->pipe, 
> > hdlcd->event);
> > +   drm_crtc_vblank_put(>crtc);
> > +   hdlcd->event = NULL;
> > +   }
> > +   spin_unlock_irqrestore(>event_lock, flags);
> > +   }
> 
> As with VC4 and Rockchip, you're missing a ->preclose handler in your
> drm_drv, to make sure that you don't try to send events to a dead
> client (which causes an OOPS):
> https://git.collabora.com/cgit/user/daniels/linux.git/commit/?h=wip/4.4.x/rockchip-drm-fixes=d14f21bcd7
> (and its parent)
 
Thanks for reviewing this!

I do acknowledge your concerns and you might correct my understanding on
how atomic DRM works, but I believe in this case we should be safe. The
event stored in hdlcd->event is taken out of the crtc->state->event during
the crtc->atomic_begin() callback. If the client is dead the callback should
not be called, so that's how we avoid the OOPS.

> Also, is there anything preventing clients from submitting multiple
> pageflips before the event is sent? I couldn't see anything from a
> quick look, so you could have the situation of:
>   - client submits pageflip, event 1 stored to hdlcd->event
>   - client submits pageflip, event 2 stored to hdlcd->event
>   - vblank arrives, event 2 is sent
>   - event 1 has disappeared and been leaked

As for multiple events being submitted before vsync interrupt happening: given
that the event is plucked out of the crtc->state, is that possible? And if so,
is there a case where having the later event overwrite the earlier one a desired
outcome?

> 
> Cheers,
> Daniel
> 

Best regards,
Liviu

-- 

| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---
¯\_(ツ)_/¯
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-03 Thread Liviu Dudau
On Thu, Dec 03, 2015 at 02:20:26PM +, Daniel Stone wrote:
> Hi Liviu,
> 
> On 3 December 2015 at 10:00, Liviu Dudau  wrote:
> > On Wed, Dec 02, 2015 at 05:21:44PM +, Daniel Stone wrote:
> >> On 2 December 2015 at 12:23, Liviu Dudau  wrote:
> >> > +   if (irq_status & HDLCD_INTERRUPT_VSYNC) {
> >> > +   unsigned long flags;
> >> > +
> >> > +   drm_handle_vblank(drm, 0);
> >> > +
> >> > +   spin_lock_irqsave(>event_lock, flags);
> >> > +   if (hdlcd->event) {
> >> > +   drm_send_vblank_event(drm, hdlcd->event->pipe, 
> >> > hdlcd->event);
> >> > +   drm_crtc_vblank_put(>crtc);
> >> > +   hdlcd->event = NULL;
> >> > +   }
> >> > +   spin_unlock_irqrestore(>event_lock, flags);
> >> > +   }
> >>
> >> As with VC4 and Rockchip, you're missing a ->preclose handler in your
> >> drm_drv, to make sure that you don't try to send events to a dead
> >> client (which causes an OOPS):
> >> https://git.collabora.com/cgit/user/daniels/linux.git/commit/?h=wip/4.4.x/rockchip-drm-fixes=d14f21bcd7
> >> (and its parent)
> >
> > Thanks for reviewing this!
> >
> > I do acknowledge your concerns and you might correct my understanding on
> > how atomic DRM works, but I believe in this case we should be safe. The
> > event stored in hdlcd->event is taken out of the crtc->state->event during
> > the crtc->atomic_begin() callback. If the client is dead the callback should
> > not be called, so that's how we avoid the OOPS.
> 
> Right, it's taken out of the CRTC state and put into the overall HDLCD
> structure. So the OOPS happens when:
>   - client submits state with event requested, async flag set
>   - atomic_begin moves crtc->state->event into hdlcd->event
>   - control returns to client, who exits immediately
>   - vblank happens
>   - hdlcd->event->base.file_priv now points to a dead client
>   - OOPS

I will try to construct an userspace test to see if I can trigger this scenario.

Thanks for explaining it to me.

Best regards,
Liviu

> 
> >> Also, is there anything preventing clients from submitting multiple
> >> pageflips before the event is sent? I couldn't see anything from a
> >> quick look, so you could have the situation of:
> >>   - client submits pageflip, event 1 stored to hdlcd->event
> >>   - client submits pageflip, event 2 stored to hdlcd->event
> >>   - vblank arrives, event 2 is sent
> >>   - event 1 has disappeared and been leaked
> >
> > As for multiple events being submitted before vsync interrupt happening: 
> > given
> > that the event is plucked out of the crtc->state, is that possible? And if 
> > so,
> > is there a case where having the later event overwrite the earlier one a 
> > desired
> > outcome?
> 
> Having events being overwritten is extremely undesirable; it could
> cause a client to stall in the right scenarios (e.g. requests
> submitted separately for different planes). It would be far better to
> turn hdlcd->event into a list of events which are sent per-vblank,
> probably just by retaining a list of pending states to apply; this
> also makes it easier to handle async commits in the future (which
> Weston in particular relies on).
> 
> Cheers,
> Daniel
> 

-- 

| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---
¯\_(ツ)_/¯
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-03 Thread Daniel Stone
Hi Liviu,

On 3 December 2015 at 10:00, Liviu Dudau  wrote:
> On Wed, Dec 02, 2015 at 05:21:44PM +, Daniel Stone wrote:
>> On 2 December 2015 at 12:23, Liviu Dudau  wrote:
>> > +   if (irq_status & HDLCD_INTERRUPT_VSYNC) {
>> > +   unsigned long flags;
>> > +
>> > +   drm_handle_vblank(drm, 0);
>> > +
>> > +   spin_lock_irqsave(>event_lock, flags);
>> > +   if (hdlcd->event) {
>> > +   drm_send_vblank_event(drm, hdlcd->event->pipe, 
>> > hdlcd->event);
>> > +   drm_crtc_vblank_put(>crtc);
>> > +   hdlcd->event = NULL;
>> > +   }
>> > +   spin_unlock_irqrestore(>event_lock, flags);
>> > +   }
>>
>> As with VC4 and Rockchip, you're missing a ->preclose handler in your
>> drm_drv, to make sure that you don't try to send events to a dead
>> client (which causes an OOPS):
>> https://git.collabora.com/cgit/user/daniels/linux.git/commit/?h=wip/4.4.x/rockchip-drm-fixes=d14f21bcd7
>> (and its parent)
>
> Thanks for reviewing this!
>
> I do acknowledge your concerns and you might correct my understanding on
> how atomic DRM works, but I believe in this case we should be safe. The
> event stored in hdlcd->event is taken out of the crtc->state->event during
> the crtc->atomic_begin() callback. If the client is dead the callback should
> not be called, so that's how we avoid the OOPS.

Right, it's taken out of the CRTC state and put into the overall HDLCD
structure. So the OOPS happens when:
  - client submits state with event requested, async flag set
  - atomic_begin moves crtc->state->event into hdlcd->event
  - control returns to client, who exits immediately
  - vblank happens
  - hdlcd->event->base.file_priv now points to a dead client
  - OOPS

>> Also, is there anything preventing clients from submitting multiple
>> pageflips before the event is sent? I couldn't see anything from a
>> quick look, so you could have the situation of:
>>   - client submits pageflip, event 1 stored to hdlcd->event
>>   - client submits pageflip, event 2 stored to hdlcd->event
>>   - vblank arrives, event 2 is sent
>>   - event 1 has disappeared and been leaked
>
> As for multiple events being submitted before vsync interrupt happening: given
> that the event is plucked out of the crtc->state, is that possible? And if so,
> is there a case where having the later event overwrite the earlier one a 
> desired
> outcome?

Having events being overwritten is extremely undesirable; it could
cause a client to stall in the right scenarios (e.g. requests
submitted separately for different planes). It would be far better to
turn hdlcd->event into a list of events which are sent per-vblank,
probably just by retaining a list of pending states to apply; this
also makes it easier to handle async commits in the future (which
Weston in particular relies on).

Cheers,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-02 Thread Daniel Stone
Hi Liviu,

On 2 December 2015 at 12:23, Liviu Dudau  wrote:
> +   if (irq_status & HDLCD_INTERRUPT_VSYNC) {
> +   unsigned long flags;
> +
> +   drm_handle_vblank(drm, 0);
> +
> +   spin_lock_irqsave(>event_lock, flags);
> +   if (hdlcd->event) {
> +   drm_send_vblank_event(drm, hdlcd->event->pipe, 
> hdlcd->event);
> +   drm_crtc_vblank_put(>crtc);
> +   hdlcd->event = NULL;
> +   }
> +   spin_unlock_irqrestore(>event_lock, flags);
> +   }

As with VC4 and Rockchip, you're missing a ->preclose handler in your
drm_drv, to make sure that you don't try to send events to a dead
client (which causes an OOPS):
https://git.collabora.com/cgit/user/daniels/linux.git/commit/?h=wip/4.4.x/rockchip-drm-fixes=d14f21bcd7
(and its parent)

Also, is there anything preventing clients from submitting multiple
pageflips before the event is sent? I couldn't see anything from a
quick look, so you could have the situation of:
  - client submits pageflip, event 1 stored to hdlcd->event
  - client submits pageflip, event 2 stored to hdlcd->event
  - vblank arrives, event 2 is sent
  - event 1 has disappeared and been leaked

Cheers,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-02 Thread Liviu Dudau
On Wed, Dec 02, 2015 at 04:24:19PM +0100, Daniel Vetter wrote:
> On Wed, Dec 02, 2015 at 12:23:00PM +, Liviu Dudau wrote:
> > The HDLCD controller is a display controller that supports resolutions
> > up to 4096x4096 pixels. It is present on various development boards
> > produced by ARM Ltd and emulated by the latest Fast Models from the
> > company.
> > 
> > Cc: David Airlie 
> > Cc: Robin Murphy 
> > 
> > Signed-off-by: Liviu Dudau 
> 
> A few small nitpicks below.
> 
> > ---
> >  drivers/gpu/drm/Kconfig  |   2 +
> >  drivers/gpu/drm/Makefile |   1 +
> >  drivers/gpu/drm/arm/Kconfig  |  29 ++
> >  drivers/gpu/drm/arm/Makefile |   2 +
> >  drivers/gpu/drm/arm/hdlcd_crtc.c | 334 +++
> >  drivers/gpu/drm/arm/hdlcd_drv.c  | 555 
> > +++
> >  drivers/gpu/drm/arm/hdlcd_drv.h  |  42 +++
> >  drivers/gpu/drm/arm/hdlcd_regs.h |  87 ++
> >  8 files changed, 1052 insertions(+)
> >  create mode 100644 drivers/gpu/drm/arm/Kconfig
> >  create mode 100644 drivers/gpu/drm/arm/Makefile
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_crtc.c
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.c
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.h
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_regs.h
> > 
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index c4bf9a1..3fd9124 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -106,6 +106,8 @@ config DRM_TDFX
> >   Choose this option if you have a 3dfx Banshee or Voodoo3 (or later),
> >   graphics card.  If M is selected, the module will be called tdfx.
> >  
> > +source "drivers/gpu/drm/arm/Kconfig"
> > +
> >  config DRM_R128
> > tristate "ATI Rage 128"
> > depends on DRM && PCI
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 1e9ff4c..6b42d75 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -35,6 +35,7 @@ CFLAGS_drm_trace_points.o := -I$(src)
> >  
> >  obj-$(CONFIG_DRM)  += drm.o
> >  obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> > +obj-$(CONFIG_DRM_ARM)  += arm/
> >  obj-$(CONFIG_DRM_TTM)  += ttm/
> >  obj-$(CONFIG_DRM_TDFX) += tdfx/
> >  obj-$(CONFIG_DRM_R128) += r128/
> > diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
> > new file mode 100644
> > index 000..5e8c8a8
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/Kconfig
> > @@ -0,0 +1,29 @@
> > +config DRM_ARM
> > +   bool "ARM Ltd. drivers"
> > +   depends on DRM && OF && (ARM || ARM64)
> > +   select DRM_KMS_HELPER
> > +   help
> > + Choose this option to select drivers for ARM's devices
> > +
> > +config DRM_HDLCD
> > +   tristate "ARM HDLCD"
> > +   depends on DRM_ARM
> > +   depends on COMMON_CLK
> > +   select COMMON_CLK_SCPI
> > +   select DMA_CMA
> > +   select DRM_KMS_CMA_HELPER
> > +   select DRM_GEM_CMA_HELPER
> > +   help
> > + Choose this option if you have an ARM High Definition Colour LCD
> > + controller.
> > +
> > + If M is selected the module will be called hdlcd.
> > +
> > +config DRM_HDLCD_SHOW_UNDERRUN
> > +   bool "Show underrun conditions"
> > +   depends on DRM_HDLCD
> > +   default n
> > +   help
> > + Enable this option to show in red colour the pixels that the
> > + HDLCD device did not fetch from framebuffer due to underrun
> > + conditions.
> > diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
> > new file mode 100644
> > index 000..89dcb7b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/Makefile
> > @@ -0,0 +1,2 @@
> > +hdlcd-y := hdlcd_drv.o hdlcd_crtc.o
> > +obj-$(CONFIG_DRM_HDLCD)+= hdlcd.o
> > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c 
> > b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > new file mode 100644
> > index 000..350c1fe
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > @@ -0,0 +1,334 @@
> > +/*
> > + * Copyright (C) 2013-2015 ARM Limited
> > + * Author: Liviu Dudau 
> > + *
> > + * This file is subject to the terms and conditions of the GNU General 
> > Public
> > + * License.  See the file COPYING in the main directory of this archive
> > + * for more details.
> > + *
> > + *  Implementation of a CRTC class for the HDLCD driver.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "hdlcd_drv.h"
> > +#include "hdlcd_regs.h"
> > +
> > +/*
> > + * The HDLCD controller is a dumb RGB streamer that gets connected to
> > + * a single HDMI transmitter or in the case of the ARM Models it gets
> > + * emulated by the software that does the actual rendering.
> > + *
> > + */
> > +
> > +static const struct drm_crtc_funcs hdlcd_crtc_funcs = {
> > +   .destroy = drm_crtc_cleanup,
> > +   .set_config = drm_atomic_helper_set_config,
> > +   .page_flip = 

Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-02 Thread Liviu Dudau
On Wed, Dec 02, 2015 at 04:33:31PM +0100, Daniel Vetter wrote:
> On Wed, Dec 02, 2015 at 04:24:19PM +0100, Daniel Vetter wrote:
> > On Wed, Dec 02, 2015 at 12:23:00PM +, Liviu Dudau wrote:
> > > The HDLCD controller is a display controller that supports resolutions
> > > up to 4096x4096 pixels. It is present on various development boards
> > > produced by ARM Ltd and emulated by the latest Fast Models from the
> > > company.
> > > 
> > > Cc: David Airlie 
> > > Cc: Robin Murphy 
> > > 
> > > Signed-off-by: Liviu Dudau 
> > 
> > A few small nitpicks below.
> 
> I've forgotten to add Acked-by: Daniel Vetter 
> with these all addressed.

Thanks! I'll make the changes and post them lately.

Best regards,
Liviu

> -Daniel
> 
> > 
> > > ---
> > >  drivers/gpu/drm/Kconfig  |   2 +
> > >  drivers/gpu/drm/Makefile |   1 +
> > >  drivers/gpu/drm/arm/Kconfig  |  29 ++
> > >  drivers/gpu/drm/arm/Makefile |   2 +
> > >  drivers/gpu/drm/arm/hdlcd_crtc.c | 334 +++
> > >  drivers/gpu/drm/arm/hdlcd_drv.c  | 555 
> > > +++
> > >  drivers/gpu/drm/arm/hdlcd_drv.h  |  42 +++
> > >  drivers/gpu/drm/arm/hdlcd_regs.h |  87 ++
> > >  8 files changed, 1052 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/arm/Kconfig
> > >  create mode 100644 drivers/gpu/drm/arm/Makefile
> > >  create mode 100644 drivers/gpu/drm/arm/hdlcd_crtc.c
> > >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.c
> > >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.h
> > >  create mode 100644 drivers/gpu/drm/arm/hdlcd_regs.h
> > > 
> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > > index c4bf9a1..3fd9124 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -106,6 +106,8 @@ config DRM_TDFX
> > > Choose this option if you have a 3dfx Banshee or Voodoo3 (or later),
> > > graphics card.  If M is selected, the module will be called tdfx.
> > >  
> > > +source "drivers/gpu/drm/arm/Kconfig"
> > > +
> > >  config DRM_R128
> > >   tristate "ATI Rage 128"
> > >   depends on DRM && PCI
> > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > index 1e9ff4c..6b42d75 100644
> > > --- a/drivers/gpu/drm/Makefile
> > > +++ b/drivers/gpu/drm/Makefile
> > > @@ -35,6 +35,7 @@ CFLAGS_drm_trace_points.o := -I$(src)
> > >  
> > >  obj-$(CONFIG_DRM)+= drm.o
> > >  obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> > > +obj-$(CONFIG_DRM_ARM)+= arm/
> > >  obj-$(CONFIG_DRM_TTM)+= ttm/
> > >  obj-$(CONFIG_DRM_TDFX)   += tdfx/
> > >  obj-$(CONFIG_DRM_R128)   += r128/
> > > diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
> > > new file mode 100644
> > > index 000..5e8c8a8
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/arm/Kconfig
> > > @@ -0,0 +1,29 @@
> > > +config DRM_ARM
> > > + bool "ARM Ltd. drivers"
> > > + depends on DRM && OF && (ARM || ARM64)
> > > + select DRM_KMS_HELPER
> > > + help
> > > +   Choose this option to select drivers for ARM's devices
> > > +
> > > +config DRM_HDLCD
> > > + tristate "ARM HDLCD"
> > > + depends on DRM_ARM
> > > + depends on COMMON_CLK
> > > + select COMMON_CLK_SCPI
> > > + select DMA_CMA
> > > + select DRM_KMS_CMA_HELPER
> > > + select DRM_GEM_CMA_HELPER
> > > + help
> > > +   Choose this option if you have an ARM High Definition Colour LCD
> > > +   controller.
> > > +
> > > +   If M is selected the module will be called hdlcd.
> > > +
> > > +config DRM_HDLCD_SHOW_UNDERRUN
> > > + bool "Show underrun conditions"
> > > + depends on DRM_HDLCD
> > > + default n
> > > + help
> > > +   Enable this option to show in red colour the pixels that the
> > > +   HDLCD device did not fetch from framebuffer due to underrun
> > > +   conditions.
> > > diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
> > > new file mode 100644
> > > index 000..89dcb7b
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/arm/Makefile
> > > @@ -0,0 +1,2 @@
> > > +hdlcd-y := hdlcd_drv.o hdlcd_crtc.o
> > > +obj-$(CONFIG_DRM_HDLCD)  += hdlcd.o
> > > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c 
> > > b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > > new file mode 100644
> > > index 000..350c1fe
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > > @@ -0,0 +1,334 @@
> > > +/*
> > > + * Copyright (C) 2013-2015 ARM Limited
> > > + * Author: Liviu Dudau 
> > > + *
> > > + * This file is subject to the terms and conditions of the GNU General 
> > > Public
> > > + * License.  See the file COPYING in the main directory of this archive
> > > + * for more details.
> > > + *
> > > + *  Implementation of a CRTC class for the HDLCD driver.
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include "hdlcd_drv.h"
> > > +#include 

Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-02 Thread Daniel Vetter
On Wed, Dec 02, 2015 at 04:24:19PM +0100, Daniel Vetter wrote:
> On Wed, Dec 02, 2015 at 12:23:00PM +, Liviu Dudau wrote:
> > The HDLCD controller is a display controller that supports resolutions
> > up to 4096x4096 pixels. It is present on various development boards
> > produced by ARM Ltd and emulated by the latest Fast Models from the
> > company.
> > 
> > Cc: David Airlie 
> > Cc: Robin Murphy 
> > 
> > Signed-off-by: Liviu Dudau 
> 
> A few small nitpicks below.

I've forgotten to add Acked-by: Daniel Vetter 
with these all addressed.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/Kconfig  |   2 +
> >  drivers/gpu/drm/Makefile |   1 +
> >  drivers/gpu/drm/arm/Kconfig  |  29 ++
> >  drivers/gpu/drm/arm/Makefile |   2 +
> >  drivers/gpu/drm/arm/hdlcd_crtc.c | 334 +++
> >  drivers/gpu/drm/arm/hdlcd_drv.c  | 555 
> > +++
> >  drivers/gpu/drm/arm/hdlcd_drv.h  |  42 +++
> >  drivers/gpu/drm/arm/hdlcd_regs.h |  87 ++
> >  8 files changed, 1052 insertions(+)
> >  create mode 100644 drivers/gpu/drm/arm/Kconfig
> >  create mode 100644 drivers/gpu/drm/arm/Makefile
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_crtc.c
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.c
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.h
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_regs.h
> > 
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index c4bf9a1..3fd9124 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -106,6 +106,8 @@ config DRM_TDFX
> >   Choose this option if you have a 3dfx Banshee or Voodoo3 (or later),
> >   graphics card.  If M is selected, the module will be called tdfx.
> >  
> > +source "drivers/gpu/drm/arm/Kconfig"
> > +
> >  config DRM_R128
> > tristate "ATI Rage 128"
> > depends on DRM && PCI
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 1e9ff4c..6b42d75 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -35,6 +35,7 @@ CFLAGS_drm_trace_points.o := -I$(src)
> >  
> >  obj-$(CONFIG_DRM)  += drm.o
> >  obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> > +obj-$(CONFIG_DRM_ARM)  += arm/
> >  obj-$(CONFIG_DRM_TTM)  += ttm/
> >  obj-$(CONFIG_DRM_TDFX) += tdfx/
> >  obj-$(CONFIG_DRM_R128) += r128/
> > diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
> > new file mode 100644
> > index 000..5e8c8a8
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/Kconfig
> > @@ -0,0 +1,29 @@
> > +config DRM_ARM
> > +   bool "ARM Ltd. drivers"
> > +   depends on DRM && OF && (ARM || ARM64)
> > +   select DRM_KMS_HELPER
> > +   help
> > + Choose this option to select drivers for ARM's devices
> > +
> > +config DRM_HDLCD
> > +   tristate "ARM HDLCD"
> > +   depends on DRM_ARM
> > +   depends on COMMON_CLK
> > +   select COMMON_CLK_SCPI
> > +   select DMA_CMA
> > +   select DRM_KMS_CMA_HELPER
> > +   select DRM_GEM_CMA_HELPER
> > +   help
> > + Choose this option if you have an ARM High Definition Colour LCD
> > + controller.
> > +
> > + If M is selected the module will be called hdlcd.
> > +
> > +config DRM_HDLCD_SHOW_UNDERRUN
> > +   bool "Show underrun conditions"
> > +   depends on DRM_HDLCD
> > +   default n
> > +   help
> > + Enable this option to show in red colour the pixels that the
> > + HDLCD device did not fetch from framebuffer due to underrun
> > + conditions.
> > diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
> > new file mode 100644
> > index 000..89dcb7b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/Makefile
> > @@ -0,0 +1,2 @@
> > +hdlcd-y := hdlcd_drv.o hdlcd_crtc.o
> > +obj-$(CONFIG_DRM_HDLCD)+= hdlcd.o
> > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c 
> > b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > new file mode 100644
> > index 000..350c1fe
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > @@ -0,0 +1,334 @@
> > +/*
> > + * Copyright (C) 2013-2015 ARM Limited
> > + * Author: Liviu Dudau 
> > + *
> > + * This file is subject to the terms and conditions of the GNU General 
> > Public
> > + * License.  See the file COPYING in the main directory of this archive
> > + * for more details.
> > + *
> > + *  Implementation of a CRTC class for the HDLCD driver.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "hdlcd_drv.h"
> > +#include "hdlcd_regs.h"
> > +
> > +/*
> > + * The HDLCD controller is a dumb RGB streamer that gets connected to
> > + * a single HDMI transmitter or in the case of the ARM Models it gets
> > + * emulated by the software that does the actual rendering.
> > + *
> > + */
> > +
> > +static const struct drm_crtc_funcs hdlcd_crtc_funcs = {
> > +   .destroy = drm_crtc_cleanup,
> > +   

Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-02 Thread Daniel Vetter
On Wed, Dec 02, 2015 at 12:23:00PM +, Liviu Dudau wrote:
> The HDLCD controller is a display controller that supports resolutions
> up to 4096x4096 pixels. It is present on various development boards
> produced by ARM Ltd and emulated by the latest Fast Models from the
> company.
> 
> Cc: David Airlie 
> Cc: Robin Murphy 
> 
> Signed-off-by: Liviu Dudau 

A few small nitpicks below.

> ---
>  drivers/gpu/drm/Kconfig  |   2 +
>  drivers/gpu/drm/Makefile |   1 +
>  drivers/gpu/drm/arm/Kconfig  |  29 ++
>  drivers/gpu/drm/arm/Makefile |   2 +
>  drivers/gpu/drm/arm/hdlcd_crtc.c | 334 +++
>  drivers/gpu/drm/arm/hdlcd_drv.c  | 555 
> +++
>  drivers/gpu/drm/arm/hdlcd_drv.h  |  42 +++
>  drivers/gpu/drm/arm/hdlcd_regs.h |  87 ++
>  8 files changed, 1052 insertions(+)
>  create mode 100644 drivers/gpu/drm/arm/Kconfig
>  create mode 100644 drivers/gpu/drm/arm/Makefile
>  create mode 100644 drivers/gpu/drm/arm/hdlcd_crtc.c
>  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.c
>  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.h
>  create mode 100644 drivers/gpu/drm/arm/hdlcd_regs.h
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index c4bf9a1..3fd9124 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -106,6 +106,8 @@ config DRM_TDFX
> Choose this option if you have a 3dfx Banshee or Voodoo3 (or later),
> graphics card.  If M is selected, the module will be called tdfx.
>  
> +source "drivers/gpu/drm/arm/Kconfig"
> +
>  config DRM_R128
>   tristate "ATI Rage 128"
>   depends on DRM && PCI
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 1e9ff4c..6b42d75 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -35,6 +35,7 @@ CFLAGS_drm_trace_points.o := -I$(src)
>  
>  obj-$(CONFIG_DRM)+= drm.o
>  obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> +obj-$(CONFIG_DRM_ARM)+= arm/
>  obj-$(CONFIG_DRM_TTM)+= ttm/
>  obj-$(CONFIG_DRM_TDFX)   += tdfx/
>  obj-$(CONFIG_DRM_R128)   += r128/
> diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
> new file mode 100644
> index 000..5e8c8a8
> --- /dev/null
> +++ b/drivers/gpu/drm/arm/Kconfig
> @@ -0,0 +1,29 @@
> +config DRM_ARM
> + bool "ARM Ltd. drivers"
> + depends on DRM && OF && (ARM || ARM64)
> + select DRM_KMS_HELPER
> + help
> +   Choose this option to select drivers for ARM's devices
> +
> +config DRM_HDLCD
> + tristate "ARM HDLCD"
> + depends on DRM_ARM
> + depends on COMMON_CLK
> + select COMMON_CLK_SCPI
> + select DMA_CMA
> + select DRM_KMS_CMA_HELPER
> + select DRM_GEM_CMA_HELPER
> + help
> +   Choose this option if you have an ARM High Definition Colour LCD
> +   controller.
> +
> +   If M is selected the module will be called hdlcd.
> +
> +config DRM_HDLCD_SHOW_UNDERRUN
> + bool "Show underrun conditions"
> + depends on DRM_HDLCD
> + default n
> + help
> +   Enable this option to show in red colour the pixels that the
> +   HDLCD device did not fetch from framebuffer due to underrun
> +   conditions.
> diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
> new file mode 100644
> index 000..89dcb7b
> --- /dev/null
> +++ b/drivers/gpu/drm/arm/Makefile
> @@ -0,0 +1,2 @@
> +hdlcd-y := hdlcd_drv.o hdlcd_crtc.o
> +obj-$(CONFIG_DRM_HDLCD)  += hdlcd.o
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c 
> b/drivers/gpu/drm/arm/hdlcd_crtc.c
> new file mode 100644
> index 000..350c1fe
> --- /dev/null
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -0,0 +1,334 @@
> +/*
> + * Copyright (C) 2013-2015 ARM Limited
> + * Author: Liviu Dudau 
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file COPYING in the main directory of this archive
> + * for more details.
> + *
> + *  Implementation of a CRTC class for the HDLCD driver.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "hdlcd_drv.h"
> +#include "hdlcd_regs.h"
> +
> +/*
> + * The HDLCD controller is a dumb RGB streamer that gets connected to
> + * a single HDMI transmitter or in the case of the ARM Models it gets
> + * emulated by the software that does the actual rendering.
> + *
> + */
> +
> +static const struct drm_crtc_funcs hdlcd_crtc_funcs = {
> + .destroy = drm_crtc_cleanup,
> + .set_config = drm_atomic_helper_set_config,
> + .page_flip = drm_atomic_helper_page_flip,
> + .reset = drm_atomic_helper_crtc_reset,
> + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> +};
> +
> +static struct simplefb_format supported_formats[] = SIMPLEFB_FORMATS;
> 

Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-02 Thread Liviu Dudau
On Wed, Dec 02, 2015 at 04:33:31PM +0100, Daniel Vetter wrote:
> On Wed, Dec 02, 2015 at 04:24:19PM +0100, Daniel Vetter wrote:
> > On Wed, Dec 02, 2015 at 12:23:00PM +, Liviu Dudau wrote:
> > > The HDLCD controller is a display controller that supports resolutions
> > > up to 4096x4096 pixels. It is present on various development boards
> > > produced by ARM Ltd and emulated by the latest Fast Models from the
> > > company.
> > > 
> > > Cc: David Airlie 
> > > Cc: Robin Murphy 
> > > 
> > > Signed-off-by: Liviu Dudau 
> > 
> > A few small nitpicks below.
> 
> I've forgotten to add Acked-by: Daniel Vetter 
> with these all addressed.

Thanks! I'll make the changes and post them lately.

Best regards,
Liviu

> -Daniel
> 
> > 
> > > ---
> > >  drivers/gpu/drm/Kconfig  |   2 +
> > >  drivers/gpu/drm/Makefile |   1 +
> > >  drivers/gpu/drm/arm/Kconfig  |  29 ++
> > >  drivers/gpu/drm/arm/Makefile |   2 +
> > >  drivers/gpu/drm/arm/hdlcd_crtc.c | 334 +++
> > >  drivers/gpu/drm/arm/hdlcd_drv.c  | 555 
> > > +++
> > >  drivers/gpu/drm/arm/hdlcd_drv.h  |  42 +++
> > >  drivers/gpu/drm/arm/hdlcd_regs.h |  87 ++
> > >  8 files changed, 1052 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/arm/Kconfig
> > >  create mode 100644 drivers/gpu/drm/arm/Makefile
> > >  create mode 100644 drivers/gpu/drm/arm/hdlcd_crtc.c
> > >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.c
> > >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.h
> > >  create mode 100644 drivers/gpu/drm/arm/hdlcd_regs.h
> > > 
> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > > index c4bf9a1..3fd9124 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -106,6 +106,8 @@ config DRM_TDFX
> > > Choose this option if you have a 3dfx Banshee or Voodoo3 (or later),
> > > graphics card.  If M is selected, the module will be called tdfx.
> > >  
> > > +source "drivers/gpu/drm/arm/Kconfig"
> > > +
> > >  config DRM_R128
> > >   tristate "ATI Rage 128"
> > >   depends on DRM && PCI
> > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > index 1e9ff4c..6b42d75 100644
> > > --- a/drivers/gpu/drm/Makefile
> > > +++ b/drivers/gpu/drm/Makefile
> > > @@ -35,6 +35,7 @@ CFLAGS_drm_trace_points.o := -I$(src)
> > >  
> > >  obj-$(CONFIG_DRM)+= drm.o
> > >  obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> > > +obj-$(CONFIG_DRM_ARM)+= arm/
> > >  obj-$(CONFIG_DRM_TTM)+= ttm/
> > >  obj-$(CONFIG_DRM_TDFX)   += tdfx/
> > >  obj-$(CONFIG_DRM_R128)   += r128/
> > > diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
> > > new file mode 100644
> > > index 000..5e8c8a8
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/arm/Kconfig
> > > @@ -0,0 +1,29 @@
> > > +config DRM_ARM
> > > + bool "ARM Ltd. drivers"
> > > + depends on DRM && OF && (ARM || ARM64)
> > > + select DRM_KMS_HELPER
> > > + help
> > > +   Choose this option to select drivers for ARM's devices
> > > +
> > > +config DRM_HDLCD
> > > + tristate "ARM HDLCD"
> > > + depends on DRM_ARM
> > > + depends on COMMON_CLK
> > > + select COMMON_CLK_SCPI
> > > + select DMA_CMA
> > > + select DRM_KMS_CMA_HELPER
> > > + select DRM_GEM_CMA_HELPER
> > > + help
> > > +   Choose this option if you have an ARM High Definition Colour LCD
> > > +   controller.
> > > +
> > > +   If M is selected the module will be called hdlcd.
> > > +
> > > +config DRM_HDLCD_SHOW_UNDERRUN
> > > + bool "Show underrun conditions"
> > > + depends on DRM_HDLCD
> > > + default n
> > > + help
> > > +   Enable this option to show in red colour the pixels that the
> > > +   HDLCD device did not fetch from framebuffer due to underrun
> > > +   conditions.
> > > diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
> > > new file mode 100644
> > > index 000..89dcb7b
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/arm/Makefile
> > > @@ -0,0 +1,2 @@
> > > +hdlcd-y := hdlcd_drv.o hdlcd_crtc.o
> > > +obj-$(CONFIG_DRM_HDLCD)  += hdlcd.o
> > > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c 
> > > b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > > new file mode 100644
> > > index 000..350c1fe
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > > @@ -0,0 +1,334 @@
> > > +/*
> > > + * Copyright (C) 2013-2015 ARM Limited
> > > + * Author: Liviu Dudau 
> > > + *
> > > + * This file is subject to the terms and conditions of the GNU General 
> > > Public
> > > + * License.  See the file COPYING in the main directory of this archive
> > > + * for more details.
> > > + *
> > > + *  Implementation of a CRTC class for the HDLCD driver.
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > 

Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-02 Thread Daniel Vetter
On Wed, Dec 02, 2015 at 12:23:00PM +, Liviu Dudau wrote:
> The HDLCD controller is a display controller that supports resolutions
> up to 4096x4096 pixels. It is present on various development boards
> produced by ARM Ltd and emulated by the latest Fast Models from the
> company.
> 
> Cc: David Airlie 
> Cc: Robin Murphy 
> 
> Signed-off-by: Liviu Dudau 

A few small nitpicks below.

> ---
>  drivers/gpu/drm/Kconfig  |   2 +
>  drivers/gpu/drm/Makefile |   1 +
>  drivers/gpu/drm/arm/Kconfig  |  29 ++
>  drivers/gpu/drm/arm/Makefile |   2 +
>  drivers/gpu/drm/arm/hdlcd_crtc.c | 334 +++
>  drivers/gpu/drm/arm/hdlcd_drv.c  | 555 
> +++
>  drivers/gpu/drm/arm/hdlcd_drv.h  |  42 +++
>  drivers/gpu/drm/arm/hdlcd_regs.h |  87 ++
>  8 files changed, 1052 insertions(+)
>  create mode 100644 drivers/gpu/drm/arm/Kconfig
>  create mode 100644 drivers/gpu/drm/arm/Makefile
>  create mode 100644 drivers/gpu/drm/arm/hdlcd_crtc.c
>  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.c
>  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.h
>  create mode 100644 drivers/gpu/drm/arm/hdlcd_regs.h
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index c4bf9a1..3fd9124 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -106,6 +106,8 @@ config DRM_TDFX
> Choose this option if you have a 3dfx Banshee or Voodoo3 (or later),
> graphics card.  If M is selected, the module will be called tdfx.
>  
> +source "drivers/gpu/drm/arm/Kconfig"
> +
>  config DRM_R128
>   tristate "ATI Rage 128"
>   depends on DRM && PCI
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 1e9ff4c..6b42d75 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -35,6 +35,7 @@ CFLAGS_drm_trace_points.o := -I$(src)
>  
>  obj-$(CONFIG_DRM)+= drm.o
>  obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> +obj-$(CONFIG_DRM_ARM)+= arm/
>  obj-$(CONFIG_DRM_TTM)+= ttm/
>  obj-$(CONFIG_DRM_TDFX)   += tdfx/
>  obj-$(CONFIG_DRM_R128)   += r128/
> diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
> new file mode 100644
> index 000..5e8c8a8
> --- /dev/null
> +++ b/drivers/gpu/drm/arm/Kconfig
> @@ -0,0 +1,29 @@
> +config DRM_ARM
> + bool "ARM Ltd. drivers"
> + depends on DRM && OF && (ARM || ARM64)
> + select DRM_KMS_HELPER
> + help
> +   Choose this option to select drivers for ARM's devices
> +
> +config DRM_HDLCD
> + tristate "ARM HDLCD"
> + depends on DRM_ARM
> + depends on COMMON_CLK
> + select COMMON_CLK_SCPI
> + select DMA_CMA
> + select DRM_KMS_CMA_HELPER
> + select DRM_GEM_CMA_HELPER
> + help
> +   Choose this option if you have an ARM High Definition Colour LCD
> +   controller.
> +
> +   If M is selected the module will be called hdlcd.
> +
> +config DRM_HDLCD_SHOW_UNDERRUN
> + bool "Show underrun conditions"
> + depends on DRM_HDLCD
> + default n
> + help
> +   Enable this option to show in red colour the pixels that the
> +   HDLCD device did not fetch from framebuffer due to underrun
> +   conditions.
> diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
> new file mode 100644
> index 000..89dcb7b
> --- /dev/null
> +++ b/drivers/gpu/drm/arm/Makefile
> @@ -0,0 +1,2 @@
> +hdlcd-y := hdlcd_drv.o hdlcd_crtc.o
> +obj-$(CONFIG_DRM_HDLCD)  += hdlcd.o
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c 
> b/drivers/gpu/drm/arm/hdlcd_crtc.c
> new file mode 100644
> index 000..350c1fe
> --- /dev/null
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -0,0 +1,334 @@
> +/*
> + * Copyright (C) 2013-2015 ARM Limited
> + * Author: Liviu Dudau 
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file COPYING in the main directory of this archive
> + * for more details.
> + *
> + *  Implementation of a CRTC class for the HDLCD driver.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "hdlcd_drv.h"
> +#include "hdlcd_regs.h"
> +
> +/*
> + * The HDLCD controller is a dumb RGB streamer that gets connected to
> + * a single HDMI transmitter or in the case of the ARM Models it gets
> + * emulated by the software that does the actual rendering.
> + *
> + */
> +
> +static const struct drm_crtc_funcs hdlcd_crtc_funcs = {
> + .destroy = drm_crtc_cleanup,
> + .set_config = drm_atomic_helper_set_config,
> + .page_flip = drm_atomic_helper_page_flip,
> + .reset = drm_atomic_helper_crtc_reset,
> + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> 

Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-02 Thread Daniel Vetter
On Wed, Dec 02, 2015 at 04:24:19PM +0100, Daniel Vetter wrote:
> On Wed, Dec 02, 2015 at 12:23:00PM +, Liviu Dudau wrote:
> > The HDLCD controller is a display controller that supports resolutions
> > up to 4096x4096 pixels. It is present on various development boards
> > produced by ARM Ltd and emulated by the latest Fast Models from the
> > company.
> > 
> > Cc: David Airlie 
> > Cc: Robin Murphy 
> > 
> > Signed-off-by: Liviu Dudau 
> 
> A few small nitpicks below.

I've forgotten to add Acked-by: Daniel Vetter 
with these all addressed.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/Kconfig  |   2 +
> >  drivers/gpu/drm/Makefile |   1 +
> >  drivers/gpu/drm/arm/Kconfig  |  29 ++
> >  drivers/gpu/drm/arm/Makefile |   2 +
> >  drivers/gpu/drm/arm/hdlcd_crtc.c | 334 +++
> >  drivers/gpu/drm/arm/hdlcd_drv.c  | 555 
> > +++
> >  drivers/gpu/drm/arm/hdlcd_drv.h  |  42 +++
> >  drivers/gpu/drm/arm/hdlcd_regs.h |  87 ++
> >  8 files changed, 1052 insertions(+)
> >  create mode 100644 drivers/gpu/drm/arm/Kconfig
> >  create mode 100644 drivers/gpu/drm/arm/Makefile
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_crtc.c
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.c
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.h
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_regs.h
> > 
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index c4bf9a1..3fd9124 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -106,6 +106,8 @@ config DRM_TDFX
> >   Choose this option if you have a 3dfx Banshee or Voodoo3 (or later),
> >   graphics card.  If M is selected, the module will be called tdfx.
> >  
> > +source "drivers/gpu/drm/arm/Kconfig"
> > +
> >  config DRM_R128
> > tristate "ATI Rage 128"
> > depends on DRM && PCI
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 1e9ff4c..6b42d75 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -35,6 +35,7 @@ CFLAGS_drm_trace_points.o := -I$(src)
> >  
> >  obj-$(CONFIG_DRM)  += drm.o
> >  obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> > +obj-$(CONFIG_DRM_ARM)  += arm/
> >  obj-$(CONFIG_DRM_TTM)  += ttm/
> >  obj-$(CONFIG_DRM_TDFX) += tdfx/
> >  obj-$(CONFIG_DRM_R128) += r128/
> > diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
> > new file mode 100644
> > index 000..5e8c8a8
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/Kconfig
> > @@ -0,0 +1,29 @@
> > +config DRM_ARM
> > +   bool "ARM Ltd. drivers"
> > +   depends on DRM && OF && (ARM || ARM64)
> > +   select DRM_KMS_HELPER
> > +   help
> > + Choose this option to select drivers for ARM's devices
> > +
> > +config DRM_HDLCD
> > +   tristate "ARM HDLCD"
> > +   depends on DRM_ARM
> > +   depends on COMMON_CLK
> > +   select COMMON_CLK_SCPI
> > +   select DMA_CMA
> > +   select DRM_KMS_CMA_HELPER
> > +   select DRM_GEM_CMA_HELPER
> > +   help
> > + Choose this option if you have an ARM High Definition Colour LCD
> > + controller.
> > +
> > + If M is selected the module will be called hdlcd.
> > +
> > +config DRM_HDLCD_SHOW_UNDERRUN
> > +   bool "Show underrun conditions"
> > +   depends on DRM_HDLCD
> > +   default n
> > +   help
> > + Enable this option to show in red colour the pixels that the
> > + HDLCD device did not fetch from framebuffer due to underrun
> > + conditions.
> > diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
> > new file mode 100644
> > index 000..89dcb7b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/Makefile
> > @@ -0,0 +1,2 @@
> > +hdlcd-y := hdlcd_drv.o hdlcd_crtc.o
> > +obj-$(CONFIG_DRM_HDLCD)+= hdlcd.o
> > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c 
> > b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > new file mode 100644
> > index 000..350c1fe
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > @@ -0,0 +1,334 @@
> > +/*
> > + * Copyright (C) 2013-2015 ARM Limited
> > + * Author: Liviu Dudau 
> > + *
> > + * This file is subject to the terms and conditions of the GNU General 
> > Public
> > + * License.  See the file COPYING in the main directory of this archive
> > + * for more details.
> > + *
> > + *  Implementation of a CRTC class for the HDLCD driver.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "hdlcd_drv.h"
> > +#include "hdlcd_regs.h"
> > +
> > +/*
> > + * The HDLCD controller is a dumb RGB streamer that gets connected to
> > + * a single HDMI transmitter or in the case of the ARM Models it gets
> > + * emulated by the software that does the actual rendering.
> > + *
> > + */
> > +

Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-02 Thread Liviu Dudau
On Wed, Dec 02, 2015 at 04:24:19PM +0100, Daniel Vetter wrote:
> On Wed, Dec 02, 2015 at 12:23:00PM +, Liviu Dudau wrote:
> > The HDLCD controller is a display controller that supports resolutions
> > up to 4096x4096 pixels. It is present on various development boards
> > produced by ARM Ltd and emulated by the latest Fast Models from the
> > company.
> > 
> > Cc: David Airlie 
> > Cc: Robin Murphy 
> > 
> > Signed-off-by: Liviu Dudau 
> 
> A few small nitpicks below.
> 
> > ---
> >  drivers/gpu/drm/Kconfig  |   2 +
> >  drivers/gpu/drm/Makefile |   1 +
> >  drivers/gpu/drm/arm/Kconfig  |  29 ++
> >  drivers/gpu/drm/arm/Makefile |   2 +
> >  drivers/gpu/drm/arm/hdlcd_crtc.c | 334 +++
> >  drivers/gpu/drm/arm/hdlcd_drv.c  | 555 
> > +++
> >  drivers/gpu/drm/arm/hdlcd_drv.h  |  42 +++
> >  drivers/gpu/drm/arm/hdlcd_regs.h |  87 ++
> >  8 files changed, 1052 insertions(+)
> >  create mode 100644 drivers/gpu/drm/arm/Kconfig
> >  create mode 100644 drivers/gpu/drm/arm/Makefile
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_crtc.c
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.c
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_drv.h
> >  create mode 100644 drivers/gpu/drm/arm/hdlcd_regs.h
> > 
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index c4bf9a1..3fd9124 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -106,6 +106,8 @@ config DRM_TDFX
> >   Choose this option if you have a 3dfx Banshee or Voodoo3 (or later),
> >   graphics card.  If M is selected, the module will be called tdfx.
> >  
> > +source "drivers/gpu/drm/arm/Kconfig"
> > +
> >  config DRM_R128
> > tristate "ATI Rage 128"
> > depends on DRM && PCI
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 1e9ff4c..6b42d75 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -35,6 +35,7 @@ CFLAGS_drm_trace_points.o := -I$(src)
> >  
> >  obj-$(CONFIG_DRM)  += drm.o
> >  obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> > +obj-$(CONFIG_DRM_ARM)  += arm/
> >  obj-$(CONFIG_DRM_TTM)  += ttm/
> >  obj-$(CONFIG_DRM_TDFX) += tdfx/
> >  obj-$(CONFIG_DRM_R128) += r128/
> > diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
> > new file mode 100644
> > index 000..5e8c8a8
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/Kconfig
> > @@ -0,0 +1,29 @@
> > +config DRM_ARM
> > +   bool "ARM Ltd. drivers"
> > +   depends on DRM && OF && (ARM || ARM64)
> > +   select DRM_KMS_HELPER
> > +   help
> > + Choose this option to select drivers for ARM's devices
> > +
> > +config DRM_HDLCD
> > +   tristate "ARM HDLCD"
> > +   depends on DRM_ARM
> > +   depends on COMMON_CLK
> > +   select COMMON_CLK_SCPI
> > +   select DMA_CMA
> > +   select DRM_KMS_CMA_HELPER
> > +   select DRM_GEM_CMA_HELPER
> > +   help
> > + Choose this option if you have an ARM High Definition Colour LCD
> > + controller.
> > +
> > + If M is selected the module will be called hdlcd.
> > +
> > +config DRM_HDLCD_SHOW_UNDERRUN
> > +   bool "Show underrun conditions"
> > +   depends on DRM_HDLCD
> > +   default n
> > +   help
> > + Enable this option to show in red colour the pixels that the
> > + HDLCD device did not fetch from framebuffer due to underrun
> > + conditions.
> > diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
> > new file mode 100644
> > index 000..89dcb7b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/Makefile
> > @@ -0,0 +1,2 @@
> > +hdlcd-y := hdlcd_drv.o hdlcd_crtc.o
> > +obj-$(CONFIG_DRM_HDLCD)+= hdlcd.o
> > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c 
> > b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > new file mode 100644
> > index 000..350c1fe
> > --- /dev/null
> > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > @@ -0,0 +1,334 @@
> > +/*
> > + * Copyright (C) 2013-2015 ARM Limited
> > + * Author: Liviu Dudau 
> > + *
> > + * This file is subject to the terms and conditions of the GNU General 
> > Public
> > + * License.  See the file COPYING in the main directory of this archive
> > + * for more details.
> > + *
> > + *  Implementation of a CRTC class for the HDLCD driver.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "hdlcd_drv.h"
> > +#include "hdlcd_regs.h"
> > +
> > +/*
> > + * The HDLCD controller is a dumb RGB streamer that gets connected to
> > + * a single HDMI transmitter or in the case of the ARM Models it gets
> > + * emulated by the software that does the actual rendering.
> > + *
> > + */
> > +
> > +static const struct drm_crtc_funcs hdlcd_crtc_funcs = {
> > +   .destroy = drm_crtc_cleanup,
> > +   

Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.

2015-12-02 Thread Daniel Stone
Hi Liviu,

On 2 December 2015 at 12:23, Liviu Dudau  wrote:
> +   if (irq_status & HDLCD_INTERRUPT_VSYNC) {
> +   unsigned long flags;
> +
> +   drm_handle_vblank(drm, 0);
> +
> +   spin_lock_irqsave(>event_lock, flags);
> +   if (hdlcd->event) {
> +   drm_send_vblank_event(drm, hdlcd->event->pipe, 
> hdlcd->event);
> +   drm_crtc_vblank_put(>crtc);
> +   hdlcd->event = NULL;
> +   }
> +   spin_unlock_irqrestore(>event_lock, flags);
> +   }

As with VC4 and Rockchip, you're missing a ->preclose handler in your
drm_drv, to make sure that you don't try to send events to a dead
client (which causes an OOPS):
https://git.collabora.com/cgit/user/daniels/linux.git/commit/?h=wip/4.4.x/rockchip-drm-fixes=d14f21bcd7
(and its parent)

Also, is there anything preventing clients from submitting multiple
pageflips before the event is sent? I couldn't see anything from a
quick look, so you could have the situation of:
  - client submits pageflip, event 1 stored to hdlcd->event
  - client submits pageflip, event 2 stored to hdlcd->event
  - vblank arrives, event 2 is sent
  - event 1 has disappeared and been leaked

Cheers,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/