RE: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
> -Original Message- > From: Daniel Vetter > Sent: Monday, October 5, 2020 6:49 AM > To: Chrisanthus, Anitha > Cc: Daniel Vetter ; dri-devel@lists.freedesktop.org; > Paauwe, Bob J ; Dea, Edmund J > ; Vetter, Daniel > Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display > > On Sat, Oct 03, 2020 at 04:58:51AM +, Chrisanthus, Anitha wrote: > > > > > > > -Original Message- > > > From: Daniel Vetter > > > Sent: Friday, October 2, 2020 2:19 AM > > > To: Chrisanthus, Anitha > > > Cc: dri-devel@lists.freedesktop.org; Paauwe, Bob J > > > ; Dea, Edmund J ; > > > Vetter, Daniel > > > Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay > Display > > > > > > On Thu, Oct 1, 2020 at 8:02 PM Chrisanthus, Anitha > > > wrote: > > > > > > > > Hi Daniel, > > > > I was finally able to test the driver with 5.9 kernel with minor changes > in > > > the driver. > > > > Ran the igt_vblank test and it passed/skipped all the subtests except > the > > > pipe-A-accuracy-idle test. > > > > Results are attached. Investigated the failing test and it seems like > > > drm_handle_vblank() is taking too long sometimes. I can work on this > after > > > we merge. > > > > > > kms_flip is the one that should check whether vblank events and page > > > flip events agree. Which I think isn't the case with your current > > > code. > > Thanks. I will run that test and work on the failures. I did send v8 > > with 5.9 testing changes today and also sent the YAML file v1 today to > > devicet...@vger.kernel.org, not sure if I should cc dri-devel,, should > > I? > > I think usually people send out the entire series, first device tree yaml, > then bindings, then drivers, to everyone. It helps with context I think > (But I don't review dt stuff myself). Thanks, I will wait for review for DT YAML and will include dri-devel if v2 is needed. Also attaching dt binding yaml patch. I am still awaiting approval for push access to drm-misc, I think there is some confusion on what this driver is. This was requested for v7. Can you grant access? https://gitlab.freedesktop.org/freedesktop/freedesktop/-/issues/291 Anitha > -Daniel > > > > > Anitha > > > -Daniel > > > > > > > > > > > Will send out V8 with the minor changes and device tree YAML > binding > > > file ASAP. Will work on the rest of the review comments after. > > > > > > > > Thanks, > > > > Anitha > > > > > > > > > From: Daniel Vetter > > > > > Sent: Thursday, September 10, 2020 1:31 AM > > > > > To: Chrisanthus, Anitha > > > > > Cc: dri-devel@lists.freedesktop.org; Paauwe, Bob J > > > > > ; Dea, Edmund J > ; > > > > > Vetter, Daniel > > > > > Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay > > > Display > > > > > > > > > > On Mon, Aug 31, 2020 at 01:02:50PM -0700, Anitha Chrisanthus > > > wrote: > > > > > > This is a basic KMS atomic modesetting display driver for KeemBay > > > family > > > > > of > > > > > > SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge > > > > > > driver at the connector level. > > > > > > > > > > > > Single CRTC with LCD controller->mipi DSI-> ADV bridge > > > > > > > > > > > > Only 1080p resolution and single plane is supported at this time. > > > > > > > > > > > > v2: moved extern to .h, removed license text > > > > > > use drm_dev_init, upclassed dev_private, removed > > > HAVE_IRQ.(Sam) > > > > > > > > > > > > v3: Squashed all 59 commits to one > > > > > > > > > > > > v4: review changes from Sam Ravnborg > > > > > > renamed dev_p to kmb > > > > > > moved clocks under kmb_clock, consolidated clk initializations > > > > > > use drmm functions > > > > > > use DRM_GEM_CMA_DRIVER_OPS_VMAP > > > > > > > > > > > > v5: corrected spellings > > > > > > v6: corrected checkpatch warnings > > > > > > v7: review changes Sam Ravnborg and Thomas Zimmerman > > > > > > removed kmb_crtc.h kmb_crtc_cleanup (Thomas) > > > > > >
Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
On Sat, Oct 03, 2020 at 04:58:51AM +, Chrisanthus, Anitha wrote: > > > > -Original Message- > > From: Daniel Vetter > > Sent: Friday, October 2, 2020 2:19 AM > > To: Chrisanthus, Anitha > > Cc: dri-devel@lists.freedesktop.org; Paauwe, Bob J > > ; Dea, Edmund J ; > > Vetter, Daniel > > Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display > > > > On Thu, Oct 1, 2020 at 8:02 PM Chrisanthus, Anitha > > wrote: > > > > > > Hi Daniel, > > > I was finally able to test the driver with 5.9 kernel with minor changes > > > in > > the driver. > > > Ran the igt_vblank test and it passed/skipped all the subtests except the > > pipe-A-accuracy-idle test. > > > Results are attached. Investigated the failing test and it seems like > > drm_handle_vblank() is taking too long sometimes. I can work on this after > > we merge. > > > > kms_flip is the one that should check whether vblank events and page > > flip events agree. Which I think isn't the case with your current > > code. > Thanks. I will run that test and work on the failures. I did send v8 > with 5.9 testing changes today and also sent the YAML file v1 today to > devicet...@vger.kernel.org, not sure if I should cc dri-devel,, should > I? I think usually people send out the entire series, first device tree yaml, then bindings, then drivers, to everyone. It helps with context I think (But I don't review dt stuff myself). -Daniel > > Anitha > > -Daniel > > > > > > > > Will send out V8 with the minor changes and device tree YAML binding > > file ASAP. Will work on the rest of the review comments after. > > > > > > Thanks, > > > Anitha > > > > > > > From: Daniel Vetter > > > > Sent: Thursday, September 10, 2020 1:31 AM > > > > To: Chrisanthus, Anitha > > > > Cc: dri-devel@lists.freedesktop.org; Paauwe, Bob J > > > > ; Dea, Edmund J ; > > > > Vetter, Daniel > > > > Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay > > Display > > > > > > > > On Mon, Aug 31, 2020 at 01:02:50PM -0700, Anitha Chrisanthus > > wrote: > > > > > This is a basic KMS atomic modesetting display driver for KeemBay > > family > > > > of > > > > > SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge > > > > > driver at the connector level. > > > > > > > > > > Single CRTC with LCD controller->mipi DSI-> ADV bridge > > > > > > > > > > Only 1080p resolution and single plane is supported at this time. > > > > > > > > > > v2: moved extern to .h, removed license text > > > > > use drm_dev_init, upclassed dev_private, removed > > HAVE_IRQ.(Sam) > > > > > > > > > > v3: Squashed all 59 commits to one > > > > > > > > > > v4: review changes from Sam Ravnborg > > > > > renamed dev_p to kmb > > > > > moved clocks under kmb_clock, consolidated clk initializations > > > > > use drmm functions > > > > > use DRM_GEM_CMA_DRIVER_OPS_VMAP > > > > > > > > > > v5: corrected spellings > > > > > v6: corrected checkpatch warnings > > > > > v7: review changes Sam Ravnborg and Thomas Zimmerman > > > > > removed kmb_crtc.h kmb_crtc_cleanup (Thomas) > > > > > renamed mode_set, kmb_load, inlined unload (Thomas) > > > > > moved remaining logging to drm_*(Thomas) > > > > > re-orged driver initialization (Thomas) > > > > > moved plane_status to drm_private (Sam) > > > > > removed unnecessary logs and defines and ifdef codes (Sam) > > > > > call helper_check in plane_atomic_check (Sam) > > > > > renamed set to get for bpp and format functions(Sam) > > > > > use drm helper functions for reset, duplicate/destroy state > > > > > instead > > > > > of kmb functions (Sam) > > > > > removed kmb_priv from kmb_plane and removed > > kmb_plane_state > > > > (Sam) > > > > > > > > > > Cc: Sam Ravnborg > > > > > Signed-off-by: Anitha Chrisanthus > > > > > Reviewed-by: Bob Paauwe > > > > > > > > Sam asked me to take a look at this too, looks reasonable overall. I've > > > > spotted a few smal
RE: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
> -Original Message- > From: Daniel Vetter > Sent: Friday, October 2, 2020 2:19 AM > To: Chrisanthus, Anitha > Cc: dri-devel@lists.freedesktop.org; Paauwe, Bob J > ; Dea, Edmund J ; > Vetter, Daniel > Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display > > On Thu, Oct 1, 2020 at 8:02 PM Chrisanthus, Anitha > wrote: > > > > Hi Daniel, > > I was finally able to test the driver with 5.9 kernel with minor changes in > the driver. > > Ran the igt_vblank test and it passed/skipped all the subtests except the > pipe-A-accuracy-idle test. > > Results are attached. Investigated the failing test and it seems like > drm_handle_vblank() is taking too long sometimes. I can work on this after > we merge. > > kms_flip is the one that should check whether vblank events and page > flip events agree. Which I think isn't the case with your current > code. Thanks. I will run that test and work on the failures. I did send v8 with 5.9 testing changes today and also sent the YAML file v1 today to devicet...@vger.kernel.org, not sure if I should cc dri-devel,, should I? Anitha > -Daniel > > > > > Will send out V8 with the minor changes and device tree YAML binding > file ASAP. Will work on the rest of the review comments after. > > > > Thanks, > > Anitha > > > > > From: Daniel Vetter > > > Sent: Thursday, September 10, 2020 1:31 AM > > > To: Chrisanthus, Anitha > > > Cc: dri-devel@lists.freedesktop.org; Paauwe, Bob J > > > ; Dea, Edmund J ; > > > Vetter, Daniel > > > Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay > Display > > > > > > On Mon, Aug 31, 2020 at 01:02:50PM -0700, Anitha Chrisanthus > wrote: > > > > This is a basic KMS atomic modesetting display driver for KeemBay > family > > > of > > > > SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge > > > > driver at the connector level. > > > > > > > > Single CRTC with LCD controller->mipi DSI-> ADV bridge > > > > > > > > Only 1080p resolution and single plane is supported at this time. > > > > > > > > v2: moved extern to .h, removed license text > > > > use drm_dev_init, upclassed dev_private, removed > HAVE_IRQ.(Sam) > > > > > > > > v3: Squashed all 59 commits to one > > > > > > > > v4: review changes from Sam Ravnborg > > > > renamed dev_p to kmb > > > > moved clocks under kmb_clock, consolidated clk initializations > > > > use drmm functions > > > > use DRM_GEM_CMA_DRIVER_OPS_VMAP > > > > > > > > v5: corrected spellings > > > > v6: corrected checkpatch warnings > > > > v7: review changes Sam Ravnborg and Thomas Zimmerman > > > > removed kmb_crtc.h kmb_crtc_cleanup (Thomas) > > > > renamed mode_set, kmb_load, inlined unload (Thomas) > > > > moved remaining logging to drm_*(Thomas) > > > > re-orged driver initialization (Thomas) > > > > moved plane_status to drm_private (Sam) > > > > removed unnecessary logs and defines and ifdef codes (Sam) > > > > call helper_check in plane_atomic_check (Sam) > > > > renamed set to get for bpp and format functions(Sam) > > > > use drm helper functions for reset, duplicate/destroy state instead > > > > of kmb functions (Sam) > > > > removed kmb_priv from kmb_plane and removed > kmb_plane_state > > > (Sam) > > > > > > > > Cc: Sam Ravnborg > > > > Signed-off-by: Anitha Chrisanthus > > > > Reviewed-by: Bob Paauwe > > > > > > Sam asked me to take a look at this too, looks reasonable overall. I've > > > spotted a few small things plus a potential functional issue around > > > vblank/event handling. I strongly recommend running the igt test suite > > > over the driver to see whether it all works reasonably ok. See below for > > > details. > > > > > > > --- > > > > drivers/gpu/drm/kmb/kmb_crtc.c | 224 + > > > > drivers/gpu/drm/kmb/kmb_drv.c | 676 > > > > > > > drivers/gpu/drm/kmb/kmb_drv.h | 170 ++ > > > > drivers/gpu/drm/kmb/kmb_plane.c | 480 > > > > > > > drivers/gpu/drm/kmb/kmb_plane.h | 110 +++ > > > > 5 files changed, 1660 insertions(+) > > >
Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
On Thu, Oct 1, 2020 at 8:02 PM Chrisanthus, Anitha wrote: > > Hi Daniel, > I was finally able to test the driver with 5.9 kernel with minor changes in > the driver. > Ran the igt_vblank test and it passed/skipped all the subtests except the > pipe-A-accuracy-idle test. > Results are attached. Investigated the failing test and it seems like > drm_handle_vblank() is taking too long sometimes. I can work on this after we > merge. kms_flip is the one that should check whether vblank events and page flip events agree. Which I think isn't the case with your current code. -Daniel > > Will send out V8 with the minor changes and device tree YAML binding file > ASAP. Will work on the rest of the review comments after. > > Thanks, > Anitha > > > From: Daniel Vetter > > Sent: Thursday, September 10, 2020 1:31 AM > > To: Chrisanthus, Anitha > > Cc: dri-devel@lists.freedesktop.org; Paauwe, Bob J > > ; Dea, Edmund J ; > > Vetter, Daniel > > Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display > > > > On Mon, Aug 31, 2020 at 01:02:50PM -0700, Anitha Chrisanthus wrote: > > > This is a basic KMS atomic modesetting display driver for KeemBay family > > of > > > SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge > > > driver at the connector level. > > > > > > Single CRTC with LCD controller->mipi DSI-> ADV bridge > > > > > > Only 1080p resolution and single plane is supported at this time. > > > > > > v2: moved extern to .h, removed license text > > > use drm_dev_init, upclassed dev_private, removed HAVE_IRQ.(Sam) > > > > > > v3: Squashed all 59 commits to one > > > > > > v4: review changes from Sam Ravnborg > > > renamed dev_p to kmb > > > moved clocks under kmb_clock, consolidated clk initializations > > > use drmm functions > > > use DRM_GEM_CMA_DRIVER_OPS_VMAP > > > > > > v5: corrected spellings > > > v6: corrected checkpatch warnings > > > v7: review changes Sam Ravnborg and Thomas Zimmerman > > > removed kmb_crtc.h kmb_crtc_cleanup (Thomas) > > > renamed mode_set, kmb_load, inlined unload (Thomas) > > > moved remaining logging to drm_*(Thomas) > > > re-orged driver initialization (Thomas) > > > moved plane_status to drm_private (Sam) > > > removed unnecessary logs and defines and ifdef codes (Sam) > > > call helper_check in plane_atomic_check (Sam) > > > renamed set to get for bpp and format functions(Sam) > > > use drm helper functions for reset, duplicate/destroy state instead > > > of kmb functions (Sam) > > > removed kmb_priv from kmb_plane and removed kmb_plane_state > > (Sam) > > > > > > Cc: Sam Ravnborg > > > Signed-off-by: Anitha Chrisanthus > > > Reviewed-by: Bob Paauwe > > > > Sam asked me to take a look at this too, looks reasonable overall. I've > > spotted a few small things plus a potential functional issue around > > vblank/event handling. I strongly recommend running the igt test suite > > over the driver to see whether it all works reasonably ok. See below for > > details. > > > > > --- > > > drivers/gpu/drm/kmb/kmb_crtc.c | 224 + > > > drivers/gpu/drm/kmb/kmb_drv.c | 676 > > > > > drivers/gpu/drm/kmb/kmb_drv.h | 170 ++ > > > drivers/gpu/drm/kmb/kmb_plane.c | 480 > > > > > drivers/gpu/drm/kmb/kmb_plane.h | 110 +++ > > > 5 files changed, 1660 insertions(+) > > > create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.c > > > create mode 100644 drivers/gpu/drm/kmb/kmb_drv.c > > > create mode 100644 drivers/gpu/drm/kmb/kmb_drv.h > > > create mode 100644 drivers/gpu/drm/kmb/kmb_plane.c > > > create mode 100644 drivers/gpu/drm/kmb/kmb_plane.h > > > > > > diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c > > b/drivers/gpu/drm/kmb/kmb_crtc.c > > > new file mode 100644 > > > index 000..a684331 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/kmb/kmb_crtc.c > > > @@ -0,0 +1,224 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Copyright (c) 2018-2020 Intel Corporation > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > + > > > +#include > > > + > > > +#include > &g
RE: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
Hi Daniel,
I was finally able to test the driver with 5.9 kernel with minor changes in the
driver.
Ran the igt_vblank test and it passed/skipped all the subtests except the
pipe-A-accuracy-idle test.
Results are attached. Investigated the failing test and it seems like
drm_handle_vblank() is taking too long sometimes. I can work on this after we
merge.
Will send out V8 with the minor changes and device tree YAML binding file ASAP.
Will work on the rest of the review comments after.
Thanks,
Anitha
> From: Daniel Vetter
> Sent: Thursday, September 10, 2020 1:31 AM
> To: Chrisanthus, Anitha
> Cc: dri-devel@lists.freedesktop.org; Paauwe, Bob J
> ; Dea, Edmund J ;
> Vetter, Daniel
> Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
>
> On Mon, Aug 31, 2020 at 01:02:50PM -0700, Anitha Chrisanthus wrote:
> > This is a basic KMS atomic modesetting display driver for KeemBay family
> of
> > SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge
> > driver at the connector level.
> >
> > Single CRTC with LCD controller->mipi DSI-> ADV bridge
> >
> > Only 1080p resolution and single plane is supported at this time.
> >
> > v2: moved extern to .h, removed license text
> > use drm_dev_init, upclassed dev_private, removed HAVE_IRQ.(Sam)
> >
> > v3: Squashed all 59 commits to one
> >
> > v4: review changes from Sam Ravnborg
> > renamed dev_p to kmb
> > moved clocks under kmb_clock, consolidated clk initializations
> > use drmm functions
> > use DRM_GEM_CMA_DRIVER_OPS_VMAP
> >
> > v5: corrected spellings
> > v6: corrected checkpatch warnings
> > v7: review changes Sam Ravnborg and Thomas Zimmerman
> > removed kmb_crtc.h kmb_crtc_cleanup (Thomas)
> > renamed mode_set, kmb_load, inlined unload (Thomas)
> > moved remaining logging to drm_*(Thomas)
> > re-orged driver initialization (Thomas)
> > moved plane_status to drm_private (Sam)
> > removed unnecessary logs and defines and ifdef codes (Sam)
> > call helper_check in plane_atomic_check (Sam)
> > renamed set to get for bpp and format functions(Sam)
> > use drm helper functions for reset, duplicate/destroy state instead
> > of kmb functions (Sam)
> > removed kmb_priv from kmb_plane and removed kmb_plane_state
> (Sam)
> >
> > Cc: Sam Ravnborg
> > Signed-off-by: Anitha Chrisanthus
> > Reviewed-by: Bob Paauwe
>
> Sam asked me to take a look at this too, looks reasonable overall. I've
> spotted a few small things plus a potential functional issue around
> vblank/event handling. I strongly recommend running the igt test suite
> over the driver to see whether it all works reasonably ok. See below for
> details.
>
> > ---
> > drivers/gpu/drm/kmb/kmb_crtc.c | 224 +
> > drivers/gpu/drm/kmb/kmb_drv.c | 676
>
> > drivers/gpu/drm/kmb/kmb_drv.h | 170 ++
> > drivers/gpu/drm/kmb/kmb_plane.c | 480
>
> > drivers/gpu/drm/kmb/kmb_plane.h | 110 +++
> > 5 files changed, 1660 insertions(+)
> > create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.c
> > create mode 100644 drivers/gpu/drm/kmb/kmb_drv.c
> > create mode 100644 drivers/gpu/drm/kmb/kmb_drv.h
> > create mode 100644 drivers/gpu/drm/kmb/kmb_plane.c
> > create mode 100644 drivers/gpu/drm/kmb/kmb_plane.h
> >
> > diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c
> b/drivers/gpu/drm/kmb/kmb_crtc.c
> > new file mode 100644
> > index 000..a684331
> > --- /dev/null
> > +++ b/drivers/gpu/drm/kmb/kmb_crtc.c
> > @@ -0,0 +1,224 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2018-2020 Intel Corporation
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "kmb_drv.h"
> > +#include "kmb_dsi.h"
> > +#include "kmb_plane.h"
> > +#include "kmb_regs.h"
> > +
> > +struct kmb_crtc_timing {
> > + u32 vfront_porch;
> > + u32 vback_porch;
> > + u32 vsync_len;
> > + u32 hfront_porch;
> > + u32 hback_porch;
> > + u32 hsync_len;
> > +};
> > +
> > +static int kmb_crtc_enable_vbla
Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
On Tue, Sep 22, 2020 at 06:20:02PM +, Chrisanthus, Anitha wrote: > Hi Daniel, > Thank you for your review. Keem Bay is developed on kernel 5.45 (Yocto) > and I am working with the program to get a working 5.9 kernel for Keem > Bay and will test this driver when that becomes available. Yeah 5.4 is a bit old for upstreaming since a bunch of drm changes aren't available there. Having an upstream kernel to test your submission is indeed needed first. -Daniel > > Thanks, > Anitha > > > > -Original Message- > > From: Daniel Vetter > > Sent: Thursday, September 10, 2020 1:31 AM > > To: Chrisanthus, Anitha > > Cc: dri-devel@lists.freedesktop.org; Paauwe, Bob J > > ; Dea, Edmund J ; > > Vetter, Daniel > > Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display > > > > On Mon, Aug 31, 2020 at 01:02:50PM -0700, Anitha Chrisanthus wrote: > > > This is a basic KMS atomic modesetting display driver for KeemBay family > > of > > > SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge > > > driver at the connector level. > > > > > > Single CRTC with LCD controller->mipi DSI-> ADV bridge > > > > > > Only 1080p resolution and single plane is supported at this time. > > > > > > v2: moved extern to .h, removed license text > > > use drm_dev_init, upclassed dev_private, removed HAVE_IRQ.(Sam) > > > > > > v3: Squashed all 59 commits to one > > > > > > v4: review changes from Sam Ravnborg > > > renamed dev_p to kmb > > > moved clocks under kmb_clock, consolidated clk initializations > > > use drmm functions > > > use DRM_GEM_CMA_DRIVER_OPS_VMAP > > > > > > v5: corrected spellings > > > v6: corrected checkpatch warnings > > > v7: review changes Sam Ravnborg and Thomas Zimmerman > > > removed kmb_crtc.h kmb_crtc_cleanup (Thomas) > > > renamed mode_set, kmb_load, inlined unload (Thomas) > > > moved remaining logging to drm_*(Thomas) > > > re-orged driver initialization (Thomas) > > > moved plane_status to drm_private (Sam) > > > removed unnecessary logs and defines and ifdef codes (Sam) > > > call helper_check in plane_atomic_check (Sam) > > > renamed set to get for bpp and format functions(Sam) > > > use drm helper functions for reset, duplicate/destroy state instead > > > of kmb functions (Sam) > > > removed kmb_priv from kmb_plane and removed kmb_plane_state > > (Sam) > > > > > > Cc: Sam Ravnborg > > > Signed-off-by: Anitha Chrisanthus > > > Reviewed-by: Bob Paauwe > > > > Sam asked me to take a look at this too, looks reasonable overall. I've > > spotted a few small things plus a potential functional issue around > > vblank/event handling. I strongly recommend running the igt test suite > > over the driver to see whether it all works reasonably ok. See below for > > details. > > > > > --- > > > drivers/gpu/drm/kmb/kmb_crtc.c | 224 + > > > drivers/gpu/drm/kmb/kmb_drv.c | 676 > > > > > drivers/gpu/drm/kmb/kmb_drv.h | 170 ++ > > > drivers/gpu/drm/kmb/kmb_plane.c | 480 > > > > > drivers/gpu/drm/kmb/kmb_plane.h | 110 +++ > > > 5 files changed, 1660 insertions(+) > > > create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.c > > > create mode 100644 drivers/gpu/drm/kmb/kmb_drv.c > > > create mode 100644 drivers/gpu/drm/kmb/kmb_drv.h > > > create mode 100644 drivers/gpu/drm/kmb/kmb_plane.c > > > create mode 100644 drivers/gpu/drm/kmb/kmb_plane.h > > > > > > diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c > > b/drivers/gpu/drm/kmb/kmb_crtc.c > > > new file mode 100644 > > > index 000..a684331 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/kmb/kmb_crtc.c > > > @@ -0,0 +1,224 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Copyright (c) 2018-2020 Intel Corporation > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > + > > > +#include > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > >
RE: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
Hi Daniel,
Thank you for your review. Keem Bay is developed on kernel 5.45 (Yocto) and I
am working with the program to get a working 5.9 kernel for Keem Bay and will
test this driver when that becomes available.
Thanks,
Anitha
> -Original Message-
> From: Daniel Vetter
> Sent: Thursday, September 10, 2020 1:31 AM
> To: Chrisanthus, Anitha
> Cc: dri-devel@lists.freedesktop.org; Paauwe, Bob J
> ; Dea, Edmund J ;
> Vetter, Daniel
> Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
>
> On Mon, Aug 31, 2020 at 01:02:50PM -0700, Anitha Chrisanthus wrote:
> > This is a basic KMS atomic modesetting display driver for KeemBay family
> of
> > SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge
> > driver at the connector level.
> >
> > Single CRTC with LCD controller->mipi DSI-> ADV bridge
> >
> > Only 1080p resolution and single plane is supported at this time.
> >
> > v2: moved extern to .h, removed license text
> > use drm_dev_init, upclassed dev_private, removed HAVE_IRQ.(Sam)
> >
> > v3: Squashed all 59 commits to one
> >
> > v4: review changes from Sam Ravnborg
> > renamed dev_p to kmb
> > moved clocks under kmb_clock, consolidated clk initializations
> > use drmm functions
> > use DRM_GEM_CMA_DRIVER_OPS_VMAP
> >
> > v5: corrected spellings
> > v6: corrected checkpatch warnings
> > v7: review changes Sam Ravnborg and Thomas Zimmerman
> > removed kmb_crtc.h kmb_crtc_cleanup (Thomas)
> > renamed mode_set, kmb_load, inlined unload (Thomas)
> > moved remaining logging to drm_*(Thomas)
> > re-orged driver initialization (Thomas)
> > moved plane_status to drm_private (Sam)
> > removed unnecessary logs and defines and ifdef codes (Sam)
> > call helper_check in plane_atomic_check (Sam)
> > renamed set to get for bpp and format functions(Sam)
> > use drm helper functions for reset, duplicate/destroy state instead
> > of kmb functions (Sam)
> > removed kmb_priv from kmb_plane and removed kmb_plane_state
> (Sam)
> >
> > Cc: Sam Ravnborg
> > Signed-off-by: Anitha Chrisanthus
> > Reviewed-by: Bob Paauwe
>
> Sam asked me to take a look at this too, looks reasonable overall. I've
> spotted a few small things plus a potential functional issue around
> vblank/event handling. I strongly recommend running the igt test suite
> over the driver to see whether it all works reasonably ok. See below for
> details.
>
> > ---
> > drivers/gpu/drm/kmb/kmb_crtc.c | 224 +
> > drivers/gpu/drm/kmb/kmb_drv.c | 676
>
> > drivers/gpu/drm/kmb/kmb_drv.h | 170 ++
> > drivers/gpu/drm/kmb/kmb_plane.c | 480
>
> > drivers/gpu/drm/kmb/kmb_plane.h | 110 +++
> > 5 files changed, 1660 insertions(+)
> > create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.c
> > create mode 100644 drivers/gpu/drm/kmb/kmb_drv.c
> > create mode 100644 drivers/gpu/drm/kmb/kmb_drv.h
> > create mode 100644 drivers/gpu/drm/kmb/kmb_plane.c
> > create mode 100644 drivers/gpu/drm/kmb/kmb_plane.h
> >
> > diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c
> b/drivers/gpu/drm/kmb/kmb_crtc.c
> > new file mode 100644
> > index 000..a684331
> > --- /dev/null
> > +++ b/drivers/gpu/drm/kmb/kmb_crtc.c
> > @@ -0,0 +1,224 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2018-2020 Intel Corporation
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "kmb_drv.h"
> > +#include "kmb_dsi.h"
> > +#include "kmb_plane.h"
> > +#include "kmb_regs.h"
> > +
> > +struct kmb_crtc_timing {
> > + u32 vfront_porch;
> > + u32 vback_porch;
> > + u32 vsync_len;
> > + u32 hfront_porch;
> > + u32 hback_porch;
> > + u32 hsync_len;
> > +};
> > +
> > +static int kmb_crtc_enable_vblank(struct drm_crtc *crtc)
> > +{
> > + struct drm_device *dev = crtc->dev;
> > + struct kmb_drm_private *kmb = to_kmb(dev);
> > +
> > + /* Clear interrupt */
> > + kmb_write
Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
Hi Anitha. On Fri, Sep 11, 2020 at 08:54:41PM +, Chrisanthus, Anitha wrote: > Hi Neil, > Thanks for your review. Is a device tree binding document like this one > enough? Entries for kmb-drm are similar to this. > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/gpu/brcm%2Cbcm-v3d.txt > > How do I submit it once I have it ready? Please take a look at: Documentation/devicetree/binding/submitting-patches.rst That should explain it all. New bindings must be in DT Schema foramt (.yaml files). You can be assured that if you fail to do a dt_binding_check there are syntax erros - speaking of experience. So you need to get the tooling up and running. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
Hi Neil,
Thanks for your review. Is a device tree binding document like this one enough?
Entries for kmb-drm are similar to this.
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/gpu/brcm%2Cbcm-v3d.txt
How do I submit it once I have it ready?
Thanks,
Anitha
> -Original Message-
> From: Neil Armstrong
> Sent: Thursday, September 10, 2020 2:33 AM
> To: Chrisanthus, Anitha ; dri-
> de...@lists.freedesktop.org; Paauwe, Bob J ;
> Dea, Edmund J
> Cc: Vetter, Daniel
> Subject: Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
>
> On 31/08/2020 22:02, Anitha Chrisanthus wrote:
> > This is a basic KMS atomic modesetting display driver for KeemBay family
> of
> > SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge
> > driver at the connector level.
> >
> > Single CRTC with LCD controller->mipi DSI-> ADV bridge
> >
> > Only 1080p resolution and single plane is supported at this time.
> >
> > v2: moved extern to .h, removed license text
> > use drm_dev_init, upclassed dev_private, removed HAVE_IRQ.(Sam)
> >
> > v3: Squashed all 59 commits to one
> >
> > v4: review changes from Sam Ravnborg
> > renamed dev_p to kmb
> > moved clocks under kmb_clock, consolidated clk initializations
> > use drmm functions
> > use DRM_GEM_CMA_DRIVER_OPS_VMAP
> >
> > v5: corrected spellings
> > v6: corrected checkpatch warnings
> > v7: review changes Sam Ravnborg and Thomas Zimmerman
> > removed kmb_crtc.h kmb_crtc_cleanup (Thomas)
> > renamed mode_set, kmb_load, inlined unload (Thomas)
> > moved remaining logging to drm_*(Thomas)
> > re-orged driver initialization (Thomas)
> > moved plane_status to drm_private (Sam)
> > removed unnecessary logs and defines and ifdef codes (Sam)
> > call helper_check in plane_atomic_check (Sam)
> > renamed set to get for bpp and format functions(Sam)
> > use drm helper functions for reset, duplicate/destroy state instead
> > of kmb functions (Sam)
> > removed kmb_priv from kmb_plane and removed kmb_plane_state
> (Sam)
> >
> > Cc: Sam Ravnborg
> > Signed-off-by: Anitha Chrisanthus
> > Reviewed-by: Bob Paauwe
> > ---
> > drivers/gpu/drm/kmb/kmb_crtc.c | 224 +
> > drivers/gpu/drm/kmb/kmb_drv.c | 676
>
> > drivers/gpu/drm/kmb/kmb_drv.h | 170 ++
> > drivers/gpu/drm/kmb/kmb_plane.c | 480
>
> > drivers/gpu/drm/kmb/kmb_plane.h | 110 +++
> > 5 files changed, 1660 insertions(+)
> > create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.c
> > create mode 100644 drivers/gpu/drm/kmb/kmb_drv.c
> > create mode 100644 drivers/gpu/drm/kmb/kmb_drv.h
> > create mode 100644 drivers/gpu/drm/kmb/kmb_plane.c
> > create mode 100644 drivers/gpu/drm/kmb/kmb_plane.h
> >
>
> [...]
>
> > +
> > +static const struct of_device_id kmb_of_match[] = {
> > + {.compatible = "intel,kmb_display"},
> > + {},
> > +};
>
> As I already commented on v1, a proper YAML bindings files
> is mandatory here, to check if the bindings are correct and if
> the drivers uses them correctly (port/endpoints, etc..)
>
> Neil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
On Thu, Sep 10, 2020 at 08:33:43PM +0200, Sam Ravnborg wrote: > Hi Daniel. > > > > + } > > > + > > > + /* Initialize MIPI DSI */ > > > + ret = kmb_dsi_init(drm, adv_bridge); > > > > Split up isn't correct here, this won't compile since the dsi code isn't > > in this patch yet. So you need to have this ordered the other way round. > > I have explicit asked for a simple file split as done here > and told Anitha this was fine. > > As the kbuild/kconfig bits are the last patch then no build breakage. > The file based split was IMO fine to ease review. > > If this is not OK then blame me :-) Ah I missed that, sounds ok for me too then :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
Hi Daniel. > > + } > > + > > + /* Initialize MIPI DSI */ > > + ret = kmb_dsi_init(drm, adv_bridge); > > Split up isn't correct here, this won't compile since the dsi code isn't > in this patch yet. So you need to have this ordered the other way round. I have explicit asked for a simple file split as done here and told Anitha this was fine. As the kbuild/kconfig bits are the last patch then no build breakage. The file based split was IMO fine to ease review. If this is not OK then blame me :-) Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
On 31/08/2020 22:02, Anitha Chrisanthus wrote:
> This is a basic KMS atomic modesetting display driver for KeemBay family of
> SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge
> driver at the connector level.
>
> Single CRTC with LCD controller->mipi DSI-> ADV bridge
>
> Only 1080p resolution and single plane is supported at this time.
>
> v2: moved extern to .h, removed license text
> use drm_dev_init, upclassed dev_private, removed HAVE_IRQ.(Sam)
>
> v3: Squashed all 59 commits to one
>
> v4: review changes from Sam Ravnborg
> renamed dev_p to kmb
> moved clocks under kmb_clock, consolidated clk initializations
> use drmm functions
> use DRM_GEM_CMA_DRIVER_OPS_VMAP
>
> v5: corrected spellings
> v6: corrected checkpatch warnings
> v7: review changes Sam Ravnborg and Thomas Zimmerman
> removed kmb_crtc.h kmb_crtc_cleanup (Thomas)
> renamed mode_set, kmb_load, inlined unload (Thomas)
> moved remaining logging to drm_*(Thomas)
> re-orged driver initialization (Thomas)
> moved plane_status to drm_private (Sam)
> removed unnecessary logs and defines and ifdef codes (Sam)
> call helper_check in plane_atomic_check (Sam)
> renamed set to get for bpp and format functions(Sam)
> use drm helper functions for reset, duplicate/destroy state instead
> of kmb functions (Sam)
> removed kmb_priv from kmb_plane and removed kmb_plane_state (Sam)
>
> Cc: Sam Ravnborg
> Signed-off-by: Anitha Chrisanthus
> Reviewed-by: Bob Paauwe
> ---
> drivers/gpu/drm/kmb/kmb_crtc.c | 224 +
> drivers/gpu/drm/kmb/kmb_drv.c | 676
>
> drivers/gpu/drm/kmb/kmb_drv.h | 170 ++
> drivers/gpu/drm/kmb/kmb_plane.c | 480
> drivers/gpu/drm/kmb/kmb_plane.h | 110 +++
> 5 files changed, 1660 insertions(+)
> create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.c
> create mode 100644 drivers/gpu/drm/kmb/kmb_drv.c
> create mode 100644 drivers/gpu/drm/kmb/kmb_drv.h
> create mode 100644 drivers/gpu/drm/kmb/kmb_plane.c
> create mode 100644 drivers/gpu/drm/kmb/kmb_plane.h
>
[...]
> +
> +static const struct of_device_id kmb_of_match[] = {
> + {.compatible = "intel,kmb_display"},
> + {},
> +};
As I already commented on v1, a proper YAML bindings files
is mandatory here, to check if the bindings are correct and if
the drivers uses them correctly (port/endpoints, etc..)
Neil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
On Mon, Aug 31, 2020 at 01:02:50PM -0700, Anitha Chrisanthus wrote:
> This is a basic KMS atomic modesetting display driver for KeemBay family of
> SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge
> driver at the connector level.
>
> Single CRTC with LCD controller->mipi DSI-> ADV bridge
>
> Only 1080p resolution and single plane is supported at this time.
>
> v2: moved extern to .h, removed license text
> use drm_dev_init, upclassed dev_private, removed HAVE_IRQ.(Sam)
>
> v3: Squashed all 59 commits to one
>
> v4: review changes from Sam Ravnborg
> renamed dev_p to kmb
> moved clocks under kmb_clock, consolidated clk initializations
> use drmm functions
> use DRM_GEM_CMA_DRIVER_OPS_VMAP
>
> v5: corrected spellings
> v6: corrected checkpatch warnings
> v7: review changes Sam Ravnborg and Thomas Zimmerman
> removed kmb_crtc.h kmb_crtc_cleanup (Thomas)
> renamed mode_set, kmb_load, inlined unload (Thomas)
> moved remaining logging to drm_*(Thomas)
> re-orged driver initialization (Thomas)
> moved plane_status to drm_private (Sam)
> removed unnecessary logs and defines and ifdef codes (Sam)
> call helper_check in plane_atomic_check (Sam)
> renamed set to get for bpp and format functions(Sam)
> use drm helper functions for reset, duplicate/destroy state instead
> of kmb functions (Sam)
> removed kmb_priv from kmb_plane and removed kmb_plane_state (Sam)
>
> Cc: Sam Ravnborg
> Signed-off-by: Anitha Chrisanthus
> Reviewed-by: Bob Paauwe
> ---
> drivers/gpu/drm/kmb/kmb_crtc.c | 224 +
> drivers/gpu/drm/kmb/kmb_drv.c | 676
>
> drivers/gpu/drm/kmb/kmb_drv.h | 170 ++
> drivers/gpu/drm/kmb/kmb_plane.c | 480
> drivers/gpu/drm/kmb/kmb_plane.h | 110 +++
> 5 files changed, 1660 insertions(+)
> create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.c
> create mode 100644 drivers/gpu/drm/kmb/kmb_drv.c
> create mode 100644 drivers/gpu/drm/kmb/kmb_drv.h
> create mode 100644 drivers/gpu/drm/kmb/kmb_plane.c
> create mode 100644 drivers/gpu/drm/kmb/kmb_plane.h
>
> diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c b/drivers/gpu/drm/kmb/kmb_crtc.c
> new file mode 100644
> index 000..a684331
> --- /dev/null
> +++ b/drivers/gpu/drm/kmb/kmb_crtc.c
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright © 2018-2020 Intel Corporation
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "kmb_drv.h"
> +#include "kmb_dsi.h"
> +#include "kmb_plane.h"
> +#include "kmb_regs.h"
> +
> +struct kmb_crtc_timing {
> + u32 vfront_porch;
> + u32 vback_porch;
> + u32 vsync_len;
> + u32 hfront_porch;
> + u32 hback_porch;
> + u32 hsync_len;
> +};
> +
> +static int kmb_crtc_enable_vblank(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct kmb_drm_private *kmb = to_kmb(dev);
> +
> + /* Clear interrupt */
> + kmb_write_lcd(kmb, LCD_INT_CLEAR, LCD_INT_VERT_COMP);
> + /* Set which interval to generate vertical interrupt */
> + kmb_write_lcd(kmb, LCD_VSTATUS_COMPARE,
> + LCD_VSTATUS_COMPARE_VSYNC);
> + /* Enable vertical interrupt */
> + kmb_set_bitmask_lcd(kmb, LCD_INT_ENABLE,
> + LCD_INT_VERT_COMP);
> + return 0;
> +}
> +
> +static void kmb_crtc_disable_vblank(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct kmb_drm_private *kmb = to_kmb(dev);
> +
> + /* Clear interrupt */
> + kmb_write_lcd(kmb, LCD_INT_CLEAR, LCD_INT_VERT_COMP);
> + /* Disable vertical interrupt */
> + kmb_clr_bitmask_lcd(kmb, LCD_INT_ENABLE,
> + LCD_INT_VERT_COMP);
> +}
> +
> +static const struct drm_crtc_funcs kmb_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,
> + .enable_vblank = kmb_crtc_enable_vblank,
> + .disable_vblank = kmb_crtc_disable_vblank,
> +};
> +
> +static void kmb_crtc_set_mode(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_display_mode *m = >state->adjusted_mode;
> + struct kmb_crtc_timing vm;
> + int vsync_start_offset;
> + int vsync_end_offset;
> + struct kmb_drm_private *kmb = to_kmb(dev);
> + unsigned int val = 0;
> +
> + /* Initialize mipi */
> + kmb_dsi_hw_init(dev, m);
> + drm_info(dev,
> + "vfp= %d vbp= %d vsyc_len=%d
Re: [PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
On Mon, Aug 31, 2020 at 01:02:50PM -0700, Anitha Chrisanthus wrote:
> This is a basic KMS atomic modesetting display driver for KeemBay family of
> SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge
> driver at the connector level.
>
> Single CRTC with LCD controller->mipi DSI-> ADV bridge
>
> Only 1080p resolution and single plane is supported at this time.
>
> v2: moved extern to .h, removed license text
> use drm_dev_init, upclassed dev_private, removed HAVE_IRQ.(Sam)
>
> v3: Squashed all 59 commits to one
>
> v4: review changes from Sam Ravnborg
> renamed dev_p to kmb
> moved clocks under kmb_clock, consolidated clk initializations
> use drmm functions
> use DRM_GEM_CMA_DRIVER_OPS_VMAP
>
> v5: corrected spellings
> v6: corrected checkpatch warnings
> v7: review changes Sam Ravnborg and Thomas Zimmerman
> removed kmb_crtc.h kmb_crtc_cleanup (Thomas)
> renamed mode_set, kmb_load, inlined unload (Thomas)
> moved remaining logging to drm_*(Thomas)
> re-orged driver initialization (Thomas)
> moved plane_status to drm_private (Sam)
> removed unnecessary logs and defines and ifdef codes (Sam)
> call helper_check in plane_atomic_check (Sam)
> renamed set to get for bpp and format functions(Sam)
> use drm helper functions for reset, duplicate/destroy state instead
> of kmb functions (Sam)
> removed kmb_priv from kmb_plane and removed kmb_plane_state (Sam)
>
> Cc: Sam Ravnborg
> Signed-off-by: Anitha Chrisanthus
> Reviewed-by: Bob Paauwe
Sam asked me to take a look at this too, looks reasonable overall. I've
spotted a few small things plus a potential functional issue around
vblank/event handling. I strongly recommend running the igt test suite
over the driver to see whether it all works reasonably ok. See below for
details.
> ---
> drivers/gpu/drm/kmb/kmb_crtc.c | 224 +
> drivers/gpu/drm/kmb/kmb_drv.c | 676
>
> drivers/gpu/drm/kmb/kmb_drv.h | 170 ++
> drivers/gpu/drm/kmb/kmb_plane.c | 480
> drivers/gpu/drm/kmb/kmb_plane.h | 110 +++
> 5 files changed, 1660 insertions(+)
> create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.c
> create mode 100644 drivers/gpu/drm/kmb/kmb_drv.c
> create mode 100644 drivers/gpu/drm/kmb/kmb_drv.h
> create mode 100644 drivers/gpu/drm/kmb/kmb_plane.c
> create mode 100644 drivers/gpu/drm/kmb/kmb_plane.h
>
> diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c b/drivers/gpu/drm/kmb/kmb_crtc.c
> new file mode 100644
> index 000..a684331
> --- /dev/null
> +++ b/drivers/gpu/drm/kmb/kmb_crtc.c
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright © 2018-2020 Intel Corporation
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "kmb_drv.h"
> +#include "kmb_dsi.h"
> +#include "kmb_plane.h"
> +#include "kmb_regs.h"
> +
> +struct kmb_crtc_timing {
> + u32 vfront_porch;
> + u32 vback_porch;
> + u32 vsync_len;
> + u32 hfront_porch;
> + u32 hback_porch;
> + u32 hsync_len;
> +};
> +
> +static int kmb_crtc_enable_vblank(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct kmb_drm_private *kmb = to_kmb(dev);
> +
> + /* Clear interrupt */
> + kmb_write_lcd(kmb, LCD_INT_CLEAR, LCD_INT_VERT_COMP);
> + /* Set which interval to generate vertical interrupt */
> + kmb_write_lcd(kmb, LCD_VSTATUS_COMPARE,
> + LCD_VSTATUS_COMPARE_VSYNC);
> + /* Enable vertical interrupt */
> + kmb_set_bitmask_lcd(kmb, LCD_INT_ENABLE,
> + LCD_INT_VERT_COMP);
> + return 0;
> +}
> +
> +static void kmb_crtc_disable_vblank(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct kmb_drm_private *kmb = to_kmb(dev);
> +
> + /* Clear interrupt */
> + kmb_write_lcd(kmb, LCD_INT_CLEAR, LCD_INT_VERT_COMP);
> + /* Disable vertical interrupt */
> + kmb_clr_bitmask_lcd(kmb, LCD_INT_ENABLE,
> + LCD_INT_VERT_COMP);
> +}
> +
> +static const struct drm_crtc_funcs kmb_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,
> + .enable_vblank = kmb_crtc_enable_vblank,
> + .disable_vblank = kmb_crtc_disable_vblank,
> +};
> +
> +static void kmb_crtc_set_mode(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_display_mode *m = >state->adjusted_mode;
> + struct
[PATCH v7 2/4] drm/kmb: Add support for KeemBay Display
This is a basic KMS atomic modesetting display driver for KeemBay family of
SOCs. Driver has no 2D or 3D graphics.It calls into the ADV bridge
driver at the connector level.
Single CRTC with LCD controller->mipi DSI-> ADV bridge
Only 1080p resolution and single plane is supported at this time.
v2: moved extern to .h, removed license text
use drm_dev_init, upclassed dev_private, removed HAVE_IRQ.(Sam)
v3: Squashed all 59 commits to one
v4: review changes from Sam Ravnborg
renamed dev_p to kmb
moved clocks under kmb_clock, consolidated clk initializations
use drmm functions
use DRM_GEM_CMA_DRIVER_OPS_VMAP
v5: corrected spellings
v6: corrected checkpatch warnings
v7: review changes Sam Ravnborg and Thomas Zimmerman
removed kmb_crtc.h kmb_crtc_cleanup (Thomas)
renamed mode_set, kmb_load, inlined unload (Thomas)
moved remaining logging to drm_*(Thomas)
re-orged driver initialization (Thomas)
moved plane_status to drm_private (Sam)
removed unnecessary logs and defines and ifdef codes (Sam)
call helper_check in plane_atomic_check (Sam)
renamed set to get for bpp and format functions(Sam)
use drm helper functions for reset, duplicate/destroy state instead
of kmb functions (Sam)
removed kmb_priv from kmb_plane and removed kmb_plane_state (Sam)
Cc: Sam Ravnborg
Signed-off-by: Anitha Chrisanthus
Reviewed-by: Bob Paauwe
---
drivers/gpu/drm/kmb/kmb_crtc.c | 224 +
drivers/gpu/drm/kmb/kmb_drv.c | 676
drivers/gpu/drm/kmb/kmb_drv.h | 170 ++
drivers/gpu/drm/kmb/kmb_plane.c | 480
drivers/gpu/drm/kmb/kmb_plane.h | 110 +++
5 files changed, 1660 insertions(+)
create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.c
create mode 100644 drivers/gpu/drm/kmb/kmb_drv.c
create mode 100644 drivers/gpu/drm/kmb/kmb_drv.h
create mode 100644 drivers/gpu/drm/kmb/kmb_plane.c
create mode 100644 drivers/gpu/drm/kmb/kmb_plane.h
diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c b/drivers/gpu/drm/kmb/kmb_crtc.c
new file mode 100644
index 000..a684331
--- /dev/null
+++ b/drivers/gpu/drm/kmb/kmb_crtc.c
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright © 2018-2020 Intel Corporation
+ */
+
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "kmb_drv.h"
+#include "kmb_dsi.h"
+#include "kmb_plane.h"
+#include "kmb_regs.h"
+
+struct kmb_crtc_timing {
+ u32 vfront_porch;
+ u32 vback_porch;
+ u32 vsync_len;
+ u32 hfront_porch;
+ u32 hback_porch;
+ u32 hsync_len;
+};
+
+static int kmb_crtc_enable_vblank(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct kmb_drm_private *kmb = to_kmb(dev);
+
+ /* Clear interrupt */
+ kmb_write_lcd(kmb, LCD_INT_CLEAR, LCD_INT_VERT_COMP);
+ /* Set which interval to generate vertical interrupt */
+ kmb_write_lcd(kmb, LCD_VSTATUS_COMPARE,
+ LCD_VSTATUS_COMPARE_VSYNC);
+ /* Enable vertical interrupt */
+ kmb_set_bitmask_lcd(kmb, LCD_INT_ENABLE,
+ LCD_INT_VERT_COMP);
+ return 0;
+}
+
+static void kmb_crtc_disable_vblank(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct kmb_drm_private *kmb = to_kmb(dev);
+
+ /* Clear interrupt */
+ kmb_write_lcd(kmb, LCD_INT_CLEAR, LCD_INT_VERT_COMP);
+ /* Disable vertical interrupt */
+ kmb_clr_bitmask_lcd(kmb, LCD_INT_ENABLE,
+ LCD_INT_VERT_COMP);
+}
+
+static const struct drm_crtc_funcs kmb_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,
+ .enable_vblank = kmb_crtc_enable_vblank,
+ .disable_vblank = kmb_crtc_disable_vblank,
+};
+
+static void kmb_crtc_set_mode(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_display_mode *m = >state->adjusted_mode;
+ struct kmb_crtc_timing vm;
+ int vsync_start_offset;
+ int vsync_end_offset;
+ struct kmb_drm_private *kmb = to_kmb(dev);
+ unsigned int val = 0;
+
+ /* Initialize mipi */
+ kmb_dsi_hw_init(dev, m);
+ drm_info(dev,
+"vfp= %d vbp= %d vsyc_len=%d hfp=%d hbp=%d hsync_len=%d\n",
+m->crtc_vsync_start - m->crtc_vdisplay,
+m->crtc_vtotal - m->crtc_vsync_end,
+m->crtc_vsync_end - m->crtc_vsync_start,
+m->crtc_hsync_start - m->crtc_hdisplay,
