Re: [RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
On Mon, Oct 19, 2015 at 02:26:38PM +0100, Russell King - ARM Linux wrote: > On Mon, Oct 19, 2015 at 02:02:58PM +0100, Liviu Dudau wrote: > > On Mon, Oct 19, 2015 at 01:25:37PM +0100, Russell King - ARM Linux wrote: > > > Please don't move this into here, it's completely inappropriate. Just > > > because something makes use of this does not mean they only support > > > 32-bit DMA. Besides, this has nothing to do with whether or not it's > > > OF-based or not. > > > > Understood. My thinking process was that component-based drivers are all > > OF-enabled (how else do you make use of the framework?) and 32-bit DMA is > > present in 2 out of 3 drivers that are converted, so it looks to be common > > enough that adding it to armada would not hurt. It was all done in the name > > of > > collecting common code in a single function. > > Which is an utterly crap reason. > > It's also not appropriate. I'm really not sure why you think that moving > this here would in any way be appropriate - from my point of view, the > mere proposal is utterly insane. > > The "container" device does not do any DMA, so it's inappropriate for > it to have DMA masks set or negotiated on it. So, actually, no one > should be setting the DMA mask for their container device. It's wrong. I think (and my opinion doesn't carry as much wheight here on dri-devel than intel-gfx) the above is over the top bashing of a new contributor to drm who really seems trying to do right. I think that's unecessary, especially since you follow up with the reasonable reply below. Thanks, Daniel > What if we have a 64-bit OF based platform wanting to use the component > helper, and they want to call this function? You prevent them doing so > by moving this into here, because they're then forced down to 32-bit DMA. > Please, get rid of it, and leave this crappiness in the respective > drivers. > > -- > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 v4 08/79] r128_drm.h: include drm/drm.h
On Thu, Oct 15, 2015 at 07:55:46AM +0200, Mikko Rapeli wrote: > Fixes compile error: > > drm/r128_drm.h:156:23: error: array type has incomplete element type > struct drm_clip_rect boxes[R128_NR_SAREA_CLIPRECTS]; > > Signed-off-by: Mikko Rapeli <mikko.rap...@iki.fi> Applied to drm-misc, thanks. -Daniel > --- > include/uapi/drm/r128_drm.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/uapi/drm/r128_drm.h b/include/uapi/drm/r128_drm.h > index 8d8878b..76b0aa3 100644 > --- a/include/uapi/drm/r128_drm.h > +++ b/include/uapi/drm/r128_drm.h > @@ -33,6 +33,8 @@ > #ifndef __R128_DRM_H__ > #define __R128_DRM_H__ > > +#include > + > /* WARNING: If you change any of these defines, make sure to change the > * defines in the X server file (r128_sarea.h) > */ > -- > 2.5.0 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 v4 13/79] drm/i810_drm.h: include drm/drm.h
On Thu, Oct 15, 2015 at 07:55:51AM +0200, Mikko Rapeli wrote: > Fixes userspace compilation error: > > error: array type has incomplete element type > struct drm_clip_rect boxes[I810_NR_SAREA_CLIPRECTS]; > > Signed-off-by: Mikko Rapeli <mikko.rap...@iki.fi> Applied to drm-misc, thanks. -Daniel > --- > include/uapi/drm/i810_drm.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/uapi/drm/i810_drm.h b/include/uapi/drm/i810_drm.h > index 7a10bb6..34736ef 100644 > --- a/include/uapi/drm/i810_drm.h > +++ b/include/uapi/drm/i810_drm.h > @@ -1,6 +1,8 @@ > #ifndef _I810_DRM_H_ > #define _I810_DRM_H_ > > +#include > + > /* WARNING: These defines must be the same as what the Xserver uses. > * if you change them, you must change the defines in the Xserver. > */ > -- > 2.5.0 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 03/98] drm.h: use __kernel_size_t instead of size_t
On Wed, Jun 03, 2015 at 06:19:48PM +0100, Emil Velikov wrote:
> On 30 May 2015 at 16:37, Mikko Rapeli <mikko.rap...@iki.fi> wrote:
> > Fixes userspace compilation error:
> >
> > drm/drm.h:132:2: error: unknown type name ‘size_t’
> >
> > Signed-off-by: Mikko Rapeli <mikko.rap...@iki.fi>
> > ---
> > include/uapi/drm/drm.h | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > index ff6ef62..b197650 100644
> > --- a/include/uapi/drm/drm.h
> > +++ b/include/uapi/drm/drm.h
> > @@ -129,11 +129,11 @@ struct drm_version {
> > int version_major;/**< Major version */
> > int version_minor;/**< Minor version */
> > int version_patchlevel; /**< Patch level */
> > - size_t name_len; /**< Length of name buffer */
> > + __kernel_size_t name_len; /**< Length of name buffer */
> > char __user *name;/**< Name of driver */
> > - size_t date_len; /**< Length of date buffer */
> > + __kernel_size_t date_len; /**< Length of date buffer */
> > char __user *date;/**< User-space buffer to hold date */
> > - size_t desc_len; /**< Length of desc buffer */
> > + __kernel_size_t desc_len; /**< Length of desc buffer */
> > char __user *desc;/**< User-space buffer to hold desc */
> > };
> >
> > @@ -143,7 +143,7 @@ struct drm_version {
> > * \sa drmGetBusid() and drmSetBusId().
> > */
> > struct drm_unique {
> > - size_t unique_len;/**< Length of unique */
> > + __kernel_size_t unique_len; /**< Length of unique */
> As the file is used by other platforms than Linux this change will
> break them. Can you add a typedef similar to how __u8 and friends are
> handled at the top of the file.
This file isn't used by anyone else than Linus since years. All of the
typedefs can be nuked imo.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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 v4 11/79] savage_drm.h: include
On Thu, Oct 15, 2015 at 07:55:49AM +0200, Mikko Rapeli wrote: > Fixes compiler error: > > drm/savage_drm.h:50:24: error: array type has incomplete element type > struct drm_tex_region texList[SAVAGE_NR_TEX_HEAPS][SAVAGE_NR_TEX_REGIONS + > > Signed-off-by: Mikko Rapeli <mikko.rap...@iki.fi> Queued for -next, thanks for the patch. -Daniel > --- > include/uapi/drm/savage_drm.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/uapi/drm/savage_drm.h b/include/uapi/drm/savage_drm.h > index 818d49b..9dc9dc1 100644 > --- a/include/uapi/drm/savage_drm.h > +++ b/include/uapi/drm/savage_drm.h > @@ -26,6 +26,8 @@ > #ifndef __SAVAGE_DRM_H__ > #define __SAVAGE_DRM_H__ > > +#include > + > #ifndef __SAVAGE_SAREA_DEFINES__ > #define __SAVAGE_SAREA_DEFINES__ > > -- > 2.5.0 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: [Intel-gfx] [PATCH 09/20] i915: switch from acpi_os_ioremap to memremap
On Tue, Oct 13, 2015 at 09:24:26AM -0700, Dan Williams wrote: > On Tue, Oct 13, 2015 at 1:24 AM, Daniel Vetter <dan...@ffwll.ch> wrote: > > On Mon, Oct 12, 2015 at 09:12:57PM +, Williams, Dan J wrote: > >> On Mon, 2015-10-12 at 09:01 +0200, Daniel Vetter wrote: > >> > On Fri, Oct 09, 2015 at 06:16:25PM -0400, Dan Williams wrote: > >> > > i915 expects the OpRegion to be cached (i.e. not __iomem), so > >> > > explicitly > >> > > map it with memremap rather than the implied cache setting of > >> > > acpi_os_ioremap(). > >> > > > >> > > Cc: Daniel Vetter <daniel.vet...@intel.com> > >> > > Cc: Jani Nikula <jani.nik...@linux.intel.com> > >> > > Cc: intel-...@lists.freedesktop.org > >> > > Cc: David Airlie <airl...@linux.ie> > >> > > Cc: dri-de...@lists.freedesktop.org > >> > > Signed-off-by: Dan Williams <dan.j.willi...@intel.com> > >> > > >> > Assuming you've run sparse over this to make sure you've caught them all, > >> > and with the nit below addressed this is > >> > > >> > Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch> > >> > >> Indeed, re-running sparse again found a few conversions of ioread* I > >> missed as well as moving the force casting out of validate_vbt() to > >> find_vbt(). > >> > >> > Feel free to pull v2 into whatever tree you think it's suitable for (but > >> > you can also resend and I'll pick it up). > >> > >> Please pick up v2 below. > > > > Queued for -next, thanks for the patch. Aside: Attached or separate mail > > seems easier, somehow git apply-mbox can't auto-eat this for of patch. > > -Daniel > > > > "git am --scissors" should detect the "8<---" cut line. TIL, works nice indeed. Thanks for the hint. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: [Intel-gfx] [PATCH] drm/i915/irq: Fix misspelled word register in kernel-doc
On Thu, Oct 08, 2015 at 09:57:49AM +0200, Javier Martinez Canillas wrote: > There is a typo in the function i915_handle_error() > kernel-doc and the word register is spelled wrongly. > > Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com> Both kerneldoc patches applied, thanks. -Daniel > --- > > drivers/gpu/drm/i915/i915_irq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 0344a9181dac..38dadd23684d 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2571,7 +2571,7 @@ static void i915_report_and_clear_eir(struct drm_device > *dev) > * i915_handle_error - handle a gpu error > * @dev: drm device > * > - * Do some basic checking of regsiter state at error time and > + * Do some basic checking of register state at error time and > * dump it to the syslog. Also call i915_capture_error_state() to make > * sure we get a record and make it available in debugfs. Fire a uevent > * so userspace knows something bad happened (should trigger collection > -- > 2.4.3 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 RESEND 1/3] drm/i915: use error path
On Thu, Oct 08, 2015 at 07:27:59PM +0530, Sudip Mukherjee wrote:
> Use goto to handle the error path to avoid duplicating the same code. In
> the error path intel_dig_port is the last one to be released as it was
> the first one to be allocated and ideally the error path should be the
> reverse of the execution path.
>
> Cc: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Jani Nikula <jani.nik...@linux.intel.com>
> Signed-off-by: Sudip Mukherjee <su...@vectorindia.org>
Queued for -next, thanks for the patch.
-Daniel
> ---
>
> Sent on 27/07/2015
>
> drivers/gpu/drm/i915/intel_dp.c | 23 ++-
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8d34ca7..18bcfbe 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6168,10 +6168,8 @@ intel_dp_init(struct drm_device *dev, int output_reg,
> enum port port)
> return;
>
> intel_connector = intel_connector_alloc();
> - if (!intel_connector) {
> - kfree(intel_dig_port);
> - return;
> - }
> + if (!intel_connector)
> + goto err_connector_alloc;
>
> intel_encoder = _dig_port->base;
> encoder = _encoder->base;
> @@ -6219,11 +6217,18 @@ intel_dp_init(struct drm_device *dev, int output_reg,
> enum port port)
> intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> dev_priv->hotplug.irq_port[port] = intel_dig_port;
>
> - if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
> - drm_encoder_cleanup(encoder);
> - kfree(intel_dig_port);
> - kfree(intel_connector);
> - }
> + if (!intel_dp_init_connector(intel_dig_port, intel_connector))
> + goto err_init_connector;
> +
> + return;
> +
> +err_init_connector:
> + drm_encoder_cleanup(encoder);
> + kfree(intel_connector);
> +err_connector_alloc:
> + kfree(intel_dig_port);
> +
> + return;
> }
>
> void intel_dp_mst_suspend(struct drm_device *dev)
> --
> 1.9.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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 RESEND 2/3] drm/i915: check for return value
On Thu, Oct 08, 2015 at 07:28:00PM +0530, Sudip Mukherjee wrote: > We were not checking the return value of drm_encoder_init() which can > fail. And if it fails then we will be working with an uninitialized > encoder. > > Cc: Daniel Vetter <daniel.vet...@intel.com> > Cc: Jani Nikula <jani.nik...@linux.intel.com> > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org> Queued for -next, thanks for the patch. -Daniel > --- > > Sent on 27/07/2015 > > drivers/gpu/drm/i915/intel_dp.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 18bcfbe..2a8b47e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -6174,8 +6174,9 @@ intel_dp_init(struct drm_device *dev, int output_reg, > enum port port) > intel_encoder = _dig_port->base; > encoder = _encoder->base; > > - drm_encoder_init(dev, _encoder->base, _dp_enc_funcs, > - DRM_MODE_ENCODER_TMDS); > + if (drm_encoder_init(dev, _encoder->base, _dp_enc_funcs, > + DRM_MODE_ENCODER_TMDS)) > + goto err_encoder_init; > > intel_encoder->compute_config = intel_dp_compute_config; > intel_encoder->disable = intel_disable_dp; > @@ -6224,6 +6225,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, > enum port port) > > err_init_connector: > drm_encoder_cleanup(encoder); > +err_encoder_init: > kfree(intel_connector); > err_connector_alloc: > kfree(intel_dig_port); > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: [Intel-gfx] [regression] [git pull] drm for 4.3
On Mon, Oct 19, 2015 at 04:19:08PM -0400, da...@codemonkey.org.uk wrote:
> On Wed, Sep 30, 2015 at 08:56:26AM +0200, Daniel Vetter wrote:
>
> > > The warning on boot seems to be gone as of rc3, but I can now trigger
> this pretty easily..
> >
> > http://patchwork.freedesktop.org/patch/60618/
>
> Back from several weeks of travel.. I tried again with rc6, and
> I'm still seeing the same traces.
Oh crap, applied patch to wrong tree. We need to cherry-pick
commit 621bd0f6982badd6483acb191eb7b6226a578328
Author: Daniel Vetter <daniel.vet...@ffwll.ch>
Date: Tue Sep 29 09:56:53 2015 +0200
drm: Fix locking for sysfs dpms file
to drm-fixes. Sorry about that screw-up. Dave, can you pls do that one? It
even comes with the needed cc: stable included (since the locking breakage
was done in 4.0, it only surface due to a new warning in 4.3).
-Daniel
>
> Dave
>
>
> > > WARNING: CPU: 2 PID: 28911 at drivers/gpu/drm/drm_atomic.c:889
> drm_atomic_get_property+0x244/0x2d0()
> > > CPU: 2 PID: 28911 Comm: trinity-c313 Not tainted 4.3.0-rc3-think+ #14
> > > 0379 8801a1377c88 8e35d5ec
> > > 8801a1377cc0 8e07a862 880500b392b8 880500a13008
> > > 880500b39290 8804fe3806d8 88003fa45668 8801a1377cd0
> > > Call Trace:
> > > [] dump_stack+0x4e/0x82
> > > [] warn_slowpath_common+0x82/0xc0
> > > [] warn_slowpath_null+0x1a/0x20
> > > [] drm_atomic_get_property+0x244/0x2d0
> > > [] drm_object_property_get_value+0x6c/0x70
> > > [] dpms_show+0x2f/0x70
> > > [] dev_attr_show+0x20/0x50
> > > [] ? sysfs_file_ops+0x41/0x60
> > > [] sysfs_kf_seq_show+0xb7/0x110
> > > [] kernfs_seq_show+0x26/0x30
> > > [] seq_read+0xe6/0x430
> > > [] kernfs_fop_read+0x127/0x170
> > > [] ? mutex_lock_nested+0x26b/0x3f0
> > > [] __vfs_read+0x28/0xe0
> > > [] ? mutex_lock_nested+0x287/0x3f0
> > > [] ? __fdget_pos+0x49/0x50
> > > [] ? __fdget_pos+0x49/0x50
> > > [] vfs_read+0x86/0x130
> > > [] SyS_read+0x49/0xb0
> > > [] entry_SYSCALL_64_fastpath+0x12/0x6f
> > > ---[ end trace e053063c697a1355 ]---
> > >
> > > 887 case DRM_MODE_OBJECT_CONNECTOR: {
> > > 888 struct drm_connector *connector =
> obj_to_connector(obj);
> > > 889
> WARN_ON(!drm_modeset_is_locked(>mode_config.connection_mutex));
> > > 890 ret = drm_atomic_connector_get_property(connector,
> > > 891 connector->state, property, val);
> > > 892 break;
> > > 893 }
> > >
> > > ___
> > > dri-devel mailing list
> > > dri-de...@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> ---end quoted text---
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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 v4 0/4] drm: Cleanup probe function for component based masters.
On Tue, Oct 20, 2015 at 10:23:11AM +0100, Liviu Dudau wrote: > Changelog: > v4: Fixed a bug where the wrong pointer was sent to component_match_add() and > component_master_add_with_match() in the armada_drv.c file that was > flagged > by kbuild test robot. Dropped the RFC tag and added Acked-bys received > from > Russell King. > v3: Removed the call to dma_set_coherent_mask() from the generic > drm_of_component_probe(). Also changes to shorten lines over 80 chars > long. > v2: Rebased the patchset on top of drm-next rather than Linus' latest -rc > > A few drivers in drivers/gpu/drm are component-enabled and use quite similar > code sequences to probe for their encoder slaves at the remote end of the > ports. > Move the code into a "generic" function and remove it from the drivers. > > The end results is that drivers get a reference count fix (imx), more thorough > error checking (imx again) plus a decrease in the overall count of LoC. > > I'm looking for comments and testing of the patchset (only compile tested > from my > end as I don't have access to all the devices touched by the changes). My main > interest is in finding out if -EINVAL is the correct code to return if > dev->of_node == NULL (handy now, as it is different from the other possible > error > codes and used in armada to trigger old platform_data support. Also looking > for > thoughts on the correctness of the patch and if it possible to co-opt more > drivers > into using the function. Merged all four to drm-misc, thanks. -Daniel > > Best regards, > Liviu > > Liviu Dudau (4): > drm: Introduce generic probe function for component based masters. > drm/imx: Convert the probe function to the generic drm_of_component_probe() > drm/rockchip: Convert the probe function to the generic > drm_of_component_probe() > drm/armada: Convert the probe function to the generic > drm_of_component_probe() > > drivers/gpu/drm/armada/armada_drv.c | 68 +++--- > drivers/gpu/drm/drm_of.c| 88 > + > drivers/gpu/drm/imx/imx-drm-core.c | 55 ++ > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 81 ++-------- > include/drm/drm_of.h| 13 + > 5 files changed, 130 insertions(+), 175 deletions(-) > > -- > 2.6.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: [GIT PULL] Raspberry Pi KMS driver
On Wed, Oct 21, 2015 at 10:53:31AM +0100, Eric Anholt wrote: > Dave suggested it was time to just send a pull request on the driver, so > here goes: Given I suggested the same: Acked-by: Daniel Vetter <daniel.vet...@ffwll.ch> > > The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f: > > Linux 4.3-rc1 (2015-09-12 16:35:56 -0700) > > are available in the git repository at: > > http://github.com/anholt/linux drm-vc4-next-2015-10-21 > > for you to fetch changes up to 98a44504541c6befb28366eb9ec432ba44070dd9: > > drm/vc4: Allow vblank to be disabled (2015-10-21 10:33:13 +0100) > > > This pull request introduces the vc4 driver, for kernel modesetting on > the Raspberry Pi (bcm2835/bcm2836 architectures). It currently > supports a display plane and cursor on the HDMI output. The driver > doesn't do 3D, power management, or overlay planes yet. > > > Derek Foreman (2): > drm/vc4: Use the fbdev_cma helpers > drm/vc4: Allow vblank to be disabled > > Eric Anholt (2): > drm/vc4: Add devicetree bindings for VC4. > drm/vc4: Add KMS support for Raspberry Pi. > > .../devicetree/bindings/display/brcm,bcm-vc4.txt | 65 ++ > drivers/gpu/drm/Kconfig| 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/vc4/Kconfig| 13 + > drivers/gpu/drm/vc4/Makefile | 17 + > drivers/gpu/drm/vc4/vc4_bo.c | 52 ++ > drivers/gpu/drm/vc4/vc4_crtc.c | 672 > + > drivers/gpu/drm/vc4/vc4_debugfs.c | 39 ++ > drivers/gpu/drm/vc4/vc4_drv.c | 298 + > drivers/gpu/drm/vc4/vc4_drv.h | 145 + > drivers/gpu/drm/vc4/vc4_hdmi.c | 590 ++ > drivers/gpu/drm/vc4/vc4_hvs.c | 163 + > drivers/gpu/drm/vc4/vc4_kms.c | 67 ++ > drivers/gpu/drm/vc4/vc4_plane.c| 320 ++ > drivers/gpu/drm/vc4/vc4_regs.h | 570 + > 15 files changed, 3014 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt > create mode 100644 drivers/gpu/drm/vc4/Kconfig > create mode 100644 drivers/gpu/drm/vc4/Makefile > create mode 100644 drivers/gpu/drm/vc4/vc4_bo.c > create mode 100644 drivers/gpu/drm/vc4/vc4_crtc.c > create mode 100644 drivers/gpu/drm/vc4/vc4_debugfs.c > create mode 100644 drivers/gpu/drm/vc4/vc4_drv.c > create mode 100644 drivers/gpu/drm/vc4/vc4_drv.h > create mode 100644 drivers/gpu/drm/vc4/vc4_hdmi.c > create mode 100644 drivers/gpu/drm/vc4/vc4_hvs.c > create mode 100644 drivers/gpu/drm/vc4/vc4_kms.c > create mode 100644 drivers/gpu/drm/vc4/vc4_plane.c > create mode 100644 drivers/gpu/drm/vc4/vc4_regs.h > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] drm: correctly check failed allocation
On Mon, Oct 19, 2015 at 04:33:30PM +, Insu Yun wrote: > drm_property_create_range can be failed in memory pressure > Therefore, check return value and handle an error > > Signed-off-by: Insu Yun <wuni...@gmail.com> There's also other drm_property_create_* functions, and we miss error handling for a bunch more of these places. Are you working on more patches? Merged this one to drm-misc meanwhile, thanks. -Daniel > --- > drivers/gpu/drm/drm_crtc.c | 30 ++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 33d877c..5fb6583 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1525,6 +1525,9 @@ int drm_mode_create_tv_properties(struct drm_device > *dev, > "select subconnector", > drm_tv_select_enum_list, > ARRAY_SIZE(drm_tv_select_enum_list)); > + if (!tv_selector) > + goto nomem; > + > dev->mode_config.tv_select_subconnector_property = tv_selector; > > tv_subconnector = > @@ -1532,6 +1535,8 @@ int drm_mode_create_tv_properties(struct drm_device > *dev, > "subconnector", > drm_tv_subconnector_enum_list, > ARRAY_SIZE(drm_tv_subconnector_enum_list)); > + if (!tv_subconnector) > + goto nomem; > dev->mode_config.tv_subconnector_property = tv_subconnector; > > /* > @@ -1539,42 +1544,67 @@ int drm_mode_create_tv_properties(struct drm_device > *dev, >*/ > dev->mode_config.tv_left_margin_property = > drm_property_create_range(dev, 0, "left margin", 0, 100); > + if (!dev->mode_config.tv_left_margin_property) > + goto nomem; > > dev->mode_config.tv_right_margin_property = > drm_property_create_range(dev, 0, "right margin", 0, 100); > + if (!dev->mode_config.tv_right_margin_property) > + goto nomem; > > dev->mode_config.tv_top_margin_property = > drm_property_create_range(dev, 0, "top margin", 0, 100); > + if (!dev->mode_config.tv_top_margin_property) > + goto nomem; > > dev->mode_config.tv_bottom_margin_property = > drm_property_create_range(dev, 0, "bottom margin", 0, 100); > + if (!dev->mode_config.tv_bottom_margin_property) > + goto nomem; > > dev->mode_config.tv_mode_property = > drm_property_create(dev, DRM_MODE_PROP_ENUM, > "mode", num_modes); > + if (!dev->mode_config.tv_mode_property) > + goto nomem; > + > for (i = 0; i < num_modes; i++) > drm_property_add_enum(dev->mode_config.tv_mode_property, i, > i, modes[i]); > > dev->mode_config.tv_brightness_property = > drm_property_create_range(dev, 0, "brightness", 0, 100); > + if (!dev->mode_config.tv_brightness_property) > + goto nomem; > > dev->mode_config.tv_contrast_property = > drm_property_create_range(dev, 0, "contrast", 0, 100); > + if (!dev->mode_config.tv_contrast_property) > + goto nomem; > > dev->mode_config.tv_flicker_reduction_property = > drm_property_create_range(dev, 0, "flicker reduction", 0, 100); > + if (!dev->mode_config.tv_flicker_reduction_property) > + goto nomem; > > dev->mode_config.tv_overscan_property = > drm_property_create_range(dev, 0, "overscan", 0, 100); > + if (!dev->mode_config.tv_overscan_property) > + goto nomem; > > dev->mode_config.tv_saturation_property = > drm_property_create_range(dev, 0, "saturation", 0, 100); > + if (!dev->mode_config.tv_saturation_property) > + goto nomem; > > dev->mode_config.tv_hue_property = > drm_property_create_range(dev, 0, "hue", 0, 100); > + if (!dev->mode_config.tv_hue_property) > + goto nomem; > > return 0; > +nomem: > + return -ENOMEM; > } > EXPORT_SYMBOL(drm_mode_create_tv_properties); > > -- > 1.9.1 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: v4.3-rc4: i915: ThinkPad Yoga 12: *ERROR* The master control interrupt lied (SDE)! [regression]
Another regression for Jairo to track. -Daniel On Sat, Oct 10, 2015 at 12:08:43PM -0700, Darren Hart wrote: > The Debian 3.16.0 kernel does not emit the error, but I have not attempted a > bisection. > > The warning was added by: > 38cc46d drm/i915/bdw: Ack interrupts before handling them (GEN8) > 2014-06-18 (1 year, 4 months ago), Oscar Mateo <oscar.ma...@intel.com> > > Follows: v3.15-rc8 > Preceedes: 3.17-rc1 > > This is not present in v3.16, so perhaps not present in the Debian kernel and > this is not a regression, but just reporting the condition as intended. > > Linux Version: v4.3-rc4 > Platform: Lenovo ThinkPad Yoga 12 > OS: Debian 8.2 > > $ dmesg | grep -i drm > [ 10.918367] [drm] Initialized drm 1.1.0 20060810 > [ 11.235005] [drm] Memory usable by graphics device = 4096M > [ 11.235009] fb: switching to inteldrmfb from simple > [ 11.235093] [drm] Replacing VGA console driver > [ 11.241160] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > [ 11.241162] [drm] Driver supports precise vblank timestamp query. > [ 11.256249] [drm:drm_calc_timestamping_constants [drm]] *ERROR* crtc 21: > Can't calculate constants, dotclock = 0! > [ 11.326946] [drm] GMBUS [i915 gmbus dpb] timed out, falling back to bit > banging on pin 5 > [ 11.344097] [drm] Initialized i915 1.6.0 20150731 for :00:02.0 on > minor 0 > [ 11.344913] fbcon: inteldrmfb (fb0) is primary device > [ 12.462509] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 12.466498] i915 :00:02.0: fb0: inteldrmfb frame buffer device > [ 12.794359] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 13.869733] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 13.869894] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 13.870058] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 22.656986] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 257.506246] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 257.506415] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 257.506584] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 257.506746] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 278.722570] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 278.722744] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 278.722908] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 1857.776390] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 1857.776549] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 1857.776710] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > [ 4057.592685] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (SDE)! > > -- > Darren Hart > Intel Open Source Technology Center -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: [Intel-gfx] [PATCH 09/20] i915: switch from acpi_os_ioremap to memremap
On Fri, Oct 09, 2015 at 06:16:25PM -0400, Dan Williams wrote:
> i915 expects the OpRegion to be cached (i.e. not __iomem), so explicitly
> map it with memremap rather than the implied cache setting of
> acpi_os_ioremap().
>
> Cc: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Jani Nikula <jani.nik...@linux.intel.com>
> Cc: intel-...@lists.freedesktop.org
> Cc: David Airlie <airl...@linux.ie>
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
Assuming you've run sparse over this to make sure you've caught them all,
and with the nit below addressed this is
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Feel free to pull v2 into whatever tree you think it's suitable for (but
you can also resend and I'll pick it up).
> diff --git a/drivers/gpu/drm/i915/intel_panel.c
> b/drivers/gpu/drm/i915/intel_panel.c
> index e2ab3f6ed022..c8444d5f549f 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -387,7 +387,7 @@ intel_panel_detect(struct drm_device *dev)
>
> /* Assume that the BIOS does not lie through the OpRegion... */
> if (!i915.panel_ignore_lid && dev_priv->opregion.lid_state) {
> - return ioread32(dev_priv->opregion.lid_state) & 0x1 ?
> + return *(dev_priv->opregion.lid_state) & 0x1 ?
() are redundant now here.
-Daniel
> connector_status_connected :
> connector_status_disconnected;
> }
>
> _______
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: [Intel-gfx] [PATCH 09/20] i915: switch from acpi_os_ioremap to memremap
On Mon, Oct 12, 2015 at 09:12:57PM +, Williams, Dan J wrote:
> On Mon, 2015-10-12 at 09:01 +0200, Daniel Vetter wrote:
> > On Fri, Oct 09, 2015 at 06:16:25PM -0400, Dan Williams wrote:
> > > i915 expects the OpRegion to be cached (i.e. not __iomem), so explicitly
> > > map it with memremap rather than the implied cache setting of
> > > acpi_os_ioremap().
> > >
> > > Cc: Daniel Vetter <daniel.vet...@intel.com>
> > > Cc: Jani Nikula <jani.nik...@linux.intel.com>
> > > Cc: intel-...@lists.freedesktop.org
> > > Cc: David Airlie <airl...@linux.ie>
> > > Cc: dri-de...@lists.freedesktop.org
> > > Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> >
> > Assuming you've run sparse over this to make sure you've caught them all,
> > and with the nit below addressed this is
> >
> > Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
>
> Indeed, re-running sparse again found a few conversions of ioread* I
> missed as well as moving the force casting out of validate_vbt() to
> find_vbt().
>
> > Feel free to pull v2 into whatever tree you think it's suitable for (but
> > you can also resend and I'll pick it up).
>
> Please pick up v2 below.
Queued for -next, thanks for the patch. Aside: Attached or separate mail
seems easier, somehow git apply-mbox can't auto-eat this for of patch.
-Daniel
>
> >
> > > diff --git a/drivers/gpu/drm/i915/intel_panel.c
> > > b/drivers/gpu/drm/i915/intel_panel.c
> > > index e2ab3f6ed022..c8444d5f549f 100644
> > > --- a/drivers/gpu/drm/i915/intel_panel.c
> > > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > > @@ -387,7 +387,7 @@ intel_panel_detect(struct drm_device *dev)
> > >
> > > /* Assume that the BIOS does not lie through the OpRegion... */
> > > if (!i915.panel_ignore_lid && dev_priv->opregion.lid_state) {
> > > - return ioread32(dev_priv->opregion.lid_state) & 0x1 ?
> > > + return *(dev_priv->opregion.lid_state) & 0x1 ?
> >
> > () are redundant now here.
>
> Yup, fixed.
>
> 8<
> Subject: i915: switch from acpi_os_ioremap to memremap
>
> From: Dan Williams <dan.j.willi...@intel.com>
>
> i915 expects the OpRegion to be cached (i.e. not __iomem), so explicitly
> map it with memremap rather than the implied cache setting of
> acpi_os_ioremap().
>
> Cc: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Jani Nikula <jani.nik...@linux.intel.com>
> Cc: intel-...@lists.freedesktop.org
> Cc: David Airlie <airl...@linux.ie>
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c |2 -
> drivers/gpu/drm/i915/i915_drv.h | 12 ++---
> drivers/gpu/drm/i915/intel_bios.c | 25 +-
> drivers/gpu/drm/i915/intel_opregion.c | 83
> -
> drivers/gpu/drm/i915/intel_panel.c|2 -
> 5 files changed, 62 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index e3ec9049081f..15989cc16e92 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1849,7 +1849,7 @@ static int i915_opregion(struct seq_file *m, void
> *unused)
> goto out;
>
> if (opregion->header) {
> - memcpy_fromio(data, opregion->header, OPREGION_SIZE);
> + memcpy(data, opregion->header, OPREGION_SIZE);
> seq_write(m, data, OPREGION_SIZE);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e1db8de52851..d8684634a31d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -444,14 +444,14 @@ struct opregion_swsci;
> struct opregion_asle;
>
> struct intel_opregion {
> - struct opregion_header __iomem *header;
> - struct opregion_acpi __iomem *acpi;
> - struct opregion_swsci __iomem *swsci;
> + struct opregion_header *header;
> + struct opregion_acpi *acpi;
> + struct opregion_swsci *swsci;
> u32 swsci_gbda_sub_functions;
> u32 swsci_sbcb_sub_functions;
> - struct opregion_asle __iomem *asle;
> - void __iomem *vbt;
> - u32 __iomem *lid_state;
> + struct opregion_asle *asle;
> + void *vbt;
> + u32 *lid_state;
> struct work_struct asle_work;
> };
> #define OPREGION_SIZE(8*1024)
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers
Re: [PATCH] MAINTAINERS: add link to the Intel Graphics for Linux web site
On Tue, Oct 13, 2015 at 11:16:48AM +0300, Jani Nikula wrote: > There's plenty of drm/i915 related hardware and software documentation, > and firmware downloads for the latest platforms. > > Cc: Daniel Vetter <daniel.vet...@ffwll.ch> > Signed-off-by: Jani Nikula <jani.nik...@intel.com> Queued for -next, thanks for the patch. -Daniel > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 5f467845ef72..95c6bcb6bf22 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3584,6 +3584,7 @@ M: Daniel Vetter <daniel.vet...@intel.com> > M: Jani Nikula <jani.nik...@linux.intel.com> > L: intel-...@lists.freedesktop.org > L: dri-de...@lists.freedesktop.org > +W: https://01.org/linuxgraphics/ > Q: http://patchwork.freedesktop.org/project/intel-gfx/ > T: git git://anongit.freedesktop.org/drm-intel > S: Supported > -- > 2.1.4 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 v6 1/4] drm/layerscape: Add Freescale DCU DRM driver
On Fri, Jul 10, 2015 at 03:26:52PM +0800, Jianwei Wang wrote: This patch add support for Two Dimensional Animation and Compositing Engine (2D-ACE) on the Freescale SoCs. 2D-ACE is a Freescale display controller. 2D-ACE describes the functionality of the module extremely well its name is a value that cannot be used as a token in programming languages. Instead the valid token DCU is used to tag the register names and function names. The Display Controller Unit (DCU) module is a system master that fetches graphics stored in internal or external memory and displays them on a TFT LCD panel. A wide range of panel sizes is supported and the timing of the interface signals is highly configurable. Graphics are read directly from memory and then blended in real-time, which allows for dynamic content creation with minimal CPU intervention. The features: (1) Full RGB888 output to TFT LCD panel. (2) For the current LCD panel, WQVGA 480x272 is supported. (3) Blending of each pixel using up to 4 source layers dependent on size of panel. (4) Each graphic layer can be placed with one pixel resolution in either axis. (5) Each graphic layer support RGB565 and RGB888 direct colors without alpha channel and BGRA BGRA ARGB1555 direct colors with an alpha channel and YUV422 format. (6) Each graphic layer support alpha blending with 8-bit resolution. This is a simplified version, only one primary plane, one framebuffer created for fbdev, one crtc, one connector for TFT LCD panel, an encoder. Signed-off-by: Alison Wang b18...@freescale.com Signed-off-by: Xiubo Li lixi...@cmss.chinamobile.com Signed-off-by: Jianwei Wang jianwei.w...@freescale.com A few small things to polish that I spotted. -Daniel --- Changed in V6 - Add NEC nl4827hc19_05b panel to panel-simple.c Adviced by Mark Yao - Add DRIVER_ATOMIC for driver_features Adviced by Mark Yao - check fsl_dev if it's NULL at PM suspend/resume Adviced by Mark Yao Changed in V5 - Update commit message - Add layer registers initialization - Remove unused functions - Rename driver folder Adviced by Stefan Agner - Move pixel clock control functions to fsl_dcu_drm_drv.c - remove redundant enable the clock implicitly using regmap - Add maintainer message Changed in V4: -This version doesn't have functionality changed Just a minor adjustment. Changed in V3: - Test driver on Vybrid board and add compatible string - Remove unused functions - set default crtc for encoder - replace legacy functions with atomic help functions Adviced by Daniel Vetter - Set the unique name of the DRM device - Implement irq handle function for vblank interrupt Changed in v2: - Add atomic support Adviced by Daniel Vetter - Modify bindings file - Rename node for compatibility - Move platform related code out for compatibility Adviced by Stefan Agner MAINTAINERS | 7 + drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile| 1 + drivers/gpu/drm/fsl-dcu/Kconfig | 18 ++ drivers/gpu/drm/fsl-dcu/Makefile| 7 + drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_connector.c | 200 + drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_connector.h | 31 ++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 172 +++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.h | 22 ++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 379 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 223 ++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_fbdev.c | 26 ++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 42 +++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.h | 17 ++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 195 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h | 23 ++ 16 files changed, 1365 insertions(+) create mode 100644 drivers/gpu/drm/fsl-dcu/Kconfig create mode 100644 drivers/gpu/drm/fsl-dcu/Makefile create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_connector.c create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_connector.h create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.h create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_fbdev.c create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.h create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c create mode 100644 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h diff --git a/MAINTAINERS b/MAINTAINERS index 6761318..b25b948 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3404,6 +3404,13 @@ S: Maintained F: drivers/gpu/drm/imx/ F: Documentation/devicetree/bindings/drm
Re: [PATCH] drm/rockchip: use drm_gem_mmap helpers
On Tue, Jul 07, 2015 at 05:03:36PM +0800, Daniel Kurtz wrote:
Rather than (incompletely [0]) re-implementing drm_gem_mmap() and
drm_gem_mmap_obj() helpers, call them directly from the rockchip mmap
routines.
Once the core functions return successfully, the rockchip mmap routines
can still use dma_mmap_attrs() to simply mmap the entire buffer.
[0] Previously, we were performing the mmap() without first taking a
reference on the underlying gem buffer. This could leak ptes if the gem
object is destroyed while userspace is still holding the mapping.
Signed-off-by: Daniel Kurtz djku...@chromium.org
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
Cc: sta...@vger.kernel.org
Applied to topic/drm-fixes to make sure it won't get lost, but I expect
rockchip maintainers to pick this one up.
-Daniel
---
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 67
+++--
1 file changed, 34 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index eb2282c..eba5f8a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -54,55 +54,56 @@ static void rockchip_gem_free_buf(struct
rockchip_gem_object *rk_obj)
rk_obj-dma_attrs);
}
-int rockchip_gem_mmap_buf(struct drm_gem_object *obj,
- struct vm_area_struct *vma)
+static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
+ struct vm_area_struct *vma)
+
{
+ int ret;
struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
struct drm_device *drm = obj-dev;
- unsigned long vm_size;
- vma-vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
- vm_size = vma-vm_end - vma-vm_start;
-
- if (vm_size obj-size)
- return -EINVAL;
+ /*
+ * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear
+ * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
+ */
+ vma-vm_flags = ~VM_PFNMAP;
- return dma_mmap_attrs(drm-dev, vma, rk_obj-kvaddr, rk_obj-dma_addr,
+ ret = dma_mmap_attrs(drm-dev, vma, rk_obj-kvaddr, rk_obj-dma_addr,
obj-size, rk_obj-dma_attrs);
+ if (ret)
+ drm_gem_vm_close(vma);
+
+ return ret;
}
-/* drm driver mmap file operations */
-int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma)
+int rockchip_gem_mmap_buf(struct drm_gem_object *obj,
+ struct vm_area_struct *vma)
{
- struct drm_file *priv = filp-private_data;
- struct drm_device *dev = priv-minor-dev;
- struct drm_gem_object *obj;
- struct drm_vma_offset_node *node;
+ struct drm_device *drm = obj-dev;
int ret;
- if (drm_device_is_unplugged(dev))
- return -ENODEV;
+ mutex_lock(drm-struct_mutex);
+ ret = drm_gem_mmap_obj(obj, obj-size, vma);
+ mutex_unlock(drm-struct_mutex);
+ if (ret)
+ return ret;
- mutex_lock(dev-struct_mutex);
+ return rockchip_drm_gem_object_mmap(obj, vma);
+}
- node = drm_vma_offset_exact_lookup(dev-vma_offset_manager,
-vma-vm_pgoff,
-vma_pages(vma));
- if (!node) {
- mutex_unlock(dev-struct_mutex);
- DRM_ERROR(failed to find vma node.\n);
- return -EINVAL;
- } else if (!drm_vma_node_is_allowed(node, filp)) {
- mutex_unlock(dev-struct_mutex);
- return -EACCES;
- }
+/* drm driver mmap file operations */
+int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+ struct drm_gem_object *obj;
+ int ret;
- obj = container_of(node, struct drm_gem_object, vma_node);
- ret = rockchip_gem_mmap_buf(obj, vma);
+ ret = drm_gem_mmap(filp, vma);
+ if (ret)
+ return ret;
- mutex_unlock(dev-struct_mutex);
+ obj = vma-vm_private_data;
- return ret;
+ return rockchip_drm_gem_object_mmap(obj, vma);
}
struct rockchip_gem_object *
--
2.4.3.573.g4eafbef
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: WARNING: CPU: 0 PID: 3634 at drivers/gpu/drm/drm_irq.c:1141 drm_wait_one_vblank
On Wed, Jul 08, 2015 at 08:40:17AM +0200, Michal Hocko wrote: I have just tried to boot 4.2-rc1 and cannot seem to reproduce the issue anymore. On Tue 07-07-15 10:43:58, Daniel Vetter wrote: [...] Can you please boot with drm.debug=0xf (lots more nois in dmesg with this) and reproduce? I only need the last few pages before the WARNING backtrace, just to understand a bit better what's going on there. And 0xf debug level seem to paper over the problem because I do not see the warning even with 4.1 where I am able to reproduce this reliably. This suggests this is a timing sensitive issue. Ok that's starting to get nasty. Can you please test what happens when you boot with drm.vblankoffdelay=0 and whether drm.vblankoffdelay=1000 has an effect on reproduction rate? No debug output needed. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 v6 1/4] drm/layerscape: Add Freescale DCU DRM driver
On Fri, Jul 10, 2015 at 10:43:11AM +, Wang J.W. wrote:
Hi Daniel,
Thank you very much for your review!
See below...
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
Vetter
Sent: Friday, July 10, 2015 4:00 PM
To: Wang Jianwei-B52261
Cc: dri-de...@lists.freedesktop.org; linux-kernel@vger.kernel.org; linux-
arm-ker...@lists.infradead.org; devicet...@vger.kernel.org;
airl...@linux.ie; daniel.vet...@ffwll.ch; mark@rock-chips.com; Wood
Scott-B07421; Wang Huan-B18965; Xiubo Li
Subject: Re: [PATCH v6 1/4] drm/layerscape: Add Freescale DCU DRM driver
On Fri, Jul 10, 2015 at 03:26:52PM +0800, Jianwei Wang wrote:
+ .atomic_check = fsl_dcu_drm_encoder_atomic_check,
.atomic_check is optional
I try to remove .atomic_check, but it will cause CPU hang when starting up
And I find this in drivers/gpu/drm/drm_atomic_helper.c
293 if (funcs-atomic_check) {
294 ret = funcs-atomic_check(encoder, crtc_state,
295 conn_state);
296 if (ret) {
297 DRM_DEBUG_ATOMIC([ENCODER:%d:%s] check
failed\n,
298 encoder-base.id,
encoder-name);
299 return ret;
300 }
301 } else {
302 ret = funcs-mode_fixup(encoder,
crtc_state-mode,
303
crtc_state-adjusted_mode);
304 if (!ret) {
305 DRM_DEBUG_ATOMIC([ENCODER:%d:%s] fixup
failed\n,
306 encoder-base.id,
encoder-name);
307 return -EINVAL;
308 }
309 }
It means that I have to implement at least one of atomic_check and mode_fixup.
So I reserve atomic_check.
Is there problem? Please give me some more comments if necessary. Thanks!
Ah sorry you are right, atomic check is not optional. Unfortunately the
optional-or-not thing has grown a bit organically so there's no clear
rules.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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 v7 1/4] drm/layerscape: Add Freescale DCU DRM driver
On Fri, Jul 10, 2015 at 07:17:40PM +0800, Jianwei Wang wrote: This patch add support for Two Dimensional Animation and Compositing Engine (2D-ACE) on the Freescale SoCs. 2D-ACE is a Freescale display controller. 2D-ACE describes the functionality of the module extremely well its name is a value that cannot be used as a token in programming languages. Instead the valid token DCU is used to tag the register names and function names. The Display Controller Unit (DCU) module is a system master that fetches graphics stored in internal or external memory and displays them on a TFT LCD panel. A wide range of panel sizes is supported and the timing of the interface signals is highly configurable. Graphics are read directly from memory and then blended in real-time, which allows for dynamic content creation with minimal CPU intervention. The features: (1) Full RGB888 output to TFT LCD panel. (2) For the current LCD panel, WQVGA 480x272 is supported. (3) Blending of each pixel using up to 4 source layers dependent on size of panel. (4) Each graphic layer can be placed with one pixel resolution in either axis. (5) Each graphic layer support RGB565 and RGB888 direct colors without alpha channel and BGRA BGRA ARGB1555 direct colors with an alpha channel and YUV422 format. (6) Each graphic layer support alpha blending with 8-bit resolution. This is a simplified version, only one primary plane, one framebuffer created for fbdev, one crtc, one connector for TFT LCD panel, an encoder. Signed-off-by: Alison Wang b18...@freescale.com Signed-off-by: Xiubo Li lixi...@cmss.chinamobile.com Signed-off-by: Jianwei Wang jianwei.w...@freescale.com Can't find any other use of deprecated functions or legacy code patterns or anything else that we've recently started cleaning up, looks good. No detailed review though (for one I lack hw docs). Acked-by: Daniel Vetter daniel.vet...@ffwll.ch Might be good to get some cross-review from some other arm soc drm driver team, then send a pull request to Dave for 4.3. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] drm/mgag200: fix kernel hang in cursor code.
On Wed, Nov 18, 2015 at 11:00:53PM +0800, Rui Wang wrote:
> The machine hang completely with the following message on the console:
>
> [ 487.777538] BUG: unable to handle kernel NULL pointer dereference at
> 0060
> [ 487.777554] IP: [] _raw_spin_lock+0xe/0x30
> [ 487.777557] PGD 42e9f7067 PUD 42f2fa067 PMD 0
> [ 487.777560] Oops: 0002 [#1] SMP
> ...
> [ 487.777618] CPU: 21 PID: 3190 Comm: Xorg Tainted: GE
> 4.4.0-rc1-3-default+ #6
> [ 487.777620] Hardware name: Intel Corporation BRICKLAND/BRICKLAND, BIOS
> BRHSXSD1.86B.0059.R00.1501081238 01/08/2015
> [ 487.777621] task: 880853ae4680 ti: 8808696d4000 task.ti:
> 8808696d4000
> [ 487.777625] RIP: 0010:[] []
> _raw_spin_lock+0xe/0x30
> [ 487.777627] RSP: 0018:8808696d79c0 EFLAGS: 00010246
> [ 487.777628] RAX: RBX: RCX:
>
> [ 487.777629] RDX: 0001 RSI: RDI:
> 0060
> [ 487.777630] RBP: 8808696d79e0 R08: R09:
> 88086924a780
> [ 487.777631] R10: 0001bb40 R11: 3246 R12:
>
> [ 487.777632] R13: 880463a27360 R14: 88046ca50218 R15:
> 0080
> [ 487.777634] FS: 7f3f81c5a8c0() GS:88086f06()
> knlGS:
> [ 487.777635] CS: 0010 DS: ES: CR0: 80050033
> [ 487.777636] CR2: 0060 CR3: 00042e678000 CR4:
> 001406e0
> [ 487.777638] DR0: DR1: DR2:
>
> [ 487.777639] DR3: DR6: fffe0ff0 DR7:
> 0400
> [ 487.777639] Stack:
> [ 487.777642] a00eb5fa 8808696d7b60 88086b87d800
>
> [ 487.777644] 8808696d7ac8 a01694b6 8808696d7ae8
> 8109c8d5
> [ 487.777647] 880469158740 880463a27000 88086b87d800
> 88086b87d800
> [ 487.777647] Call Trace:
> [ 487.777674] [] ? drm_gem_object_lookup+0x1a/0xa0 [drm]
> [ 487.777681] [] mga_crtc_cursor_set+0xc6/0xb60 [mgag200]
> [ 487.777691] [] ? find_busiest_group+0x35/0x4a0
> [ 487.777696] [] ? __might_sleep+0x44/0x80
> [ 487.777699] [] ? __ww_mutex_lock+0x22/0x9c
> [ 487.22] [] ? drm_modeset_lock+0x34/0xf0 [drm]
> [ 487.33] [] restore_fbdev_mode+0xee/0x2a0
> [drm_kms_helper]
> [ 487.42] []
> drm_fb_helper_restore_fbdev_mode_unlocked+0x2e/0x70 [drm_kms_helper]
> [ 487.48] [] drm_fb_helper_set_par+0x27/0x50
> [drm_kms_helper]
> [ 487.52] [] fb_set_var+0x18c/0x3f0
> [ 487.77] [] ?
> __ext4_handle_dirty_metadata+0x8a/0x210 [ext4]
> [ 487.83] [] fbcon_blank+0x1b7/0x2b0
> [ 487.90] [] do_unblank_screen+0xb3/0x1c0
> [ 487.95] [] vt_ioctl+0x118a/0x1210
> [ 487.777801] [] tty_ioctl+0x3f0/0xc90
> [ 487.777808] [] ? kzfree+0x28/0x30
> [ 487.777813] [] ? mntput+0x1f/0x30
> [ 487.777817] [] do_vfs_ioctl+0x30d/0x570
> [ 487.777822] [] ? task_work_run+0x8a/0xa0
> [ 487.777825] [] SyS_ioctl+0x74/0x80
> [ 487.777829] [] entry_SYSCALL_64_fastpath+0x12/0x71
> [ 487.777851] Code: 65 ff 0d ce 02 a8 7e 5d c3 ba 01 00 00 00 f0 0f b1 17 85
> c0 75 e8 b0 01 5d c3 0f 1f 00 65 ff 05 b1 02 a8 7e 31 c0 ba 01 00 00 00
> 0f b1 17 85 c0 75 01 c3 55 89 c6 48 89 e5 e8 4e f5 b1 ff 5d
> [ 487.777854] RIP [] _raw_spin_lock+0xe/0x30
> [ 487.777855] RSP
> [ 487.777856] CR2: 0060
> [ 487.777860] ---[ end trace 672a2cd555e0ebd3 ]---
>
> The cursor code may be entered with file_priv == NULL && handle == NULL.
> The problem was introduced by:
>
> "bf89209 drm/mga200g: Hold a proper reference for cursor_set"
>
> which calls drm_gem_object_lookup(dev, file_priv...). Previously this wasn't
> a problem because we checked the handle. Move the check early in the function
> can fix the problem.
>
> Signed-off-by: Rui Wang <rui.y.w...@intel.com>
Oh dear, thanks for catching this.
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Dave, please pick this up.
-Daniel
> ---
> drivers/gpu/drm/mgag200/mgag200_cursor.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c
> b/drivers/gpu/drm/mgag200/mgag200_cursor.c
> index 4f2068f..a7bf6a9 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
> @@ -70,6 +70,11 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
> BUG_ON(pixels_2 != pixels_current && pixels_2 != pixels_prev);
> BUG_ON(pixels_current == pixels_prev);
>
> + if (!handle || !file_priv) {
> + mga_hide
Re: Regression on Chromebook Pixel 2015 due to i915 fastboot always-on
On Tue, Nov 17, 2015 at 7:18 PM, Linus Torvalds <torva...@linux-foundation.org> wrote: > On Tue, Nov 17, 2015 at 9:53 AM, Olof Johansson <o...@lixom.net> wrote: >> >> The problem as I see it is that it's unknown how many machines depends >> on previous behavior. If it's only Pixel 2015 then I think a whitelist >> would be just fine. > > Considering how many problems we historically have had with backlight > handling, I would strongly urge people to *not* start going down the > whitelist approach. > > If the backlight doesn't get set up correctly, the machine might as > well be considered dead. Very few people are going to give good > reports of it. So the backlight code needs to bend oevr backwards in > being robust even more so than most other code, and "whitelist > known-working setups" is absolutely the reverse of robust. It's a > hack, and it's guaranteed to not be maintainable. > > Yes, yes, we have whitelists for other things. I hate them in other > places too. But things like "this device has very odd audio > configuration" is very different from "this machine appears dead on > boot", for example. > > So reverting quickly is definitely the right thing to do. Or applying > the patch that apparently fixes it for Olof, and hopefully fixes it in > general - without any kind of random "on _this_ machine we do _that_" > crap. > > If drm people don't want the revert, send me a pull request with the fix. Imo revert. With all the QA awol fail we've suffered the past few months we've become a bit too lax imo with reverting fast, and the point of the split-out commit was to allow exactly that. On top I don't really like the casting Maarten's current hack does, we probably need a per-encoder ->sanitize hook for this stuff. Better to retry for 4.5. Can you pls push the revert? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- 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 v2] drm: support hotspot for universal plane cursors
On Tue, Nov 17, 2015 at 06:47:28PM +, John Keeping wrote:
> On Tue, 17 Nov 2015 19:31:32 +0100, Daniel Vetter wrote:
>
> > On Tue, Nov 17, 2015 at 12:07:24PM -0500, Alex Deucher wrote:
> > > On Tue, Nov 17, 2015 at 11:29 AM, Daniel Vetter <dan...@ffwll.ch>
> > > wrote:
> > > > On Tue, Nov 17, 2015 at 03:59:43PM +, John Keeping wrote:
> > > >> On Tue, 17 Nov 2015 17:39:32 +0200, Ville Syrjälä wrote:
> > > >>
> > > >> > On Tue, Nov 17, 2015 at 03:05:34PM +, John Keeping wrote:
> > > >> > > The request's hot_x and hot_y are set correctly for both
> > > >> > > DRM_IOCTL_MODE_CURSOR and DRM_IOCTL_MODE_CURSOR2 so we just
> > > >> > > need to save the values and then apply the offset to the
> > > >> > > cursor plane when the cursor moves.
> > > >> > >
> > > >> > > Signed-off-by: John Keeping <j...@metanate.com>
> > > >> > > ---
> > > >> > > v2:
> > > >> > > - add kerneldoc for hot_x and hot_y in struct drm_crtc
> > > >> > >
> > > >> > > drivers/gpu/drm/drm_crtc.c | 11 +++
> > > >> > > include/drm/drm_crtc.h | 6 ++
> > > >> > > 2 files changed, 13 insertions(+), 4 deletions(-)
> > > >> > >
> > > >> > > diff --git a/drivers/gpu/drm/drm_crtc.c
> > > >> > > b/drivers/gpu/drm/drm_crtc.c index 720a153..40f5b84 100644
> > > >> > > --- a/drivers/gpu/drm/drm_crtc.c
> > > >> > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > >> > > @@ -2831,6 +2831,9 @@ static int
> > > >> > > drm_mode_cursor_universal(struct drm_crtc *crtc,
> > > >> > > DRM_DEBUG_KMS("failed to wrap cursor buffer in drm
> > > >> > > framebuffer\n"); return PTR_ERR(fb); }
> > > >> > > +
> > > >> > > + crtc->hot_x = req->hot_x;
> > > >> > > + crtc->hot_y = req->hot_y;
> > > >> > > } else {
> > > >> > > fb = NULL;
> > > >> > > }
> > > >> > > @@ -2841,11 +2844,11 @@ static int
> > > >> > > drm_mode_cursor_universal(struct drm_crtc *crtc, }
> > > >> > >
> > > >> > > if (req->flags & DRM_MODE_CURSOR_MOVE) {
> > > >> > > - crtc_x = req->x;
> > > >> > > - crtc_y = req->y;
> > > >> > > + crtc_x = req->x - crtc->hot_x;
> > > >> > > + crtc_y = req->y - crtc->hot_y;
> > > >> > > } else {
> > > >> > > - crtc_x = crtc->cursor_x;
> > > >> > > - crtc_y = crtc->cursor_y;
> > > >> > > + crtc_x = crtc->cursor_x - crtc->hot_x;
> > > >> > > + crtc_y = crtc->cursor_y - crtc->hot_y;
> > > >> >
> > > >> > Why does the location of the hotspot affect the plane
> > > >> > position?
> > > >>
> > > >> hot_{x,y} specify the location of the active pixel within the
> > > >> cursor plane and cursor_{x,y} specify the location of the active
> > > >> pixel on the display so we need to offset the plane position in
> > > >> order for the active pixel to be in the correct place.
> > > >
> > > > Nope, hot_x/y is just for virtual machines to indicate where the
> > > > logical cursor position is within the cursor plane. It should
> > > > have 2 effect on how something is displayed. And no, I have
> > > > absolutely no idea why radeon is pulling some tricks here, which
> > > > have been added in
> > > >
> > > > commit 78b1a6010b46a69bcd47b723a80f92693f26d17b
> > > > Author: Michel Dänzer <michel.daen...@amd.com>
> > > > Date: Tue Nov 18 18:00:08 2014 +0900
> > > >
> > > > drm/radeon: Use cursor_set2 hook for enabling / disabling the
> > > > HW cursor
> > > >
> > > > Michel/Alex, can you please shed some light onto this? radeon is
> > > > the only driver doing this, making this interface inconsistent.
> > > > Is the ddx doing something funny compared to -modeseting or
>
Re: [PATCH] drm/tegra: fix locking of SET_TILING ioctl
On Thu, Nov 12, 2015 at 04:09:56PM +0900, Alexandre Courbot wrote: > drm_gem_object_unreference() now expects obj->dev->struct_mutex to be > held. Use the newly-introduced drm_gem_object_unreference_unlocked() > which handles locking for us. This rule has been really old, I simply made the checking more anal in commit ef4c6270bf2867e2f8032e9614d1a8cfc6c71663 Author: Daniel Vetter <daniel.vet...@ffwll.ch> Date: Thu Oct 15 09:36:25 2015 +0200 drm/gem: Check locking in drm_gem_object_unreference With the above added this is: Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch> > > If we don't do this drm_gem_object_unreference() will get noisy about > struct_mutex not being held every time we call the SET_TILING ioctl. > > Signed-off-by: Alexandre Courbot <acour...@nvidia.com> > --- > drivers/gpu/drm/tegra/drm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c > index cc48334ef164..c0ae89865958 100644 > --- a/drivers/gpu/drm/tegra/drm.c > +++ b/drivers/gpu/drm/tegra/drm.c > @@ -683,7 +683,7 @@ static int tegra_gem_set_tiling(struct drm_device *drm, > void *data, > bo->tiling.mode = mode; > bo->tiling.value = value; > > - drm_gem_object_unreference(gem); > + drm_gem_object_unreference_unlocked(gem); > > return 0; > } > -- > 2.6.2 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 v2 3/7] drm/vc4: Add KMS support for Raspberry Pi.
On Tue, Aug 18, 2015 at 02:54:04PM -0700, Eric Anholt wrote: This is the start of a full VC4 driver. Right now this just supports configuring the display using a pre-existing video mode (because changing the pixel clock isn't available yet, and doesn't work when it is). However, this is enough for fbcon and bringing up X using xf86-video-modesetting. Signed-off-by: Eric Anholt e...@anholt.net --- v2: Drop FB_HELPER select thanks to Archit's patches. Do manual init ordering instead of using the .load hook. Structure registration more like tegra's, but still using the typical component code. Drop no-op hooks for atomic_begin and mode_fixup() now that they're optional. Drop sentinel in Makefile. Fix minor style nits I noticed on another reread. fwiw Acked-by: Daniel Vetter daniel.vet...@ffwll.ch since Eric has done all the changes I requested - simply forgot to add that in my earlier review. No full r-b since I don't have a clue about the hw (and didn't take a close look at the hw touching code hence). Cheers, Daniel drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/vc4/Kconfig | 13 + drivers/gpu/drm/vc4/Makefile | 17 + drivers/gpu/drm/vc4/vc4_bo.c | 52 drivers/gpu/drm/vc4/vc4_crtc.c| 565 ++ drivers/gpu/drm/vc4/vc4_debugfs.c | 38 +++ drivers/gpu/drm/vc4/vc4_drv.c | 271 drivers/gpu/drm/vc4/vc4_drv.h | 120 drivers/gpu/drm/vc4/vc4_hdmi.c| 633 ++ drivers/gpu/drm/vc4/vc4_hvs.c | 161 ++ drivers/gpu/drm/vc4/vc4_kms.c | 84 + drivers/gpu/drm/vc4/vc4_plane.c | 320 +++ drivers/gpu/drm/vc4/vc4_regs.h| 562 + 14 files changed, 2839 insertions(+) create mode 100644 drivers/gpu/drm/vc4/Kconfig create mode 100644 drivers/gpu/drm/vc4/Makefile create mode 100644 drivers/gpu/drm/vc4/vc4_bo.c create mode 100644 drivers/gpu/drm/vc4/vc4_crtc.c create mode 100644 drivers/gpu/drm/vc4/vc4_debugfs.c create mode 100644 drivers/gpu/drm/vc4/vc4_drv.c create mode 100644 drivers/gpu/drm/vc4/vc4_drv.h create mode 100644 drivers/gpu/drm/vc4/vc4_hdmi.c create mode 100644 drivers/gpu/drm/vc4/vc4_hvs.c create mode 100644 drivers/gpu/drm/vc4/vc4_kms.c create mode 100644 drivers/gpu/drm/vc4/vc4_plane.c create mode 100644 drivers/gpu/drm/vc4/vc4_regs.h diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 06ae500..19bb0db 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -260,3 +260,5 @@ source drivers/gpu/drm/sti/Kconfig source drivers/gpu/drm/amd/amdkfd/Kconfig source drivers/gpu/drm/imx/Kconfig + +source drivers/gpu/drm/vc4/Kconfig diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 8858510..283ed8e 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -42,6 +42,7 @@ obj-$(CONFIG_DRM_MGA) += mga/ obj-$(CONFIG_DRM_I810) += i810/ obj-$(CONFIG_DRM_I915) += i915/ obj-$(CONFIG_DRM_MGAG200) += mgag200/ +obj-$(CONFIG_DRM_VC4) += vc4/ obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/ obj-$(CONFIG_DRM_SIS) += sis/ obj-$(CONFIG_DRM_SAVAGE)+= savage/ diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig new file mode 100644 index 000..e810ef7 --- /dev/null +++ b/drivers/gpu/drm/vc4/Kconfig @@ -0,0 +1,13 @@ +config DRM_VC4 + tristate Broadcom VC4 Graphics + depends on ARCH_BCM2835 + depends on DRM + select DRM_KMS_HELPER + select DRM_KMS_CMA_HELPER + help + Choose this option if you have a system that has a Broadcom + VC4 GPU, such as the Raspberry Pi or other BCM2708/BCM2835. + + This driver requires that avoid_warnings=2 be present in + the config.txt for the firmware, to keep it from smashing + our display setup. diff --git a/drivers/gpu/drm/vc4/Makefile b/drivers/gpu/drm/vc4/Makefile new file mode 100644 index 000..32b4f9c --- /dev/null +++ b/drivers/gpu/drm/vc4/Makefile @@ -0,0 +1,17 @@ +ccflags-y := -Iinclude/drm + +# Please keep these build lists sorted! + +# core driver code +vc4-y := \ + vc4_bo.o \ + vc4_crtc.o \ + vc4_drv.o \ + vc4_kms.o \ + vc4_hdmi.o \ + vc4_hvs.o \ + vc4_plane.o + +vc4-$(CONFIG_DEBUG_FS) += vc4_debugfs.o + +obj-$(CONFIG_DRM_VC4) += vc4.o diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c new file mode 100644 index 000..ab9f510 --- /dev/null +++ b/drivers/gpu/drm/vc4/vc4_bo.c @@ -0,0 +1,52 @@ +/* + * Copyright © 2015 Broadcom + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/* DOC: VC4 GEM BO management support
Re: [PATCH 1/7] drm/vc4: Add devicetree bindings for VC4.
On Tue, Aug 25, 2015 at 04:42:18PM -0400, Rob Clark wrote: On Mon, Aug 24, 2015 at 9:47 AM, Rob Herring robherri...@gmail.com wrote: On Mon, Aug 17, 2015 at 1:30 PM, Eric Anholt e...@anholt.net wrote: Stephen Warren swar...@wwwdotorg.org writes: On 08/12/2015 06:56 PM, Eric Anholt wrote: Signed-off-by: Eric Anholt e...@anholt.net This one definitely needs a patch description, since someone might not know what a VC4 is, and git log won't show the text from the binding doc itself. I'd suggest adding the initial paragraph of the binding doc as the patch description, or more. diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt +- hvss: List of references to HVS video scalers +- encoders: List of references to output encoders (HDMI, SDTV) Would it make sense to make all those nodes child node of the vc4 object. That way, there's no need to have these lists of objects; they can be automatically built up as the DT is enumerated. I know that e.g. the NVIDIA Tegra host1x binding works this way, and I think it may have been inspired by other similar cases. I've looked at tegra, and the component system used by msm appears to be nicer than it. To follow tegra's model, it looks like I need to build this extra bus thing corresponding to host1x that is effectively the drivers/base/component.c code, so that I can get at vc4's structure from the component drivers. Of course, this is only appropriate if the HW modules really are logically children of the VC4 HW module. Perhaps they aren't. If they aren't though, I wonder what this vc4 module actually represents in HW? It's the subsystem, same as we use a subsystem node for msm, sti, rockchip, imx, and exynos. This appears to be the common model of how the collection of graphics-related components is represented in the DT. I think most of these bindings are wrong. They are grouped together because that is what DRM wants not because that reflects the h/w. So convince me this is one block, not that it is what other people do. I think, when it comes to more complex driver subsystems (like drm in particular) we have a bit of mismatch between how things look from the pure hw ignoring sw perspective, and the how sw and in particular userspace expects things perspective. Maybe it is less a problem in other subsystems, where bindings map to things that are only visible in the kernel, or well defined devices like uart or sata controller. But when given the choice, I'm going to err on the side of not confusing userspace and the large software stack that sits above drm/kms, over dt purity. Maybe it would be nice to have a sort of dt overlay that adds the bits needed to tie together hw blocks that should be assembled into a single logical device for linux and userspace (but maybe not some other hypothetical operating system). But so far that doesn't exist. All we have is a hammer (devicetree), everything looks like a nail. End result is we end up adding some things in the bindings which aren't purely about the hw. Until someone invents a screwdriver, I'm not sure what else to do. In the end, other hypothetical OS is free to ignore those extra fields in the bindings if it doesn't need them. So meh? I thought we agreed a while back that these kind of pull everything for the logical device together dt nodes which just have piles of phandles are totally accepted? At least that's the point behind the component helpers, and Eric even suggested to create dt-specific component helpers to cut down a bit on the usual boilerplate. dt maintainers are also fine with this approach afaik. From my understanding tegra with the host1x bus really is the odd one out and not the norm. Given that and with the hope that we'll eventually see a dt-enabled component functions to standardize this even more the overall concept is Acked-by: Daniel Vetter daniel.vet...@ffwll.ch Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 2/3] drm:msm: Initial Add Writeback Support (V2)
On Tue, Aug 25, 2015 at 9:11 PM, Rob Clark robdcl...@gmail.com wrote: On Tue, Aug 25, 2015 at 3:05 AM, Daniel Vetter dan...@ffwll.ch wrote: On Wed, Aug 19, 2015 at 03:00:04PM -0400, Rob Clark wrote: On Tue, Apr 7, 2015 at 2:09 PM, Jilai Wang jil...@codeaurora.org wrote: So one thing that I wanted sorting out before we let userspace see streaming writeback (where I do think v4l is the right interface), is a way to deal w/ permissions/security.. Ie. only the kms master should control access to writeback. Ie. an process that the compositor isn't aware of / doesn't trust, should not be able to open the v4l device and start snooping on the screen contents. And I don't think just file permissions in /dev is sufficient. You likely don't want to run your helper process doing video encode and streaming as a privilaged user. One way to handle this would be some sort of dri2 style getmagic/authmagic sort of interface between the drm/kms master, and v4l device, to unlock streaming. But that is kind of passe. Fd passing is the fashionable thing now. So instead we could use a dummy v4l2_file_opererations::open() which always returns an error. So v4l device shows up in /dev.. but no userspace can open it. And instead, the way to get a fd for the v4l dev would be via a drm/kms ioctl (with DRM_MASTER flag set). Once compositor gets the fd, it can use fd passing, if needed, to hand it off to a helper process, etc. (probably use something like alloc_file() to get the 'struct file *', then call directly into v4l2_fh_open(), and then get_unused_fd_flags() + fd_install() to get fd to return to userspace) Just following up here, but consensus from the lpc track is that we don't need this as long as writeback is something which needs a specific action to initiate. I.e. if we have a separate writeback connector which won't grab any frames at all as long as it's disconnected we should be fine. Wrt session handling that's something which would need to be fixed between v4l and logind (or whatever). Was that consensus? I agree that something should initiate writeback from the kms side of things. But if we don't have *something* to ensure whatever is on the other end of writeback is who we think it is, it seems at least racy.. In general I don't like hand-rolling our own proprietary v4l-open process, it means that all the existing v4l testdev tooling must be changed, even when you're root. well, I know that, for example, gst v4l2src allows you to pass in an already opened v4l dev fd, which fits in pretty well with what I propose. The alternative, I think, is a dri2 style auth handshake between drm/kms and v4l, which I am less thrilled about. I would have to *assume* that userspace is at least prepared to deal with -EPERM when it tries to open a device.. at least more so than special auth ioctl sequence. Imo the right approach for that is to defer to logind. We already have these issues, e.g. you probably don't want the wrong seat to be able to access camera devices. Maybe no one bothered to support this yet, but the problem is definitely there already. And with that broder use-case of just making sure the right process can access the v4l device nodes drm writeback is just another special case. At least that's what I wanted to say at lpc, might have done a bad job at phrasing it ;-) DRM auth dance otoh is something entirely different and only needed because we want multiple processes to use the same device. v4l doesn't support that model yet at all, it really only needs some form of revoke + logind adjusting permissions to match whatever the current user is on a given seat. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- 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] drm/doc: Fixing xml documentation warning
On Fri, Aug 21, 2015 at 04:46:14PM -0300, Danilo Cesar Lemes de Paula wrote:
/** should be used for kernel-doc documentation only.
It causes a warning with the new in struct body format.
Signed-off-by: Danilo Cesar Lemes de Paula danilo.ce...@collabora.co.uk
Cc: Randy Dunlap rdun...@infradead.org
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: Jonathan Corbet cor...@lwn.net
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: Stephan Mueller smuel...@chronox.de
Cc: Michal Marek mma...@suse.cz
Cc: linux-kernel@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: intel-gfx intel-...@lists.freedesktop.org
Cc: dri-devel dri-de...@lists.freedesktop.org
Cc: Graham Whaley graham.wha...@linux.intel.com
Applied to drm-misc, thanks.
-Daniel
---
include/drm/drm_modeset_lock.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 70595ff..c16a3ec 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -43,19 +43,19 @@ struct drm_modeset_acquire_ctx {
struct ww_acquire_ctx ww_ctx;
- /**
+ /*
* Contended lock: if a lock is contended you should only call
* drm_modeset_backoff() which drops locks and slow-locks the
* contended lock.
*/
struct drm_modeset_lock *contended;
- /**
+ /*
* list of held locks (drm_modeset_lock)
*/
struct list_head locked;
- /**
+ /*
* Trylock mode, use only for panic handlers!
*/
bool trylock_only;
@@ -70,12 +70,12 @@ struct drm_modeset_acquire_ctx {
* Used for locking CRTCs and other modeset resources.
*/
struct drm_modeset_lock {
- /**
+ /*
* modeset lock
*/
struct ww_mutex mutex;
- /**
+ /*
* Resources that are locked as part of an atomic update are added
* to a list (so we know what to unlock at the end).
*/
--
2.4.3
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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] drm/crtc: Add a helper func to get a registered crtc from its index
On Tue, Aug 25, 2015 at 11:13:51AM +0800, Xinliang Liu wrote:
This patch add a helper func to get a registered crtc from its index.
In some case, where we know the crtc's index and we want to know the
crtc too.
For example, the enable_vblank func of struct drm_driver:
In the implementation of this func, we know the index of the crtc but
we want to know the crtc. This helper func can get the crtc easily.
A sample impelmentation of enable_vblank is as shown as bellow:
int hisi_drm_crtc_enable_vblank(struct drm_device *dev, int c)
{
struct drm_crtc *crtc = drm_get_crtc_from_index(dev, c);
struct hisi_crtc *hcrtc = to_hisi_crtc(crtc);
struct hisi_crtc_ops *ops = hcrtc-ops;
int ret = 0;
if (ops-enable_vblank)
ret = ops-enable_vblank(hcrtc);
return ret;
}
Signed-off-by: Xinliang Liu xinliang@linaro.org
Yeah unfortunately drm_irq.c is still stick in the old pre-KMS days. I
think we should go a bit further here though to allow new drivers to be
completely free of int pipe:
- add a new array pointer dev-mode_conifg.crtc_arr, which is
(re-)allocated in drm_crtc_init_with_planes. Then a pipe-crtc lookup
will be just
crtc = dev-mode_config.crtc_arr[pipe];
- add new hooks for vblank handling int drm_crtc_helper_funcs for
enable_vblanke, disable_vblank, get_vblank_timestamp and get_scanoutpos.
Ofc also anotate the docs for the existing hooks and make it clear new
drivers should use the new ones. Ofc these new hooks should directly
take a struct drm_crtc * instead of inte pipe.
- change the code in drm_irq.c to wrap all callbacks and first check
whether the new ones are there and only if that's not the case call the
old ones.
With these changes drivers can be completely free of int pipe and use
struct drm_crtc exclusivly I think, and the mess would be fully restricted
to drm_irq.c.
Cheers, Daniel
---
drivers/gpu/drm/drm_crtc.c | 25 +
include/drm/drm_crtc.h | 2 ++
2 files changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b9ba061..8764765 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -747,6 +747,31 @@ unsigned int drm_crtc_index(struct drm_crtc *crtc)
}
EXPORT_SYMBOL(drm_crtc_index);
+/**
+ * drm_get_crtc_from_index - find a registered CRTC from the index
+ * @dev: DRM device
+ * @index: index of a registered CRTC
+ *
+ * Given a index, return the registered CRTC within a DRM
+ * device's list of CRTCs.
+ */
+struct drm_crtc *drm_get_crtc_from_index(struct drm_device *dev,
+ unsigned int index)
+{
+ unsigned int index_tmp = 0;
+ struct drm_crtc *crtc;
+
+ list_for_each_entry(crtc, dev-mode_config.crtc_list, head) {
+ if (index_tmp == index)
+ return crtc;
+
+ index_tmp++;
+ }
+
+ BUG();
+}
+EXPORT_SYMBOL(drm_get_crtc_from_index);
+
/*
* drm_mode_remove - remove and free a mode
* @connector: connector list to modify
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 57ca8cc..3a46d39d 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1225,6 +1225,8 @@ extern int drm_crtc_init_with_planes(struct drm_device
*dev,
const struct drm_crtc_funcs *funcs);
extern void drm_crtc_cleanup(struct drm_crtc *crtc);
extern unsigned int drm_crtc_index(struct drm_crtc *crtc);
+extern struct drm_crtc *drm_get_crtc_from_index(struct drm_device *dev,
+ unsigned int index);
/**
* drm_crtc_mask - find the mask of a registered CRTC
--
1.9.1
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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 1/1] signals: kill block_all_signals() and unblock_all_signals()
On Thu, Aug 27, 2015 at 06:25:29PM +0200, Oleg Nesterov wrote:
> It is hardly possible to enumerate all problems with block_all_signals()
> and unblock_all_signals(). Just for example,
>
> 1. block_all_signals(SIGSTOP/etc) simply can't help if the caller is
>multithreaded. Another thread can dequeue the signal and force the
>group stop.
>
> 2. Even is the caller is single-threaded, it will "stop" anyway. It
>will not sleep, but it will spin in kernel space until SIGCONT or
>SIGKILL.
>
> And a lot more. In short, this interface doesn't work at all, at least
> the last 10+ years.
>
> Signed-off-by: Oleg Nesterov <o...@redhat.com>
Yeah the only times I played around with the DRM_LOCK stuff was when old
drivers accidentally deadlocked - my impression is that the entire
DRM_LOCK thing was never really tested properly ;-) Hence I'm all for
purging where this leaks out of the drm subsystem.
Acked-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> ---
> drivers/gpu/drm/drm_lock.c | 41 ---
> include/drm/drmP.h |1 -
> include/linux/sched.h |7 +-
> kernel/signal.c| 51
> +---
> 4 files changed, 2 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
> index f861361..4477b87 100644
> --- a/drivers/gpu/drm/drm_lock.c
> +++ b/drivers/gpu/drm/drm_lock.c
> @@ -38,8 +38,6 @@
> #include "drm_legacy.h"
> #include "drm_internal.h"
>
> -static int drm_notifier(void *priv);
> -
> static int drm_lock_take(struct drm_lock_data *lock_data, unsigned int
> context);
>
> /**
> @@ -115,14 +113,8 @@ int drm_legacy_lock(struct drm_device *dev, void *data,
>* really probably not the correct answer but lets us debug xkb
>* xserver for now */
> if (!file_priv->is_master) {
> - sigemptyset(>sigmask);
> - sigaddset(>sigmask, SIGSTOP);
> - sigaddset(>sigmask, SIGTSTP);
> - sigaddset(>sigmask, SIGTTIN);
> - sigaddset(>sigmask, SIGTTOU);
> dev->sigdata.context = lock->context;
> dev->sigdata.lock = master->lock.hw_lock;
> - block_all_signals(drm_notifier, dev, >sigmask);
> }
>
> if (dev->driver->dma_quiescent && (lock->flags & _DRM_LOCK_QUIESCENT))
> @@ -163,7 +155,6 @@ int drm_legacy_unlock(struct drm_device *dev, void *data,
> struct drm_file *file_
> /* FIXME: Should really bail out here. */
> }
>
> - unblock_all_signals();
> return 0;
> }
>
> @@ -282,38 +273,6 @@ int drm_legacy_lock_free(struct drm_lock_data
> *lock_data, unsigned int context)
> }
>
> /**
> - * If we get here, it means that the process has called DRM_IOCTL_LOCK
> - * without calling DRM_IOCTL_UNLOCK.
> - *
> - * If the lock is not held, then let the signal proceed as usual. If the
> lock
> - * is held, then set the contended flag and keep the signal blocked.
> - *
> - * \param priv pointer to a drm_device structure.
> - * \return one if the signal should be delivered normally, or zero if the
> - * signal should be blocked.
> - */
> -static int drm_notifier(void *priv)
> -{
> - struct drm_device *dev = priv;
> - struct drm_hw_lock *lock = dev->sigdata.lock;
> - unsigned int old, new, prev;
> -
> - /* Allow signal delivery if lock isn't held */
> - if (!lock || !_DRM_LOCK_IS_HELD(lock->lock)
> - || _DRM_LOCKING_CONTEXT(lock->lock) != dev->sigdata.context)
> - return 1;
> -
> - /* Otherwise, set flag to force call to
> -drmUnlock */
> - do {
> - old = lock->lock;
> - new = old | _DRM_LOCK_CONT;
> - prev = cmpxchg(>lock, old, new);
> - } while (prev != old);
> - return 0;
> -}
> -
> -/**
> * This function returns immediately and takes the hw lock
> * with the kernel context if it is free, otherwise it gets the highest
> priority when and if
> * it is eventually released.
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 62c4077..0859c35 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -815,7 +815,6 @@ struct drm_device {
>
> struct drm_sg_mem *sg; /**< Scatter gather memory */
> unsigned int num_crtcs; /**< Number of CRTCs on this
> device */
> - sigset_t sigmask;
>
> struct {
> int context;
> diff --git a/include/linux/sched.h b/include/linux/s
Re: linux-next: build failure after merge of the drm-misc tree
On Tue, Sep 08, 2015 at 06:48:34AM +0200, Maarten Lankhorst wrote:
> Op 08-09-15 om 01:42 schreef Stephen Rothwell:
> > Hi all,
> >
> > On Thu, 3 Sep 2015 10:49:19 +1000 Stephen Rothwell <s...@canb.auug.org.au>
> > wrote:
> >> After merging the drm-misc tree, today's linux-next build (arm
> >> multi_v7_defconfig) failed like this:
> >>
> >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c: In function
> >> 'mdp5_plane_cleanup_fb':
> >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:272:26: error: 'fb' redeclared
> >> as different kind of symbol
> >> struct drm_framebuffer *fb = old_state->fb;
> >> ^
> >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:267:27: note: previous
> >> definition of 'fb' was here
> >>struct drm_framebuffer *fb,
> >>^
> >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c: At top level:
> >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:383:3: warning: initialization
> >> from incompatible pointer type
> >>.cleanup_fb = mdp5_plane_cleanup_fb,
> >>^
> >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:383:3: warning: (near
> >> initialization for 'mdp5_plane_helper_funcs.cleanup_fb')
> >>
> >> Caused by commit
> >>
> >> a317290af0db ("drm/atomic: Make prepare_fb/cleanup_fb only take state,
> >> v3")
> >>
> >> Well, that was clearly never build tested :-(
> >>
> >> I have used the drm-misc tree from next-20150902 for today.
> > This is still in today's linux-next :-(
> >
> I just got around to making an amend fix patch for it yesterday. Can you fold
> it into v3 daniel?
Squashed in, thanks.
-Daniel
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> index 4d390b699e9a..a0f5ff0ce8dc 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -264,7 +264,6 @@ static int mdp5_plane_prepare_fb(struct drm_plane *plane,
> }
>
> static void mdp5_plane_cleanup_fb(struct drm_plane *plane,
> - struct drm_framebuffer *fb,
> const struct drm_plane_state *old_state)
> {
> struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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 2/3] drm:msm: Initial Add Writeback Support (V2)
On Wed, Aug 19, 2015 at 03:00:04PM -0400, Rob Clark wrote: On Tue, Apr 7, 2015 at 2:09 PM, Jilai Wang jil...@codeaurora.org wrote: So one thing that I wanted sorting out before we let userspace see streaming writeback (where I do think v4l is the right interface), is a way to deal w/ permissions/security.. Ie. only the kms master should control access to writeback. Ie. an process that the compositor isn't aware of / doesn't trust, should not be able to open the v4l device and start snooping on the screen contents. And I don't think just file permissions in /dev is sufficient. You likely don't want to run your helper process doing video encode and streaming as a privilaged user. One way to handle this would be some sort of dri2 style getmagic/authmagic sort of interface between the drm/kms master, and v4l device, to unlock streaming. But that is kind of passe. Fd passing is the fashionable thing now. So instead we could use a dummy v4l2_file_opererations::open() which always returns an error. So v4l device shows up in /dev.. but no userspace can open it. And instead, the way to get a fd for the v4l dev would be via a drm/kms ioctl (with DRM_MASTER flag set). Once compositor gets the fd, it can use fd passing, if needed, to hand it off to a helper process, etc. (probably use something like alloc_file() to get the 'struct file *', then call directly into v4l2_fh_open(), and then get_unused_fd_flags() + fd_install() to get fd to return to userspace) Just following up here, but consensus from the lpc track is that we don't need this as long as writeback is something which needs a specific action to initiate. I.e. if we have a separate writeback connector which won't grab any frames at all as long as it's disconnected we should be fine. Wrt session handling that's something which would need to be fixed between v4l and logind (or whatever). In general I don't like hand-rolling our own proprietary v4l-open process, it means that all the existing v4l testdev tooling must be changed, even when you're root. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] drm/doc: Fixing xml documentation warning
On Fri, Aug 21, 2015 at 04:46:14PM -0300, Danilo Cesar Lemes de Paula wrote:
/** should be used for kernel-doc documentation only.
It causes a warning with the new in struct body format.
Signed-off-by: Danilo Cesar Lemes de Paula danilo.ce...@collabora.co.uk
Cc: Randy Dunlap rdun...@infradead.org
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: Jonathan Corbet cor...@lwn.net
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: Stephan Mueller smuel...@chronox.de
Cc: Michal Marek mma...@suse.cz
Cc: linux-kernel@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: intel-gfx intel-...@lists.freedesktop.org
Cc: dri-devel dri-de...@lists.freedesktop.org
Cc: Graham Whaley graham.wha...@linux.intel.com
---
include/drm/drm_modeset_lock.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 70595ff..c16a3ec 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -43,19 +43,19 @@ struct drm_modeset_acquire_ctx {
struct ww_acquire_ctx ww_ctx;
- /**
+ /*
* Contended lock: if a lock is contended you should only call
* drm_modeset_backoff() which drops locks and slow-locks the
* contended lock.
*/
struct drm_modeset_lock *contended;
On second thought this is perfect use-case for the new per-member
kerneldoc support we've recently added.
- /**
+ /*
* list of held locks (drm_modeset_lock)
*/
struct list_head locked;
- /**
+ /*
* Trylock mode, use only for panic handlers!
*/
bool trylock_only;
@@ -70,12 +70,12 @@ struct drm_modeset_acquire_ctx {
* Used for locking CRTCs and other modeset resources.
*/
struct drm_modeset_lock {
- /**
+ /*
* modeset lock
*/
Same here.
-Daniel
struct ww_mutex mutex;
- /**
+ /*
* Resources that are locked as part of an atomic update are added
* to a list (so we know what to unlock at the end).
*/
--
2.4.3
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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] drm/crtc: Add a helper func to get a registered crtc from its index
On Thu, Sep 10, 2015 at 04:07:16PM +0800, Xinliang Liu wrote:
> On 25 August 2015 at 17:36, Daniel Vetter <dan...@ffwll.ch> wrote:
> Hi Daniel,
> Thank you very much for reply. Sorry, I just come back from vacation.
> Very happy that you have a good idea to solve the mess.
> Looking forward to see your patch soon!
>
> On Tue, Aug 25, 2015 at 11:13:51AM +0800, Xinliang Liu wrote:
> > > This patch add a helper func to get a registered crtc from its index.
> > > In some case, where we know the crtc's index and we want to know the
> > > crtc too.
> > >
> > > For example, the enable_vblank func of struct drm_driver:
> > > In the implementation of this func, we know the index of the crtc but
> > > we want to know the crtc. This helper func can get the crtc easily.
> > > A sample impelmentation of enable_vblank is as shown as bellow:
> > >
> > > int hisi_drm_crtc_enable_vblank(struct drm_device *dev, int c)
> > > {
> > > struct drm_crtc *crtc = drm_get_crtc_from_index(dev, c);
> > > struct hisi_crtc *hcrtc = to_hisi_crtc(crtc);
> > > struct hisi_crtc_ops *ops = hcrtc->ops;
> > > int ret = 0;
> > >
> > > if (ops->enable_vblank)
> > > ret = ops->enable_vblank(hcrtc);
> > >
> > > return ret;
> > > }
> > >
> > > Signed-off-by: Xinliang Liu <xinliang@linaro.org>
> >
> > Yeah unfortunately drm_irq.c is still stick in the old pre-KMS days. I
> > think we should go a bit further here though to allow new drivers to be
> > completely free of int pipe:
> >
> > - add a new array pointer dev->mode_conifg.crtc_arr, which is
> > (re-)allocated in drm_crtc_init_with_planes. Then a pipe->crtc lookup
> > will be just
> >
> > crtc = dev->mode_config.crtc_arr[pipe];
> >
> > - add new hooks for vblank handling int drm_crtc_helper_funcs for
> > enable_vblanke, disable_vblank, get_vblank_timestamp and get_scanoutpos.
> > Ofc also anotate the docs for the existing hooks and make it clear new
> > drivers should use the new ones. Ofc these new hooks should directly
> > take a struct drm_crtc * instead of inte pipe.
> >
> I agree too. It seems that vblank is a part of crtc.
>
>
> > - change the code in drm_irq.c to wrap all callbacks and first check
> > whether the new ones are there and only if that's not the case call the
> > old ones.
> >
> > With these changes drivers can be completely free of int pipe and use
> > struct drm_crtc exclusivly I think, and the mess would be fully restricted
> > to drm_irq.c.
> >
> For the drm_irq_install(struct drm_device *dev, int irq) function, I
> suggest to add one param such as "void *data" to pass crtc
> so that in the irq_handler we can find crtc easily.
Imo there's no need to change drm_irq_install at all, this is just a
convenience wrapper for (mostly pci) devices which only have 1 interrupt.
If this doesn't fit your hw (e.g. per-crtc interrupt sources) then just
don't use it, it's purely optional and there's a bunch of drivers which
don't use it.
I have a long-term plan to split the vblank handling code out from the
optional irq handling parts into a new drm_vblank.c file, so that this
split between optional irq helpers and core vblank infrastructure is
clearer.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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 0/6] scripts/kernel-doc: Kernel-doc improvements
On Sat, Sep 12, 2015 at 03:24:49PM -0600, Jonathan Corbet wrote: > On Mon, 7 Sep 2015 17:01:58 -0300 > Danilo Cesar Lemes de Paula <danilo.ce...@collabora.co.uk> wrote: > > > The following series contains: > > * kernel-doc: markdown support and improvements. > > OK, I've spent a while looking this stuff over. I like the general idea, > but I do have a couple of concerns. > > 1 Installing pandoc on a Fedora system wants to drag in 70(!) packages > for 100MB of total stuff. Installing it on Arch is ... well ... enough > to make you want to switch to Fedora. If we add a dependency on a tool > this massive, people are going to complain, loudly. > > Have you looked at using something like cmark instead? I don't know > the tool well, but it seems like it can do the job simply enough. It's > focused, written in C, and doesn't drag in a diskful of Haskell > stuff. There's lot of other converters out there too, I'm not tied to > this one (though I think CommonMark deserves support), but I do think > this question needs to be considered. Personally I don't care which kind of text markup we pick and wich converter, as long as the project looks reasonable far away from immeninent death (way too many one-person projects on github in this area). But if we have this discussion I'd like to decouple it from the other kerneldoc improvemnts in this series (patches 1, 5 and 6). If we cant agree on the text markup then drm docs will simply look a bit funny for everyone else. But if the inline struct stuff won't happen 0-day will scream around (and there's already patches which use the new layout). > 2 We're constructing an increasingly complicated document-processing > mechanism with a lot of independently moving parts. What if we > converted the whole document to markdown and dispensed with the XML > part altogether? Making the source files simpler and dispensing with > the xmlto requirement would be a big win, IMO. Who's going to convert the almost 30kloc of xml template (which often have large amounts of texts) and the over 60k kerneldoc comments that we have already? > I will not make #2 be a precondition to getting some form of this work > merged, but I would like to have a good answer for #1. Adding such a > heavyweight dependency (even as an optional one) needs to have a pretty > good story behind it. Should we discuss #1 at ks? Imo as long as no one pipes up to do the massive conversion away from the current toolchain (and subsystem maintainers won't just kill that effort because it causes too much churn) moving forward with the kerneldoc toolchain we have makes sense. Hence I'd like to see those patches landed before we resolve #1 if possible. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] drm/i915: Fix warnings while make xmldocs caused by intel_lrc.c
On Sun, Sep 13, 2015 at 09:08:31PM +0900, Masanari Iida wrote: > This patch fix following warnings while "make xmldocs". > .//drivers/gpu/drm/i915/intel_lrc.c:780: warning: No description > found for parameter 'req' > .//drivers/gpu/drm/i915/intel_lrc.c:780: warning: Excess function > parameter 'request' description in 'intel_logical_ring_begin' > .//drivers/gpu/drm/i915/intel_lrc.c:780: warning: Excess function > parameter 'ctx' description in 'intel_logical_ring_begin' > > Signed-off-by: Masanari Iida <standby2...@gmail.com> Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/intel_lrc.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index 72e0edd..963992a 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -765,8 +765,7 @@ static int logical_ring_prepare(struct > drm_i915_gem_request *req, int bytes) > /** > * intel_logical_ring_begin() - prepare the logical ringbuffer to accept > some commands > * > - * @request: The request to start some new work for > - * @ctx: Logical ring context whose ringbuffer is being prepared. > + * @req: The request to start some new work for > * @num_dwords: number of DWORDs that we plan to write to the ringbuffer. > * > * The ringbuffer might not be ready to accept the commands right away > (maybe it needs to > -- > 2.6.0.rc0.24.gec371ff > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 0/6] scripts/kernel-doc: Kernel-doc improvements
On Sun, Sep 13, 2015 at 9:13 PM, Jonathan Corbet <cor...@lwn.net> wrote: > On Sun, 13 Sep 2015 12:36:07 +0200 > Daniel Vetter <dan...@ffwll.ch> wrote: > >> Personally I don't care which kind of text markup we pick and wich >> converter, as long as the project looks reasonable far away from >> immeninent death (way too many one-person projects on github in this >> area). >> >> But if we have this discussion I'd like to decouple it from the other >> kerneldoc improvemnts in this series (patches 1, 5 and 6). If we cant >> agree on the text markup then drm docs will simply look a bit funny for >> everyone else. But if the inline struct stuff won't happen 0-day will >> scream around (and there's already patches which use the new layout). > > Those patches are: > > 1) scripts/kernel-doc: Replacing highlights hash by an array > 5) scripts/kernel-doc: Improve Markdown results > 6) scripts/kernel-doc: Processing -nofunc for functions only > > #1 is fine, I'll merge that today. #6 is already merged. #5 is a markdown > patch, though, and doesn't make sense without the others? Are you thinking > about #3 (drm/doc: Convert to markdown)? That one would almost work (it > depends on #2 currently) and it nicely shows *why* I'd like to get away from > XML... Sorry I mixed things up, #5 is ok to leave out. I thought about "scripts/kernel-doc Allow struct arguments documentation in struct body" but you already merged that one for 4.2. Which I missed, but which is great since that's the one that would cause the big conflicts. >> > 2 We're constructing an increasingly complicated document-processing >> > mechanism with a lot of independently moving parts. What if we >> > converted the whole document to markdown and dispensed with the XML >> > part altogether? Making the source files simpler and dispensing with >> > the xmlto requirement would be a big win, IMO. >> >> Who's going to convert the almost 30kloc of xml template (which often have >> large amounts of texts) and the over 60k kerneldoc comments that we have >> already? > > I thought you'd do that :) > > Seriously, though, a change would be a big job. There's a reason I've said > several times that it would make no sense to delay the work at hand in the > hopes of somebody doing this whole job instead. But we can certainly > ponder what might be better. > > Getting rid of the XML stuff may well simplify the whole process and make > the documentation much more accessible for those who would change it; that > could be a goal worth going for. Oh, and anything requiring changes to the > kerneldoc comments is going nowhere, that was never something I'd > contemplated. Those comments are fine. Well my goal is to be able to have all the programmer docs in the code, including any high-level overview sections to tie everything together (hence hyperlinks and better formatting). But then you still need some skeleton to make a coherent whole out of all the parts, and I think the docbook xml templates we have serve rather fine for that role. Writing text in xml is indeed horrid, but if we can create in-line DOC: sections with decent formatting there's really no need for that. From that angle this work here already has the goal to get rid of the xml - I plane to move all the existing text in the drm.tmpl xml into inline DOC: kerneldoc commets. And at least gtkdoc (which we use extensively for the userpace test/tools repo) relies on some xml thing to tie different topics together too. So that seems pretty standard. > But any such change needs a lot of thought and a reasonable proof of > concept. Meanwhile we have work that can make the docs better now, and I > want to merge it. But we should choose the tools we use carefully, and I > anticipate a lot of opposition to one that has to drag in 70 Haskell > packages with it. Well personally I don't care about the exact tooling, as long as: - it's a reasonable alive project - packages available on distros (works for me on both debian and fedora) - we can muck around with how things are integrated into overall docs (which is the case since pandoc is only used as a paragraph formatter here). The last one is something we stumbled over for the userspace side where we wanted to have better tooling for documenting testcases. Upstream gtkdoc is receptive, but getting anything in takes forever and then there's the 6 month release schedule too. And there's also the problem that we can't change the kerneldoc grammar (e.g. gtkdoc has the code comment parser integrated and ofc it has slightly different rules to mark up references to structs/enums). I'm also trying to build up a doc team here with a full-time tech writer for drm/i915 topics. So I'm very much interested in k
Re: [PATCH 1/4] drm: Put platform driver registration/unregistration loops in core.
tatic int exynos_drm_register_drivers(struct platform_driver * const *drv,
> - int count)
> -{
> - int i, ret;
> -
> - for (i = 0; i < count; ++i) {
> - ret = platform_driver_register(drv[i]);
> - if (!ret)
> - continue;
> -
> - while (--i >= 0)
> - platform_driver_unregister(drv[i]);
> -
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> static inline int exynos_drm_register_kms_drivers(void)
> {
> - return exynos_drm_register_drivers(exynos_drm_kms_drivers,
> - ARRAY_SIZE(exynos_drm_kms_drivers));
> + return drm_platform_register_drivers(exynos_drm_kms_drivers,
> +
> ARRAY_SIZE(exynos_drm_kms_drivers));
> }
>
> static inline int exynos_drm_register_non_kms_drivers(void)
> {
> - return exynos_drm_register_drivers(exynos_drm_non_kms_drivers,
> -
> ARRAY_SIZE(exynos_drm_non_kms_drivers));
> + return drm_platform_register_drivers(exynos_drm_non_kms_drivers,
> +
> ARRAY_SIZE(exynos_drm_non_kms_drivers));
> }
>
> static inline void exynos_drm_unregister_kms_drivers(void)
> {
> - exynos_drm_unregister_drivers(exynos_drm_kms_drivers,
> + drm_platform_unregister_drivers(exynos_drm_kms_drivers,
> ARRAY_SIZE(exynos_drm_kms_drivers));
> }
>
> static inline void exynos_drm_unregister_non_kms_drivers(void)
> {
> - exynos_drm_unregister_drivers(exynos_drm_non_kms_drivers,
> + drm_platform_unregister_drivers(exynos_drm_non_kms_drivers,
>
> ARRAY_SIZE(exynos_drm_non_kms_drivers));
> }
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 8b5ce7c..a774574 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1087,6 +1087,10 @@ extern int drm_pcie_get_speed_cap_mask(struct
> drm_device *dev, u32 *speed_mask);
> /* platform section */
> extern int drm_platform_init(struct drm_driver *driver, struct
> platform_device *platform_device);
> extern int drm_platform_set_busid(struct drm_device *d, struct drm_master
> *m);
> +int drm_platform_register_drivers(struct platform_driver *const *drv,
> + int count);
> +void drm_platform_unregister_drivers(struct platform_driver *const *drv,
> +int count);
>
> /* returns true if currently okay to sleep */
> static __inline__ bool drm_can_sleep(void)
> --
> 2.1.4
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
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 RFC 0/8] Add New DRM Driver for Hisilicon's Hi6220 SoC
On Wed, Sep 16, 2015 at 04:23:35PM +0100, Daniel Stone wrote: > The biggest issue though, is that this driver should become an atomic > modesetting driver. Atomic modesetting, rather than sending small > individual commands (enable CRTC, change plane position, etc) is based > on validating and passing around complete sets of hardware state. > Daniel Vetter's blog has an article on how to convert your driver: > http://blog.ffwll.ch/2014/11/atomic-modeset-support-for-kms-drivers.html Yeah, any new driver should really be built on top of atomic - it's a lot more flexible than the old thing and it's also what you want long-term. I've also just done a presenation about atomic for drivers: http://people.freedesktop.org/~danvet/presentations/xdc-2015.pdf Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: [Intel-gfx] [PATCH] drm/i915: Fix comparison bug
On Tue, Sep 29, 2015 at 10:37:30AM +0300, Jani Nikula wrote:
> On Tue, 29 Sep 2015, Rasmus Villemoes <li...@rasmusvillemoes.dk> wrote:
> > ->stolen->start has type u64 aka unsigned long long; relying on the
> > difference (effectively cast to int) for sorting is wrong.
> >
> > It wouldn't be a problem in practice if the values compared are always
> > within INT_MAX of each other (so that the difference is actually
> > representable in an int), but 440fd5283a87 ("drm/mm: Support 4 GiB and
> > larger ranges") strongly suggests that's not the case.
> >
> > Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
>
> Reviewed-by: Jani Nikula <jani.nik...@intel.com>
drm_mm is the shared gpu memmory manager, but stolen itself can only go to
about 1.5G currently. So safe, but I merged your patch anyway with a small
note added for 4.4.
Thanks, Daniel
>
>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index e3ec9049081f..5207e681a987 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -258,7 +258,11 @@ static int obj_rank_by_stolen(void *priv,
> > struct drm_i915_gem_object *b =
> > container_of(B, struct drm_i915_gem_object, obj_exec_link);
> >
> > - return a->stolen->start - b->stolen->start;
> > + if (a->stolen->start < b->stolen->start)
> > + return -1;
> > + if (a->stolen->start > b->stolen->start)
> > + return 1;
> > + return 0;
> > }
> >
> > static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
> > --
> > 2.1.3
> >
>
> --
> Jani Nikula, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: No more new fbdev drivers, please
On Tue, Sep 29, 2015 at 12:51:38AM +0200, Noralf Trønnes wrote:
>
> Den 27.09.2015 18:08, skrev Emil Velikov:
> >Hi all,
> >
> >On 27 September 2015 at 14:09, Noralf Trønnes <nor...@tronnes.org> wrote:
> >>Den 24.09.2015 14:27, skrev Tomi Valkeinen:
> >>>Hi all,
> >>>
> >>>fbdev is (more or less) maintained, but it's a deprecated framework. All
> >>>new Linux display drivers should be done on DRM.
> >>>
> >>>So let's not add any more new fbdev drivers.
> >>>
> >>>I will continue to maintain the current fbdev drivers, and I don't mind
> >>>adding some new features to those current drivers, as long as the amount
> >>>of code required to add the features stays sensible.
> >>>
> >>>I see we have three fbdev drivers in staging: xgifb, fbtft and sm750fb,
> >>>and the question is what to do with those.
> >>>
> >>>xgifb was added in 2010, and is still in staging.
> >>>
> >>>fbtft looks like maybe some kind of framework on top of fbdev, with
> >>>fbtft specific subdrivers... I didn't look at it in detail, but my gut
> >>>says "never".
> >>
> >>I have done some work [1] to try and make fbtft look more like the rest
> >>of the kernel (doc [2]), but that work will result in an almost complete
> >>rewrite of fbtft.
> > From a very quick skim fbtft looks pretty much like drm/panel. We
> >presently have 30+ 'simple' dsi panels, plus a bunch of spi ones. Have
> >you had a look at these ?
>
> Thanks, that was useful.
> I can use drm_panel to setup the controller (prepare) and do backlight
> (enable/disable), but I need a way to send framebuffer changes.
> I could do this:
>
> struct tinydrm_panel_funcs {
> int (*update)(struct drm_framebuffer *fb,
> struct drm_gem_cma_object *cma_obj,
> unsigned flags, unsigned color,
> struct drm_clip_rect *clips, unsigned num_clips);
> };
>
> struct tinydrm_panel {
> struct drm_panel panel;
> u32 width;
> u32 height;
> void *dev_private;
>
> const struct tinydrm_panel_funcs *funcs;
> };
I'm not sure whether putting the manual-update stuff into drm_panel is a
good idea - it's transport/bus (spi, dsi, ...) specific. Not sure how to
best solve that.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: No more new fbdev drivers, please
On Mon, Sep 28, 2015 at 01:52:31PM -0700, Bernie Thompson wrote: > On Sat, Sep 26, 2015 at 11:01 AM, Geert Uytterhoeven <ge...@linux-m68k.org> > wrote: > > The smallest of these (udl) still counts in at ca. 2800 LoC, > > Note udlfb.c, the original fbdev driver that I helped write and that the > udl DRM driver was based on, is ~1800 LoC ... so we're actually talking in > the ballpark of 2x (rather than 10x) between fbdev and DRM in this case. > That said, the complexity difference is probably higher than the LoC > difference. I know I personally have struggled in the shift from > understanding fbdev to understanding DRM. udl has a bit of room for improvement, we really should push the worker logicy for fbdev emulation into the core drm fbdev helpers using the ->dirtyfb callback. That should rip out quite a few lines. The other thing to consider is that drm/udl supports PRIME buffer sharing for seamlessly extending your desktop by just plugging in an usb dongle. > The fact that there's drivers of both types and USB hardware might make udl > may be a good driver to use as a base for any additional simplification / > helper work. David Airlie and David Herrmann both have this hardware. David > Airlie did the port from fbdev to DRM, so he's made it an exemplary > driver. And if anyone needs any hardware which works with udlfb and udl, > we're happy to send free hardware to any programmers who are willing to > contribute in the form of code or testing: > http://plugable.com/projects/plugable-open-source-hardware-samples-program For example drivers I think it's better to look at the latest drm driver merged - those are up-to-date wrt best practices. udl has already accumulated a bit of cruft (e.g. still using legacy modeset helpers and not the atomic ones). > More simplification and documentation would be great. In particular, the > optimization for the connector+encoder+crtc combination others have > mentioned seems like it would be worthwhile. Atomic helpers already make almost everything optional except for the crtc-level enable/disable callbacks and the per-plane atomic_plane_update (for buffer flips/panning/rotation/...). So a comibined helper would be mostly for cutting down the structure setup/teardown boilerplate. So should be fairly easy to implement even for drm beginners (when using one of the latest drivers as a template for what needs to be done). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: [Intel-gfx] [PATCH 32/39] drm: i915: drop null test before destroy functions
On Sun, Sep 13, 2015 at 02:15:25PM +0200, Julia Lawall wrote: > Remove unneeded NULL test. > > The semantic patch that makes this change is as follows: > (http://coccinelle.lip6.fr/) > > // > @@ expression x; @@ > -if (x != NULL) > \(kmem_cache_destroy\|mempool_destroy\|dma_pool_destroy\)(x); > // > > Signed-off-by: Julia Lawall <julia.law...@lip6.fr> Queued for -next, thanks for the patch. -Daniel > > --- > drivers/gpu/drm/i915/i915_dma.c | 19 ++- > 1 file changed, 6 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index ab37d11..1dc7c21 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1059,12 +1059,9 @@ out_freecsr: > put_bridge: > pci_dev_put(dev_priv->bridge_dev); > free_priv: > - if (dev_priv->requests) > - kmem_cache_destroy(dev_priv->requests); > - if (dev_priv->vmas) > - kmem_cache_destroy(dev_priv->vmas); > - if (dev_priv->objects) > - kmem_cache_destroy(dev_priv->objects); > + kmem_cache_destroy(dev_priv->requests); > + kmem_cache_destroy(dev_priv->vmas); > + kmem_cache_destroy(dev_priv->objects); > kfree(dev_priv); > return ret; > } > @@ -1150,13 +1147,9 @@ int i915_driver_unload(struct drm_device *dev) > if (dev_priv->regs != NULL) > pci_iounmap(dev->pdev, dev_priv->regs); > > - if (dev_priv->requests) > - kmem_cache_destroy(dev_priv->requests); > - if (dev_priv->vmas) > - kmem_cache_destroy(dev_priv->vmas); > - if (dev_priv->objects) > - kmem_cache_destroy(dev_priv->objects); > - > + kmem_cache_destroy(dev_priv->requests); > + kmem_cache_destroy(dev_priv->vmas); > + kmem_cache_destroy(dev_priv->objects); > pci_dev_put(dev_priv->bridge_dev); > kfree(dev_priv); > > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: [regression] [git pull] drm for 4.3
On Tue, Sep 29, 2015 at 09:07:22PM -0400, da...@codemonkey.org.uk wrote:
> On Thu, Sep 24, 2015 at 04:26:28PM +0300, Jani Nikula wrote:
> > On Thu, 24 Sep 2015, "da...@codemonkey.org.uk" <da...@codemonkey.org.uk>
> wrote:
> > > On Wed, Sep 23, 2015 at 11:07:56AM +, Lankhorst, Maarten wrote:
> > > > Hey,
> > > >
> > > > Dave Jones schreef op di 22-09-2015 om 21:49 [-0400]:
> > > > > On Tue, Sep 22, 2015 at 09:15:58AM -0700, Matt Roper wrote:
> > > > > > On Tue, Sep 22, 2015 at 05:13:55PM +0200, Daniel Vetter wrote:
> > > > > > > On Tue, Sep 22, 2015 at 08:00:17AM -0700, Jesse Barnes wrote:
> > > > > > > > Cc'ing Maarten and Matt; I'm guessing this may be related to
> one of
> > > > > > > > their recent patches.
> > > > > >
> > > > > > Sounds like this showed up before my recent work, but I think I
> might
> > > > > > have seen similar problems while working on atomic watermarks;
> the
> > > > > > issues I was seeing were because the initial hardware readout
> could
> > > > > > leave primary->visible set to true even when the CRTC was off.
> My
> > > > > > series (which is still under development) contains this patch to
> fix
> > > > > > that:
> > > > > >
> > > > > > http://patchwork.freedesktop.org/patch/59564/
> > > > > >
> > > > > > Does applying that help with the problems reported here?
> > > > >
> > > > > No difference at all for me.
> > > > Looks like a (reopened) dup of 91952?
> > > >
> > > > Can you apply "[PATCH] drm/i915: Add primary plane to mask if it's
> > > > visible", and get me the results?
> > >
> > > This doesn't apply on top of Linus' current tree.
> > > If you let me know what it's dependant on, I'll do a build with
> > > those patches tomorrow.
> >
> > It's now part of the drm-intel-fixes pull request [1], maybe it's
> > easiest to pull that in? Just four commits on top of
> > v4.3-rc2. Alternatively pick it up from our repo [2].
>
> The warning on boot seems to be gone as of rc3, but I can now trigger this
> pretty easily..
http://patchwork.freedesktop.org/patch/60618/
Cheers, Daniel
>
> WARNING: CPU: 2 PID: 28911 at drivers/gpu/drm/drm_atomic.c:889
> drm_atomic_get_property+0x244/0x2d0()
> CPU: 2 PID: 28911 Comm: trinity-c313 Not tainted 4.3.0-rc3-think+ #14
> 0379 8801a1377c88 8e35d5ec
> 8801a1377cc0 8e07a862 880500b392b8 880500a13008
> 880500b39290 8804fe3806d8 88003fa45668 8801a1377cd0
> Call Trace:
> [] dump_stack+0x4e/0x82
> [] warn_slowpath_common+0x82/0xc0
> [] warn_slowpath_null+0x1a/0x20
> [] drm_atomic_get_property+0x244/0x2d0
> [] drm_object_property_get_value+0x6c/0x70
> [] dpms_show+0x2f/0x70
> [] dev_attr_show+0x20/0x50
> [] ? sysfs_file_ops+0x41/0x60
> [] sysfs_kf_seq_show+0xb7/0x110
> [] kernfs_seq_show+0x26/0x30
> [] seq_read+0xe6/0x430
> [] kernfs_fop_read+0x127/0x170
> [] ? mutex_lock_nested+0x26b/0x3f0
> [] __vfs_read+0x28/0xe0
> [] ? mutex_lock_nested+0x287/0x3f0
> [] ? __fdget_pos+0x49/0x50
> [] ? __fdget_pos+0x49/0x50
> [] vfs_read+0x86/0x130
> [] SyS_read+0x49/0xb0
> [] entry_SYSCALL_64_fastpath+0x12/0x6f
> ---[ end trace e053063c697a1355 ]---
>
> 887 case DRM_MODE_OBJECT_CONNECTOR: {
> 888 struct drm_connector *connector = obj_to_connector(obj);
> 889
> WARN_ON(!drm_modeset_is_locked(>mode_config.connection_mutex));
> 890 ret = drm_atomic_connector_get_property(connector,
> 891 connector->state, property, val);
> 892 break;
> 893 }
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: linux-next: build failure after merge of the drm-misc tree
On Thu, Oct 01, 2015 at 12:47:17PM +1000, Stephen Rothwell wrote: > Hi all, > > After merging the drm-misc tree, today's linux-next build (x86_64 > allmodconfig) failed like this: > > ERROR: "drm_agp_release" [drivers/gpu/drm/radeon/radeon.ko] undefined! > ERROR: "drm_agp_acquire" [drivers/gpu/drm/radeon/radeon.ko] undefined! > ERROR: "drm_agp_enable" [drivers/gpu/drm/radeon/radeon.ko] undefined! > ERROR: "drm_agp_info" [drivers/gpu/drm/radeon/radeon.ko] undefined! > ERROR: "drm_agp_release" [drivers/gpu/drm/mga/mga.ko] undefined! > ERROR: "drm_agp_acquire" [drivers/gpu/drm/mga/mga.ko] undefined! > ERROR: "drm_agp_free" [drivers/gpu/drm/mga/mga.ko] undefined! > ERROR: "drm_agp_bind" [drivers/gpu/drm/mga/mga.ko] undefined! > ERROR: "drm_agp_unbind" [drivers/gpu/drm/mga/mga.ko] undefined! > ERROR: "drm_agp_alloc" [drivers/gpu/drm/mga/mga.ko] undefined! > ERROR: "drm_agp_enable" [drivers/gpu/drm/mga/mga.ko] undefined! > ERROR: "drm_agp_info" [drivers/gpu/drm/mga/mga.ko] undefined! > ERROR: "drm_agp_unbind_ioctl" [drivers/gpu/drm/drm.ko] undefined! > ERROR: "drm_agp_init" [drivers/gpu/drm/drm.ko] undefined! > ERROR: "drm_agp_alloc_ioctl" [drivers/gpu/drm/drm.ko] undefined! > ERROR: "drm_agp_clear" [drivers/gpu/drm/drm.ko] undefined! > ERROR: "drm_agp_info_ioctl" [drivers/gpu/drm/drm.ko] undefined! > ERROR: "drm_agp_enable_ioctl" [drivers/gpu/drm/drm.ko] undefined! > ERROR: "drm_agp_release_ioctl" [drivers/gpu/drm/drm.ko] undefined! > ERROR: "drm_agp_bind_ioctl" [drivers/gpu/drm/drm.ko] undefined! > ERROR: "drm_agp_acquire_ioctl" [drivers/gpu/drm/drm.ko] undefined! > ERROR: "drm_agp_free_ioctl" [drivers/gpu/drm/drm.ko] undefined! > > Not quite sure which commit caused this, but I have used the drm-misc > tree from next-20150930 for today. Build fail for CONFIG_AGP=m (bot not y or n). I'm wrestling kbuild atm. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] vgaarb: use kzalloc in vga_arbiter_add_pci_device()
On Thu, Oct 01, 2015 at 11:26:40AM +0300, Jani Nikula wrote:
> On Wed, 30 Sep 2015, Rasmus Villemoes <li...@rasmusvillemoes.dk> wrote:
> > Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
>
> Reviewed-by: Jani Nikula <jani.nik...@intel.com>
Applied to drm-misc, thanks.
-Daniel
>
> > ---
> > drivers/gpu/vga/vgaarb.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> > index a0b433456107..3166e4bc4eb6 100644
> > --- a/drivers/gpu/vga/vgaarb.c
> > +++ b/drivers/gpu/vga/vgaarb.c
> > @@ -531,7 +531,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
> > *pdev)
> > return false;
> >
> > /* Allocate structure */
> > - vgadev = kmalloc(sizeof(struct vga_device), GFP_KERNEL);
> > + vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
> > if (vgadev == NULL) {
> > pr_err("failed to allocate pci device\n");
> > /*
> > @@ -541,8 +541,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
> > *pdev)
> > return false;
> > }
> >
> > - memset(vgadev, 0, sizeof(*vgadev));
> > -
> > /* Take lock & check for duplicates */
> > spin_lock_irqsave(_lock, flags);
> > if (vgadev_find(pdev) != NULL) {
> > --
> > 2.1.3
> >
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Jani Nikula, Intel Open Source Technology Center
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: No more new fbdev drivers, please
On Mon, Sep 28, 2015 at 09:39:13AM +0200, Gerd Hoffmann wrote: > Hi, > > > As Daniel mentioned, the connector+encoder+crtc combination is one of > > those simplifications that would make sense if more such drivers are > > added. > > Another one is memory management. It's pretty complex because it can > handle _way_ more than what simple drivers need, and the result is > _alot_ of ttm boilerplate in the drivers. ttm is pretty impressive overkill for most simplistic drm drivers. If you just need contiguous framebuffers for display then the cma helpers should take care of pretty much all the boilerplate for you. They have ready-made simple gem and dumb framebuffer mmap support, which is all a basic kms driver needs. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 1/6] driver-core: platform: Provide helpers for multi-driver modules
upport enables more hands-off userspace setup:
> * (a) environment variable lets new-style hotplug events work once system is
> * fully running: "modprobe $MODALIAS"
> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> index bba08f44cc97..0c9f16bfdd99 100644
> --- a/include/linux/platform_device.h
> +++ b/include/linux/platform_device.h
> @@ -270,6 +270,11 @@ extern struct platform_device *__platform_create_bundle(
> struct resource *res, unsigned int n_res,
> const void *data, size_t size, struct module *module);
>
> +int platform_register_drivers(struct platform_driver * const *drivers,
> + unsigned int count);
> +void platform_unregister_drivers(struct platform_driver * const *drivers,
> +unsigned int count);
> +
> /* early platform driver interface */
> struct early_platform_driver {
> const char *class_str;
> --
> 2.5.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
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: WARNING: CPU: 4 PID: 863 at include/drm/drm_crtc.h:1577 drm_helper_choose_encoder_dpms+0x88/0x90()
On Wed, Sep 23, 2015 at 10:59:51AM +0200, Borislav Petkov wrote:
> On Wed, Sep 23, 2015 at 09:25:23AM +0200, Daniel Vetter wrote:
> > Strange thing is that I've tested this on a radeon over here and I don't
> > see this backtrace ... wut. Below diff should appease the backtraces at
> > least.
>
> Doesn't look like it.
sorry I sprinkled the locking stuff in the wrong places. Still confused
why the resume side doesn't blow up anywhere ... Oh well. New patch below.
Thanks, Daniel
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index d8319dae8358..f3f562f6d848 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1573,10 +1573,12 @@ int radeon_suspend_kms(struct drm_device *dev, bool
suspend, bool fbcon)
drm_kms_helper_poll_disable(dev);
+ drm_modeset_lock_all(dev);
/* turn off display hw */
list_for_each_entry(connector, >mode_config.connector_list, head) {
drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
}
+ drm_modeset_unlock_all(dev);
/* unpin the front buffers and cursors */
list_for_each_entry(crtc, >mode_config.crtc_list, head) {
@@ -1734,9 +1736,11 @@ int radeon_resume_kms(struct drm_device *dev, bool
resume, bool fbcon)
if (fbcon) {
drm_helper_resume_force_mode(dev);
/* turn on display hw */
+ drm_modeset_lock_all(dev);
list_for_each_entry(connector,
>mode_config.connector_list, head) {
drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON);
}
+ drm_modeset_unlock_all(dev);
}
drm_kms_helper_poll_enable(dev);
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: No more new fbdev drivers, please
On Thu, Sep 24, 2015 at 02:46:21PM +0200, Thomas Petazzoni wrote: > Hello, > > On Thu, 24 Sep 2015 15:27:01 +0300, Tomi Valkeinen wrote: > > > fbdev is (more or less) maintained, but it's a deprecated framework. All > > new Linux display drivers should be done on DRM. > > > > So let's not add any more new fbdev drivers. > > > > I will continue to maintain the current fbdev drivers, and I don't mind > > adding some new features to those current drivers, as long as the amount > > of code required to add the features stays sensible. > > > > I see we have three fbdev drivers in staging: xgifb, fbtft and sm750fb, > > and the question is what to do with those. > > > > xgifb was added in 2010, and is still in staging. > > > > fbtft looks like maybe some kind of framework on top of fbdev, with > > fbtft specific subdrivers... I didn't look at it in detail, but my gut > > says "never". > > fbtft mainly drives some very simple I2C-based or SPI-based displays, > and DRM is I believe overkill for such displays. Last time I talked > with Laurent Pinchart about such drivers, I believe he said that such > simple drivers could probably continue to use the fbdev subsystem. > > Or are there some plans to make the writing of DRM drivers for very > simple/trivial devices a bit simpler? Since years I'm trying to sell someone on implementing support for drm_simple_outputs which would collapse the crtc->encoder->connector chain into 1 entity. Would be trivial to implement and then trivial to write simple drivers on top of that. And besides that drm already has piles of reallly simple drivers with just one output and one framebuffer. There's no reason not to use drm for gfx drivers at all. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: No more new fbdev drivers, please
On Thu, Sep 24, 2015 at 11:21:15AM -0400, Austin S Hemmelgarn wrote: > On 2015-09-24 08:46, Thomas Petazzoni wrote: > >Hello, > > > >On Thu, 24 Sep 2015 15:27:01 +0300, Tomi Valkeinen wrote: > > > >>fbdev is (more or less) maintained, but it's a deprecated framework. All > >>new Linux display drivers should be done on DRM. > >> > >>So let's not add any more new fbdev drivers. > >> > >>I will continue to maintain the current fbdev drivers, and I don't mind > >>adding some new features to those current drivers, as long as the amount > >>of code required to add the features stays sensible. > >> > >>I see we have three fbdev drivers in staging: xgifb, fbtft and sm750fb, > >>and the question is what to do with those. > >> > >>xgifb was added in 2010, and is still in staging. > >> > >>fbtft looks like maybe some kind of framework on top of fbdev, with > >>fbtft specific subdrivers... I didn't look at it in detail, but my gut > >>says "never". > > > >fbtft mainly drives some very simple I2C-based or SPI-based displays, > >and DRM is I believe overkill for such displays. Last time I talked > >with Laurent Pinchart about such drivers, I believe he said that such > >simple drivers could probably continue to use the fbdev subsystem. > I have to agree, using DRM _really_ doesn't make sense for these, the > devices in question are (AFAIK) simple I2C or SPI connected frame-buffer > chips that are hooked up to equally simple TFT displays. There's no 3d > acceleration at all from what I can tell, there's _very_ limited 2d > acceleration, and most of the stuff that the DRM framework provides > call-backs for would have to be done on the CPU anyway. On top of that, > it's targeted at small embedded systems with limited memory, and the DRM > framework is by no-means lightweight (TBH, fbdev isn't really either, but > it's much more light weight than DRM). See my other mail, but you can write very simple drm drivers. And if there's really a bloat problem for small systems we can add Kconfig knobs to throw out everything not needed for simple drivers. The only problem really is that everyone with such simple drivers doesn't even consider drm "because I don't have a desktop gpu" which is just silly - drm has become rather flexible. And that's essentially why writing simple drm drivers still has a bit too much boilerplate, since no one yet bothered to add a bit of helper support needed. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] drm: drm_atomic_crtc_get_property should be static
On Thu, Sep 24, 2015 at 03:01:03AM -0700, Geliang Tang wrote:
> Fixes the following sparse warning:
> drivers/gpu/drm/drm_atomic.c:442:5: warning: symbol
> 'drm_atomic_crtc_get_property' was not declared. Should it be static?
>
> Signed-off-by: Geliang Tang <geliangt...@163.com>
Applied to drm-misc, thansk.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 940f80b..7bb3845 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -438,7 +438,8 @@ EXPORT_SYMBOL(drm_atomic_crtc_set_property);
> * consistent behavior you must call this function rather than the
> * driver hook directly.
> */
> -int drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> +static int
> +drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> const struct drm_crtc_state *state,
> struct drm_property *property, uint64_t *val)
> {
> --
> 1.9.1
>
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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 v2 5/6] virtio-gpu: add basic prime support
On Tue, Sep 22, 2015 at 05:24:29PM +0200, Gerd Hoffmann wrote:
> > > +int virtgpu_gem_prime_mmap(struct drm_gem_object *obj,
> > > +struct vm_area_struct *area)
> > > +{
> > > + WARN_ONCE(1, "not implemented");
> > > + return ENOSYS;
> >
> > This can get called by userspace, so please don't WARN here. Also missing
> > negate sign:
> >
> > return -ENOSYS;
>
> Hmm now checkpatch throws a warning at me:
>
>
>WARNING: ENOSYS means 'invalid syscall nr' and nothing else
>#12: FILE: drivers/gpu/drm/virtio/virtgpu_prime.c:70:
>+ return -ENOSYS;
>
>
> I guess I should use something else then (here and elsewhere in the
> file)? Maybe -EINVAL? Other suggestions?
-ENODEV is what we occasionally pick. drm is fairly creative at errno
abuse, e.g. we already use -ENOENT to signal any kind of lookup failure in
ioctls (even if the fd itself is obviously there so not possible that the
fd isn't there).
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: WARNING: CPU: 4 PID: 863 at include/drm/drm_crtc.h:1577 drm_helper_choose_encoder_dpms+0x88/0x90()
Sorry, I can't think of anything off hand. I suspect it was some
> change or cleanup in the core drm code.
The locking check is new, but I was only adding locking checks, not yet
reworking the locking itself. So the backtrace is likely (but not 100%
guaranteed) a red herring.
Strange thing is that I've tested this on a radeon over here and I don't
see this backtrace ... wut. Below diff should appease the backtraces at
least.
-Daniel
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index d8319dae8358..9f05de73ae97 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1734,9 +1734,11 @@ int radeon_resume_kms(struct drm_device *dev, bool
resume, bool fbcon)
if (fbcon) {
drm_helper_resume_force_mode(dev);
/* turn on display hw */
+ drm_modeset_lock_all(dev);
list_for_each_entry(connector,
>mode_config.connector_list, head) {
drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON);
}
+ drm_modeset_unlock_all(dev);
}
drm_kms_helper_poll_enable(dev);
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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/
[regression] Re: [Linux-v4.2-10463-g9a9952bbd76a] i915: WARNING: intel_display.c:1377 assert_planes_disabled
>> [ 20.921344] [] intel_fbdev_initial_config+0x1b/0x20 > >> [i915] > >> [ 20.921346] [] async_run_entry_fn+0x37/0xe0 > >> [ 20.921349] [] process_one_work+0x1e1/0x620 > >> [ 20.921352] [] ? process_one_work+0x156/0x620 > >> [ 20.921353] [] worker_thread+0x69/0x480 > >> [ 20.921355] [] ? cancel_delayed_work_sync+0x20/0x20 > >> [ 20.921357] [] kthread+0x10a/0x120 > >> [ 20.921360] [] ? kthread_create_on_node+0x200/0x200 > >> [ 20.921364] [] ret_from_fork+0x3f/0x70 > >> [ 20.921366] [] ? kthread_create_on_node+0x200/0x200 > >> [ 20.921367] ---[ end trace 85fce5c152ff4528 ]--- > >> > >> Some intelgfx hardware infos (see attached Xorg.0.log)... > >> > >> [27.487] (--) intel(0): Integrated Graphics Chipset: Intel(R) HD > >> Graphics 3000 > >> [27.565] (II) intel(0): SNA initialized with Sandybridge (gen6, gt2) > >> backend > >> > >> Please have a look at it. > >> > >> If this is a known issue, please let me know and kindly point me to a > >> patch (patchwork preferred) or commit-id in drm-intel Git. > >> ( Sorry, I have not checked any mailing-lists or patchwork. ) > >> > >> If you need more infos, logs and/or debug material please let me know > >> and give clear instructions (especially for debugging). > >> > >> > > > > I still see this issue with Linux v4.3-rc1. > > > > Hi Daniel, > > any progress with this reported issue? > > I looked into drm-intel.git#drm-intel-fixes, but see only a kernel-doc fix. > ( I tried also some drm-intel-nightly, not the latest - and still saw > the issue. ) > > This issue is still alive in Linux v4.3-rc2. > > What can I do to help? > > Please, take care of this. > > Thanks!. > > Regards, > - Sedat - -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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/
[regression] Re: Linux 4.3-rc1
Another regression for Jairo to track. -Daniel On Tue, Sep 15, 2015 at 03:26:13AM +0200, Sedat Dilek wrote: > Hi, > > I have reported the same issue in [0]... > > You write in [1]... > > [ cut here ] > WARNING: CPU: 3 PID: 24 at drivers/gpu/drm/i915/intel_display.c:1377 > assert_planes_disabled+0xe4/0x150 [i915]() > plane A assertion failure, should be disabled but not > ... > > I can confirm this was not seen in v4.2 here on Ubuntu/precise AMD64. > > Regards, > - Sedat - > > [0] http://marc.info/?t=14417666342=1=2 > [1] http://marc.info/?l=linux-kernel=144227950327112=2 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: No more new fbdev drivers, please
On Thu, Sep 24, 2015 at 07:12:27PM +0200, Ondrej Zary wrote: > On Thursday 24 September 2015 17:59:12 Daniel Vetter wrote: > > On Thu, Sep 24, 2015 at 11:21:15AM -0400, Austin S Hemmelgarn wrote: > > > On 2015-09-24 08:46, Thomas Petazzoni wrote: > > > >Hello, > > > > > > > >On Thu, 24 Sep 2015 15:27:01 +0300, Tomi Valkeinen wrote: > > > >>fbdev is (more or less) maintained, but it's a deprecated framework. > > > >> All new Linux display drivers should be done on DRM. > > > >> > > > >>So let's not add any more new fbdev drivers. > > > >> > > > >>I will continue to maintain the current fbdev drivers, and I don't mind > > > >>adding some new features to those current drivers, as long as the > > > >> amount of code required to add the features stays sensible. > > > >> > > > >>I see we have three fbdev drivers in staging: xgifb, fbtft and sm750fb, > > > >>and the question is what to do with those. > > > >> > > > >>xgifb was added in 2010, and is still in staging. > > > >> > > > >>fbtft looks like maybe some kind of framework on top of fbdev, with > > > >>fbtft specific subdrivers... I didn't look at it in detail, but my gut > > > >>says "never". > > > > > > > >fbtft mainly drives some very simple I2C-based or SPI-based displays, > > > >and DRM is I believe overkill for such displays. Last time I talked > > > >with Laurent Pinchart about such drivers, I believe he said that such > > > >simple drivers could probably continue to use the fbdev subsystem. > > > > > > I have to agree, using DRM _really_ doesn't make sense for these, the > > > devices in question are (AFAIK) simple I2C or SPI connected frame-buffer > > > chips that are hooked up to equally simple TFT displays. There's no 3d > > > acceleration at all from what I can tell, there's _very_ limited 2d > > > acceleration, and most of the stuff that the DRM framework provides > > > call-backs for would have to be done on the CPU anyway. On top of that, > > > it's targeted at small embedded systems with limited memory, and the DRM > > > framework is by no-means lightweight (TBH, fbdev isn't really either, but > > > it's much more light weight than DRM). > > > > See my other mail, but you can write very simple drm drivers. And if > > there's really a bloat problem for small systems we can add Kconfig knobs > > to throw out everything not needed for simple drivers. The only problem > > really is that everyone with such simple drivers doesn't even consider drm > > "because I don't have a desktop gpu" which is just silly - drm has become > > rather flexible. And that's essentially why writing simple drm drivers > > still has a bit too much boilerplate, since no one yet bothered to add a > > bit of helper support needed. > > Is there a simple way to convert existing fbdev drivers to DRM? Let's say I > want to convert tridentfb to DRM, keeping the 2D acceleration (pan, fillrect, > copyarea, imageblit) to be usable by the console (and maybe extend it to X11 > using some generic 2D driver?) DRM doesn't do generic 2d accel, it's all driver specific. And consensus for 2d accel (at least in X) is pretty much that if you have a 3d gpu use glamour. If you don't have that then use the cpu. There's a hint for drm userspace whether to use shadowfb for cpu rendering or not. What you can do though if you want is keep your accel code for the fbdev emulation on top of the drm modesetting driver, there's a few oddball drivers who do that. And panning is of course already supported by the modeset api. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: No more new fbdev drivers, please
On Fri, Sep 25, 2015 at 04:09:46PM +0300, Tomi Valkeinen wrote: > > > On 25/09/15 13:41, Kamil Lulko wrote: > > Hi, > > > >> fbdev is (more or less) maintained, but it's a deprecated framework. All > >> new Linux display drivers should be done on DRM. > > > > What about no-mmu platforms? DRM has a big fat MMU dependency in the > > kconfig, is there a way to write DRM driver for such devices? > > I guess not. > > Then again, I don't see why DRM would have a hard dependency to MMU, if > the work is done to make DRM work optionally without MMU. How much work > that is, I have no idea. We have plenty drivers in drm without hw mmu, and yeah there's probably no reason at all why the drm subsystem has a hard depency on cpu MMUs. Might be some #ifdef fallout that needs to be done, but there shouldn't be anything fundamental. Maybe the old dri1 days code has something, but that's all historical cruft anyway. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 RFC 0/8] Add New DRM Driver for Hisilicon's Hi6220 SoC
On Thu, Sep 17, 2015 at 05:51:28PM +0800, Xinliang Liu wrote: > On 17 September 2015 at 04:16, Daniel Vetter <dan...@ffwll.ch> wrote: > > > On Wed, Sep 16, 2015 at 04:23:35PM +0100, Daniel Stone wrote: > > > The biggest issue though, is that this driver should become an atomic > > > modesetting driver. Atomic modesetting, rather than sending small > > > individual commands (enable CRTC, change plane position, etc) is based > > > on validating and passing around complete sets of hardware state. > > > Daniel Vetter's blog has an article on how to convert your driver: > > > http://blog.ffwll.ch/2014/11/atomic-modeset-support-for-kms-drivers.html > > > > Yeah, any new driver should really be built on top of atomic - it's a lot > > more flexible than the old thing and it's also what you want long-term. > > > > I've also just done a presenation about atomic for drivers: > > > > http://people.freedesktop.org/~danvet/presentations/xdc-2015.pdf > > > Hi Daniel, > This is a good presentation. It gives us very detail and good suggection on > implementation. > Thank you for sharing this. > > We have a open source KMS hwc: > wiki: > https://wiki.linaro.org/BenjaminGaignard/HWComposer_DRM?highlight=%28hwcomposer%2 > source code: git://git.linaro.org/people/benjamin.gaignard/hwcomposer.git > Now only STI and Hikey boards are tested on it. And atomic mode setting is > not support now. > I think we should add support for atomic mode setting next. > > One difficulty I am facing is that one setting should be made sure is ok in > the prepare function of hwc. > If not, the set function of hwc may be fail and display will not properly. > I don't know atomic mode setting how to handle this situation. And it seems > that in the prepare function, > it should check the hardware's capabilities, such as clip, scale, rotation, > blending and so on. Specifically to support things like hwc's ->prepare callback atomic supports a TEST_ONLY mode. Hence in your the prepare code build up the atomic state, but set the TEST_ONLY flag when calling the ioctl. When the kernel is happy you can store it somewhere and tell surface flinger the configuration so it can render the remaining buffers with GL. The idea is that generic userspace does use TEST_ONLY mode iterative to build up the maximal configuration of hw planes that a given kms driver supports, leaving no hw-specific code in userspace. For optimized hwc drivers running on atomic you then just need to tune the selection, but detailed constraint checking would still be done by the kernel. The weston patches from Daniel Stone have a similar design, would be worth checking those out. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 2/3] drm: fix kernel-doc warnings in drm_crtc.h
On Wed, Sep 16, 2015 at 11:25:22PM -0700, Geliang Tang wrote:
> Fix the following 'make htmldocs' warning:
>
> .//include/drm/drm_crtc.h:929: warning: Excess struct/union/enum/typedef
> member 'base' description in 'drm_bridge'
>
> Signed-off-by: Geliang Tang <geliangt...@163.com>
Applied to drm-misc, thanks.
-Daniel
> ---
> include/drm/drm_crtc.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c0366e9..6566f72 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -911,7 +911,6 @@ struct drm_bridge_funcs {
> * @next: the next bridge in the encoder chain
> * @of_node: device node pointer to the bridge
> * @list: to keep track of all added bridges
> - * @base: base mode object
> * @funcs: control functions
> * @driver_private: pointer to the bridge driver's internal context
> */
> --
> 1.9.1
>
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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 03/38] drm/i915: fix handling gen8_emit_flush_coherentl3_wa result
On Mon, Sep 21, 2015 at 04:59:58PM +0300, Jani Nikula wrote: > On Mon, 21 Sep 2015, Andrzej Hajda <a.ha...@samsung.com> wrote: > > The function can return negative value. > > > > The problem has been detected using proposed semantic patch > > scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1]. > > > > [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576 > > > > Signed-off-by: Andrzej Hajda <a.ha...@samsung.com> > > Reviewed-by: Jani Nikula <jani.nik...@intel.com> Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 v2 5/6] virtio-gpu: add basic prime support
The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Authors: Andreas Pokorny
> + */
> +
> +#include "virtgpu_drv.h"
> +
> +/* Empty Implementations as there should not be any other driver for a
> virtual
> + * device that might share buffers with virtgpu */
> +
> +int virtgpu_gem_prime_pin(struct drm_gem_object *obj)
> +{
> + WARN_ONCE(1, "not implemented");
> + return -ENOSYS;
> +}
> +
> +void virtgpu_gem_prime_unpin(struct drm_gem_object *obj)
> +{
> + WARN_ONCE(1, "not implemented");
> +}
> +
> +
> +struct sg_table *virtgpu_gem_prime_get_sg_table(struct drm_gem_object *obj)
> +{
> + WARN_ONCE(1, "not implemented");
> + return ERR_PTR(-ENOSYS);
> +}
> +
> +struct drm_gem_object *virtgpu_gem_prime_import_sg_table(
> + struct drm_device *dev, struct dma_buf_attachment *attach,
> + struct sg_table *table)
> +{
> + WARN_ONCE(1, "not implemented");
> + return ERR_PTR(-ENOSYS);
> +}
> +
> +void *virtgpu_gem_prime_vmap(struct drm_gem_object *obj)
> +{
> + WARN_ONCE(1, "not implemented");
> + return ERR_PTR(-ENOSYS);
> +}
> +
> +void virtgpu_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
> +{
> + WARN_ONCE(1, "not implemented");
> +}
> +
> +int virtgpu_gem_prime_mmap(struct drm_gem_object *obj,
> +struct vm_area_struct *area)
> +{
> + WARN_ONCE(1, "not implemented");
> + return ENOSYS;
This can get called by userspace, so please don't WARN here. Also missing
negate sign:
return -ENOSYS;
Cheers, Daniel
> +}
> --
> 1.8.3.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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/
[regression] [git pull] drm for 4.3
On Tue, Sep 22, 2015 at 08:00:17AM -0700, Jesse Barnes wrote: > Cc'ing Maarten and Matt; I'm guessing this may be related to one of > their recent patches. Adding Jairo to track this regression. -Daniel > > Jesse > > On 09/21/2015 11:48 AM, Dave Jones wrote: > > On Mon, Sep 07, 2015 at 02:45:59PM -0400, Dave Jones wrote: > > > On Fri, Sep 04, 2015 at 11:40:53PM +0100, Dave Airlie wrote: > > > > > > > > Hi Linus, > > > > > > > > This is the main pull request for the drm for 4.3. Nouveau is > > probably the biggest > > > > amount of changes in here, since it missed 4.2. Highlights below, > > along with the usual > > > > bunch of fixes. There are a few minor conflicts with your tree but > > nothing > > > > you can't handle. All stuff outside drm should have applicable acks. > > > > > > > > Highlights: > > > > > > > > ... > > > > i915: > > > > Skylake support enabled by default > > > > legacy modesetting using atomic infrastructure > > > > Skylake fixes > > > > GEN9 workarounds > > > > > > Since this merge, I'm seeing this twice during boot.. > > > > And still there in -rc2. Several other people reported this too, > > and they also got no reponse. > > > > I'll start bisecting when I get home tonight. It shouldn't be too hard, > > as 4.2 was fine. > > > > Dave > > > > > [ cut here ] > > > WARNING: CPU: 0 PID: 6 at drivers/gpu/drm/i915/intel_display.c:1377 > > assert_planes_disabled+0xdf/0x140() > > > plane A assertion failure, should be disabled but not > > > CPU: 0 PID: 6 Comm: kworker/u8:0 Not tainted 4.2.0-think+ #9 > > > Workqueue: events_unbound async_run_entry_fn > > > 0561 88050392b6f8 8d7dccce 88050392b740 > > > 88050392b730 8d079ee2 880500a6 > > > 8805008e99c8 88050392b790 > > > Call Trace: > > > [] dump_stack+0x4e/0x79 > > > [] warn_slowpath_common+0x82/0xc0 > > > [] warn_slowpath_fmt+0x4c/0x50 > > > [] assert_planes_disabled+0xdf/0x140 > > > [] intel_disable_pipe+0x4b/0x2c0 > > > [] haswell_crtc_disable+0x8a/0x2e0 > > > [] intel_atomic_commit+0xff/0x1320 > > > [] ? drm_atomic_check_only+0x21e/0x550 > > > [] drm_atomic_commit+0x37/0x60 > > > [] drm_atomic_helper_set_config+0x1c5/0x430 > > > [] drm_mode_set_config_internal+0x65/0x110 > > > [] restore_fbdev_mode+0xbe/0xe0 > > > [] drm_fb_helper_restore_fbdev_mode_unlocked+0x25/0x70 > > > [] drm_fb_helper_set_par+0x2d/0x50 > > > [] intel_fbdev_set_par+0x1a/0x60 > > > [] fbcon_init+0x545/0x5d0 > > > [] visual_init+0xca/0x130 > > > [] do_bind_con_driver+0x1c5/0x3b0 > > > [] do_take_over_console+0x149/0x1a0 > > > [] do_fbcon_takeover+0x57/0xb0 > > > [] fbcon_event_notify+0x66c/0x760 > > > [] notifier_call_chain+0x3e/0xb0 > > > [] __blocking_notifier_call_chain+0x4d/0x70 > > > [] blocking_notifier_call_chain+0x16/0x20 > > > [] fb_notifier_call_chain+0x1b/0x20 > > > [] register_framebuffer+0x1e7/0x300 > > > [] drm_fb_helper_initial_config+0x252/0x3e0 > > > [] intel_fbdev_initial_config+0x1b/0x20 > > > [] async_run_entry_fn+0x4a/0x140 > > > [] process_one_work+0x1fd/0x670 > > > [] ? process_one_work+0x16c/0x670 > > > [] worker_thread+0x4e/0x450 > > > [] ? process_one_work+0x670/0x670 > > > [] kthread+0x101/0x120 > > > [] ? kthread_create_on_node+0x250/0x250 > > > [] ret_from_fork+0x3f/0x70 > > > [] ? kthread_create_on_node+0x250/0x250 > > > ---[ end trace 54cab2e0c772d5d9 ]--- > > > > > > > > > 00:02.0 VGA compatible controller: Intel Corporation Xeon E3-1200 v3 > > Processor Integrated Graphics Controller (rev 06) > > > > ___ > > Intel-gfx mailing list > > intel-...@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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/
Shared scaffolding to unit-test kernel code in userspace (was Re: [Intel-gfx] [PATCH] drm/i915: Add link training test)
On Wed, Sep 23, 2015 at 11:18 AM, Ander Conselvan De Oliveira <conselv...@gmail.com> wrote: > On Wed, 2015-09-23 at 10:24 +0200, Daniel Vetter wrote: >> On Tue, Sep 15, 2015 at 04:08:53PM +0300, Ander Conselvan De Oliveira wrote: >> > On Mon, 2015-09-14 at 16:38 +0300, Ander Conselvan De Oliveira wrote: >> > > On Mon, 2015-09-14 at 15:11 +0200, Daniel Vetter wrote: >> > > > On Mon, Sep 14, 2015 at 02:51:51PM +0300, Ander Conselvan de Oliveira >> > > > wrote: >> > > > > --- >> > > > > >> > > > > On Fri, 2015-09-11 at 17:11 +0300, Ander Conselvan de Oliveira wrote: >> > > > > > On Wed, 2015-09-09 at 11:33 +0100, Thomas Wood wrote: >> > > > > > > On 8 September 2015 at 13:28, Ander Conselvan de Oliveira >> > > > > > > <ander.conselvan.de.olive...@intel.com> wrote: >> > > > > > > > >> > > > > > >> > > > > > > > diff --git a/link-training-test/Makefile >> > > > > > > > b/link-training-test/Makefile >> > > > > > > >> > > > > > > If this is meant to be part of the test suite, then it needs to >> > > > > > > be in >> > > > > > > the tests directory and use the igt test infrastructure. >> > > > > > > Otherwise it >> > > > > > > should be placed in tools or tools/link-training-test. >> > > > > > >> > > > > > I made the test use the igt infrastructure, but I'm not sure if >> > > > > > this is >> > > > > > a good fit for it. The dependency on the kernel is on build time, >> > > > > > but >> > > > > > once compiled this can be run on any machine. This can also >> > > > > > introduce >> > > > > > build failures if the test is not kept in sync with the driver >> > > > > > source. >> > > > > > Ideally that a failure to build this would be reported as the test >> > > > > > failing, but I have no idea of how to achieve that. >> > > > > >> > > > > Alternatively, this could be in the kernel source tree directly. This >> > > > > patch adds a test subdir to the i915 source dir, containing the link >> > > > > training test. The test is compiled as part of the normal build using >> > > > > the extra-y variable so that it doesn't get linked to the final >> > > > > kernel. >> > > > > >> > > > > When make is run from the tests directory, a thin wrapper around the >> > > > > tests is built and linked to the object file compiled as part of the >> > > > > kernel build. Running make run_tests from the test dir runs the test >> > > > > and reports success or failure. >> > > > > >> > > > > Any thoughts? >> > > > >> > > > I think there's some precedence in other subsystems to integrate unit >> > > > tests directly in the kernel, e.g. locking selftest or similar things. >> > > > Usual approach is to either have a special module (but that often means >> > > > piles of EXPORT_SYMBOL only for that selftest module). Or just a y/n >> > > > Kconfig option which enables that code and runs all the self/unit tests >> > > > when the module loads. >> > > > >> > > > I'd go with that approach since it's simpler. And we'd only need to >> > > > tell >> > > > QA to enable that Kconfig option for more testing. >> > > >> > > I'll have a look into that Kconfig approach, but there's a couple of >> > > things >> > > I like about having the unit test as user space binaries: >> > > >> > > - there's no need to boot the newly compiled kernel, so doing a test >> > > run >> > > is super fast; >> > > - the binaries can be debugged with gdb just like other user space >> > > stuff. >> > >> > I implemented the test using the Kconfig approach, and it seems to work >> > well >> > without impacting the points above. I added the call to run the test as the >> > first thing in i915_init(), and with the driver compiled built-in, running >> > the kernel under qemu will run the tests. And qemu can also provide a gdb >> &g
Re: PROBLEM: Intel VGA output busticated on 4.3-rc2 (regression)
Another regression for Jairo to track. -Daniel On Thu, Sep 24, 2015 at 05:22:09PM -0400, Nick Bowler wrote: > Hi, > > Testing out 4.3-rc2, first thing I notice is that the VGA output is > not working. Specifically, the display is continuously powering on > and off -- at no point is any image visible on the screen (I am expecting > to see the console output). The display connected to the HDMI output is > working fine. > > Linux 4.2 did not suffer from this problem. > > In dmesg I see the following messages, which I do not see on a working > kernel. Full dmesg from 4.3-rc2 is attached (gzipped). > > [0.115339] [drm:drm_calc_timestamping_constants] *ERROR* crtc > 21: Can't calculate constants, dotclock = 0! > [0.117582] [drm:intel_opregion_init] *ERROR* No ACPI video bus found > > This is an older machine with Intel G45 graphics. > > Unfortunately bisection is proving difficult, because the commits it > wants me to test have a different problem: neither display comes up at > all (both remain in standby). Partial results follow. > > Thanks, > Nick > > git bisect start 'drivers/gpu/drm/i915' > # bad: [1f93e4a96c9109378204c147b3eec0d0e8100fde] Linux 4.3-rc2 > git bisect bad 1f93e4a96c9109378204c147b3eec0d0e8100fde > # good: [64291f7db5bd8150a74ad2036f1037e6a0428df2] Linux 4.2 > git bisect good 64291f7db5bd8150a74ad2036f1037e6a0428df2 > # skip: [a7a6c498927ea42c9a3b26e0caa5c854a980d58c] drm/i915: > POSTING_READ() in intel_set_memory_cxsr() > git bisect skip a7a6c498927ea42c9a3b26e0caa5c854a980d58c > # skip: [031b698a77a70a6c394568034437b5486a44e868] drm/i915: > Unconditionally do fb tracking invalidate in set_domain > git bisect skip 031b698a77a70a6c394568034437b5486a44e868 > # skip: [adeca76d8e2b34b5c739a36f4191aed63080da40] drm/i915: > Simplify i915_gem_execbuffer_retire_commands() parameters > git bisect skip adeca76d8e2b34b5c739a36f4191aed63080da40 > # good: [369712e89404089fa559235bb1ee8fc40d976e6b] drm/i915: reduce > duplicate conditions in i9xx_hpd_irq_handler > git bisect good 369712e89404089fa559235bb1ee8fc40d976e6b > # skip: [6eb1a6817246f1a67de4d6959a84d09efead5329] drm/i915: Read wm > values from hardware at init on CHV > git bisect skip 6eb1a6817246f1a67de4d6959a84d09efead5329 > # bad: [d14e7b6d1d8747826cb900db852351c550e00fdd] drm/i915: Check DP > link status on long hpd too > git bisect bad d14e7b6d1d8747826cb900db852351c550e00fdd > # skip: [fe36f55d4d4447679923fc74564786ae423ca4bd] drm/i915/gtt: > Cleanup page directory encoding > git bisect skip fe36f55d4d4447679923fc74564786ae423ca4bd > _______ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] drm/i915: fix task reference leak in i915_debugfs.c
On Fri, Sep 25, 2015 at 02:16:07PM +0300, Jani Nikula wrote:
> On Fri, 25 Sep 2015, Geliang Tang <geliangt...@163.com> wrote:
> > Leak a task reference in i915_ppgtt_info(), add put_task_struct()
> > to fix it.
>
> Introduced by
>
> commit 1c60fef535d143860d5bf6593e24ab6417f5227c
> Author: Ben Widawsky <b...@bwidawsk.net>
> Date: Fri Dec 6 14:11:30 2013 -0800
>
> drm/i915: Dump all ppgtt
Queued for -next, thanks for the patch.
-Daniel
>
>
> >
> > Signed-off-by: Geliang Tang <geliangt...@163.com>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 5615d3d..8797717 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2288,9 +2288,13 @@ static int i915_ppgtt_info(struct seq_file *m, void
> > *data)
> >
> > list_for_each_entry_reverse(file, >filelist, lhead) {
> > struct drm_i915_file_private *file_priv = file->driver_priv;
> > + struct task_struct *task;
> >
> > - seq_printf(m, "\nproc: %s\n",
> > - get_pid_task(file->pid, PIDTYPE_PID)->comm);
> > + task = get_pid_task(file->pid, PIDTYPE_PID);
> > + if (!task)
> > + return -ESRCH;
> > + seq_printf(m, "\nproc: %s\n", task->comm);
> > + put_task_struct(task);
> > idr_for_each(_priv->context_idr, per_file_ctx,
> > (void *)(unsigned long)m);
> > }
> > --
> > 1.9.1
> >
> >
>
> --
> Jani Nikula, Intel Open Source Technology Center
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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] staging/android: add TODO to de-stage android sync framework
On Mon, Dec 07, 2015 at 04:28:45PM -0200, Gustavo Padovan wrote: > Hi, > > any comments/update on this? Thanks My ack from the previous version still stands. -Daniel > > Gustavo > > 2015-11-26 Gustavo Padovan <gust...@padovan.org>: > > > From: Gustavo Padovan <gustavo.pado...@collabora.co.uk> > > > > - remove CONFIG_SW_SYNC_USER, it is used only for testing/debugging and > >should not be upstreamed. > > - port CONFIG_SW_SYNC_USER tests interfaces to use debugfs somehow > > - port libsync tests to kselftest > > - clean up and ABI check for security issues > > - move the sync framework to drivers/base/dma-buf > > > > Cc: Arve Hjønnevåg <a...@android.com> > > Cc: Riley Andrews <riandr...@android.com> > > Cc: Daniel Vetter <daniel.vet...@ffwll.ch> > > Cc: Rob Clark <robdcl...@gmail.com> > > Cc: Greg Hackmann <ghackm...@google.com> > > Cc: John Harrison <john.c.harri...@intel.com> > > Signed-off-by: Gustavo Padovan <gustavo.pado...@collabora.co.uk> > > --- > > drivers/staging/android/TODO | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO > > index 8f3ac37..64d8c87 100644 > > --- a/drivers/staging/android/TODO > > +++ b/drivers/staging/android/TODO > > @@ -25,5 +25,13 @@ ion/ > > exposes existing cma regions and doesn't reserve unecessarily memory > > when > > booting a system which doesn't use ion. > > > > +sync framework: > > + - remove CONFIG_SW_SYNC_USER, it is used only for testing/debugging and > > + should not be upstreamed. > > + - port CONFIG_SW_SYNC_USER tests interfaces to use debugfs somehow > > + - port libsync tests to kselftest > > + - clean up and ABI check for security issues > > + - move it to drivers/base/dma-buf > > + > > Please send patches to Greg Kroah-Hartman <g...@kroah.com> and Cc: > > Arve Hjønnevåg <a...@android.com> and Riley Andrews <riandr...@android.com> > > -- > > 2.1.0 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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.
vice *drm = dev_get_drvdata(dev);
> + struct hdlcd_drm_private *hdlcd = drm->dev_private;
> +
> + if (hdlcd->fbdev) {
> + drm_fbdev_cma_fini(hdlcd->fbdev);
> + hdlcd->fbdev = NULL;
> + }
> + drm_kms_helper_poll_fini(drm);
> + component_unbind_all(dev, drm);
> + drm_vblank_cleanup(drm);
> + pm_runtime_get_sync(drm->dev);
> + drm_irq_uninstall(drm);
> + pm_runtime_put_sync(drm->dev);
> + pm_runtime_disable(drm->dev);
> + of_reserved_mem_device_release(drm->dev);
> + if (!IS_ERR(hdlcd->clk)) {
> + devm_clk_put(drm->dev, hdlcd->clk);
> + hdlcd->clk = NULL;
> + }
> + drm_mode_config_cleanup(drm);
> + drm_dev_unregister(drm);
> + drm_dev_unref(drm);
> + drm->dev_private = NULL;
> + dev_set_drvdata(dev, NULL);
> +}
> +
> +static const struct component_master_ops hdlcd_master_ops = {
> + .add_components = hdlcd_add_components,
> + .bind = hdlcd_drm_bind,
> + .unbind = hdlcd_drm_unbind,
> +};
> +
> +static int hdlcd_probe(struct platform_device *pdev)
> +{
> + return component_master_add(>dev, _master_ops);
> +}
> +
> +static int hdlcd_remove(struct platform_device *pdev)
> +{
> + component_master_del(>dev, _master_ops);
> + return 0;
> +}
> +
> +static const struct of_device_id hdlcd_of_match[] = {
> + { .compatible = "arm,hdlcd" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, hdlcd_of_match);
> +
> +static int hdlcd_pm_suspend(struct device *dev)
> +{
> + struct drm_device *drm = dev_get_drvdata(dev);
> + struct drm_crtc *crtc;
> +
> + if (pm_runtime_suspended(dev))
> + return 0;
> +
> + drm_modeset_lock_all(drm);
> + list_for_each_entry(crtc, >mode_config.crtc_list, head)
> + hdlcd_crtc_suspend(crtc);
> + drm_modeset_unlock_all(drm);
> + return 0;
> +}
> +
> +static int hdlcd_pm_resume(struct device *dev)
> +{
> + struct drm_device *drm = dev_get_drvdata(dev);
> + struct drm_crtc *crtc;
> +
> + if (!pm_runtime_suspended(dev))
> + return 0;
> +
> + drm_modeset_lock_all(drm);
> + list_for_each_entry(crtc, >mode_config.crtc_list, head)
> + hdlcd_crtc_resume(crtc);
> + drm_modeset_unlock_all(drm);
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(hdlcd_pm_ops, hdlcd_pm_suspend, hdlcd_pm_resume);
> +
> +static struct platform_driver hdlcd_platform_driver = {
> + .probe = hdlcd_probe,
> + .remove = hdlcd_remove,
> + .driver = {
> + .name = "hdlcd",
> + .pm = _pm_ops,
> + .of_match_table = hdlcd_of_match,
> + },
> +};
> +
> +module_platform_driver(hdlcd_platform_driver);
> +
> +MODULE_AUTHOR("Liviu Dudau");
> +MODULE_DESCRIPTION("ARM HDLCD DRM driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.h b/drivers/gpu/drm/arm/hdlcd_drv.h
> new file mode 100644
> index 000..f2eedfb
> --- /dev/null
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.h
> @@ -0,0 +1,42 @@
> +/*
> + * ARM HDLCD Controller register definition
> + */
> +
> +#ifndef __HDLCD_DRV_H__
> +#define __HDLCD_DRV_H__
> +
> +struct hdlcd_drm_private {
> + void __iomem*mmio;
> + struct clk *clk;
> + struct drm_fbdev_cma*fbdev;
> + struct drm_framebuffer *fb;
> + struct drm_pending_vblank_event *event;
> + struct drm_crtc crtc;
> + struct drm_plane*plane;
> +#ifdef CONFIG_DEBUG_FS
> + atomic_t buffer_underrun_count;
> + atomic_t bus_error_count;
> + atomic_t vsync_count;
> + atomic_t dma_end_count;
> +#endif
> +};
> +
> +#define crtc_to_hdlcd_priv(x)container_of(x, struct
> hdlcd_drm_private, crtc)
> +
> +static inline void hdlcd_write(struct hdlcd_drm_private *hdlcd,
> +unsigned int reg, u32 value)
> +{
> + writel(value, hdlcd->mmio + reg);
> +}
> +
> +static inline u32 hdlcd_read(struct hdlcd_drm_private *hdlcd, unsigned int
> reg)
> +{
> + return readl(hdlcd->mmio + reg);
> +}
> +
> +int hdlcd_setup_crtc(struct drm_device *dev);
> +void hdlcd_set_scanout(struct hdlcd_drm_private *hdlcd);
> +void hdlcd_crtc_suspend(struct drm_crtc *crtc);
> +void hdlcd_crtc_resume(struct drm_crtc *crtc);
> +
> +#endif /* __HDLCD_DRV_H__ */
> diff --git a/drivers/gpu/drm/arm/hdlcd_regs.h
> b/drivers/gpu/dr
Re: [PATCH v3 2/4] drm: Add support for ARM's HDLCD controller.
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 <airl...@linux.ie> > > Cc: Robin Murphy <robin.mur...@arm.com> > > > > Signed-off-by: Liviu Dudau <liviu.du...@arm.com> > > A few small nitpicks below. I've forgotten to add Acked-by: Daniel Vetter <daniel.vet...@ffwll.ch> 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 <liviu.du...@arm.com> > > + * > >
Re: [PATCH 1/9] drm: Create a driver hook for allocating GEM object structs.
On Tue, Dec 01, 2015 at 12:35:50PM -0800, Eric Anholt wrote:
> The CMA helpers had no way for a driver to extend the struct with its
> own fields. Since the CMA helpers are mostly "Allocate a
> drm_gem_cma_object, then fill in a few fields", it's hard to write as
> pure helpers without passing in a driver callback for the allocate
> step.
>
> Signed-off-by: Eric Anholt <e...@anholt.net>
Yeah there's no other way to make this happen without ridiculous amounts
of boilerplate.
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> ---
> drivers/gpu/drm/drm_gem_cma_helper.c | 10 ++
> include/drm/drmP.h | 7 +++
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c
> index e109b49..0f7b00ba 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -59,11 +59,13 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size)
> struct drm_gem_object *gem_obj;
> int ret;
>
> - cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
> - if (!cma_obj)
> + if (drm->driver->gem_create_object)
> + gem_obj = drm->driver->gem_create_object(drm, size);
> + else
> + gem_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
> + if (!gem_obj)
> return ERR_PTR(-ENOMEM);
> -
> - gem_obj = _obj->base;
> + cma_obj = container_of(gem_obj, struct drm_gem_cma_object, base);
>
> ret = drm_gem_object_init(drm, gem_obj, size);
> if (ret)
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 0b921ae..22ff162 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -580,6 +580,13 @@ struct drm_driver {
> int (*gem_open_object) (struct drm_gem_object *, struct drm_file *);
> void (*gem_close_object) (struct drm_gem_object *, struct drm_file *);
>
> + /**
> + * Hook for allocating the GEM object struct, for use by core
> + * helpers.
> + */
> + struct drm_gem_object *(*gem_create_object)(struct drm_device *dev,
> + size_t size);
> +
> /* prime: */
> /* export handle -> fd (see drm_gem_prime_handle_to_fd() helper) */
> int (*prime_handle_to_fd)(struct drm_device *dev, struct drm_file
> *file_priv,
> --
> 2.6.2
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: [RFC PATCH 7/9] drm/rockchip: force enable vop when do mode setting
On Wed, Dec 02, 2015 at 05:55:36PM +0100, Thierry Reding wrote: > On Tue, Dec 01, 2015 at 11:32:01AM +0800, Mark Yao wrote: > > When do mode setting, mean that we want to enable display output, > > but sometimes, vop_crtc_enable is after mode_set, we can't allow > > that, so force enable vop in mode setting. > > > > Signed-off-by: Mark Yao <mark@rock-chips.com> > > --- > > drivers/gpu/drm/rockchip/rockchip_drm_vop.c |1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > index c65b454..7c07537 100644 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > @@ -1110,6 +1110,7 @@ static void vop_crtc_mode_set_nofb(struct drm_crtc > > *crtc) > > u16 vact_end = vact_st + vdisplay; > > uint32_t val; > > > > + vop_crtc_enable(crtc); > > /* > > * If dclk rate is zero, mean that scanout is stop, > > * we don't need wait any more. > > Have you considered simply moving everything into ->enable()? That's > what I did for Tegra, for much the same reasons that you gave in the > commit message. Doing so gives you a much simpler call graph. Really > the only thing you need to do is move around the code, and perhaps a > different way to get ahold of the display mode, but you can use the > Tegra driver as a reference for how to do that. Yeah if writing mode related registers requires the thing to be on on your hw then you can't use the ->mode_set hooks. Those are explicitly called when everything is off (not just sometimes, at least with atomic helpers). Like Thierry said the recommendation is to just shovel that code into ->enable hooks. ->mode_set_nofb is mostly there to support easier transition for drivers which started with the legacy crtc helpers. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 9/9] drm/vc4: Add an interface for capturing the GPU state after a hang.
On Wed, Dec 02, 2015 at 11:35:16AM -0800, Eric Anholt wrote: > Emil Velikov <emil.l.veli...@gmail.com> writes: > > > On 1 December 2015 at 20:35, Eric Anholt <e...@anholt.net> wrote: > >> This can be parsed with vc4-gpu-tools tools for trying to figure out > >> what was going on. > >> > > I might be pushing my luck here ... have you thought about basing > > (forking) vc4-gpu-tools of intel-gpu-tools ? I'd imagine that the > > macros and helpers will come in handy, despite that some are quite > > intel specific. > > > > On a related note - with the above project in place I'd imagine you > > have (re)considered about having libdrm-vc4 ? Copying hunks around > > might lead to interesting moments (as you have already noticed :-P) > > > > On that note I'll stop now with beating the libdrm drum :-) > > The headers and code that I copy between the two userspace locations > will go in libdrm when I have a kernel ABI, but vc4_drm.h can't go in > until merging to the kernel, and there's not a whole lot of point > without that. > > Yes, I have thought about basing vc4-gpu-tools off of intel-gpu-tools. > I've actually tried to build and use the kms testing stuff on vc4, and > it was a total bust. Someone needs to do a lot of work to make igt > useful for non-intel. If you'd like me to build my vc4 testing inside > of igt, I'd someone to demo one of my tests building inside of igt, with > the test runner working and none of the intel-specific tests reporting > failure, and get me permission to just push code to that repository > (It's hard enough getting piglit tests reviewed, vc4-specific tests and > tools would never get review). Daniel Stone claimed that this Just Works but evidently it doesn't. There's some autoconfig fail where igt wants too much intel crap that just doesn't build on arm. Iirc Daniel had some patches floating around for that. Wrt building tests itself I think we should require libdrm_$vendor everywhere, since without that you can embedded a vendor specific testcase into a shared one (e.g. to test tiled pageflipping). And if that's not possible then either we'll end up in #ifdef hell or code sharing is really hard, rendering a shared testsuite pretty useless. Wrt commit access I'm ok with that, atm rule for igt is to send to intel-gfx and push after 24h of no complaints. For purely vc4-specific stuff we should probably create a vc4 subdir. Wrt tests failing: I sometimes run the entire suite here on a radeon, and at least in the past that resulted in a perfect 100% skip score. So should work, too. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] drm: do not use device name as a format string
On Sat, Dec 05, 2015 at 10:45:50AM +0100, Nicolas Iooss wrote: > Hello, > I sent the path below a few weeks ago and did not have any feedback. > Is there any issue in it that I need to fix before submitting it again? Sorry, must have missed this. > > Thanks, > Nicolas Iooss > > On 11/18/2015 06:58 PM, Nicolas Iooss wrote: > > drm_dev_set_unique() formats its parameter using kvasprintf() but many > > of its callers directly pass dev_name(dev) as printf format string, > > without any format parameter. This can cause some issues when the > > device name contains '%' characters. > > > > To avoid any potential issue, always use "%s" when using > > drm_dev_set_unique() with dev_name(). Not sure this is worth it really, normally people don't place % characters into their device names, ever. And if they do it'll blow up. There's also no security issue here since userspace can't set this name. If the maintainers of the affected drivers don't want this I won't merge this patch. Thanks, Daniel > > > > Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org> > > --- > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +- > > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c| 2 +- > > drivers/gpu/drm/tegra/drm.c | 2 +- > > drivers/gpu/drm/vc4/vc4_drv.c| 2 +- > > include/drm/drmP.h | 1 + > > 5 files changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > > b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > > index 244df0a440b7..0d720d3a7ee0 100644 > > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c > > @@ -733,7 +733,7 @@ static int atmel_hlcdc_dc_drm_probe(struct > > platform_device *pdev) > > if (!ddev) > > return -ENOMEM; > > > > - ret = drm_dev_set_unique(ddev, dev_name(ddev->dev)); > > + ret = drm_dev_set_unique(ddev, "%s", dev_name(ddev->dev)); > > if (ret) > > goto err_unref; > > > > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > > b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > > index 1930234ba5f1..947d75f59881 100644 > > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > > @@ -363,7 +363,7 @@ static int fsl_dcu_drm_probe(struct platform_device > > *pdev) > > fsl_dev->np = dev->of_node; > > drm->dev_private = fsl_dev; > > dev_set_drvdata(dev, fsl_dev); > > - drm_dev_set_unique(drm, dev_name(dev)); > > + drm_dev_set_unique(drm, "%s", dev_name(dev)); > > > > ret = drm_dev_register(drm, 0); > > if (ret < 0) > > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c > > index 159ef515cab1..b278f60f4376 100644 > > --- a/drivers/gpu/drm/tegra/drm.c > > +++ b/drivers/gpu/drm/tegra/drm.c > > @@ -991,7 +991,7 @@ static int host1x_drm_probe(struct host1x_device *dev) > > if (!drm) > > return -ENOMEM; > > > > - drm_dev_set_unique(drm, dev_name(>dev)); > > + drm_dev_set_unique(drm, "%s", dev_name(>dev)); > > dev_set_drvdata(>dev, drm); > > > > err = drm_dev_register(drm, 0); > > diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c > > index 6e730605edcc..c90a451aaa79 100644 > > --- a/drivers/gpu/drm/vc4/vc4_drv.c > > +++ b/drivers/gpu/drm/vc4/vc4_drv.c > > @@ -168,7 +168,7 @@ static int vc4_drm_bind(struct device *dev) > > vc4->dev = drm; > > drm->dev_private = vc4; > > > > - drm_dev_set_unique(drm, dev_name(dev)); > > + drm_dev_set_unique(drm, "%s", dev_name(dev)); > > > > drm_mode_config_init(drm); > > if (ret) > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > > index 0b921ae06cd8..995fb96cb740 100644 > > --- a/include/drm/drmP.h > > +++ b/include/drm/drmP.h > > @@ -1049,6 +1049,7 @@ void drm_dev_ref(struct drm_device *dev); > > void drm_dev_unref(struct drm_device *dev); > > int drm_dev_register(struct drm_device *dev, unsigned long flags); > > void drm_dev_unregister(struct drm_device *dev); > > +__printf(2, 3) > > int drm_dev_set_unique(struct drm_device *dev, const char *fmt, ...); > > > > struct drm_minor *drm_minor_acquire(unsigned int minor_id); > > > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] drm: do not use device name as a format string
On Sun, Dec 06, 2015 at 11:16:32AM +0100, Nicolas Iooss wrote:
> On 12/06/2015 10:35 AM, Daniel Vetter wrote:
> >> On 11/18/2015 06:58 PM, Nicolas Iooss wrote:
> >>> drm_dev_set_unique() formats its parameter using kvasprintf() but many
> >>> of its callers directly pass dev_name(dev) as printf format string,
> >>> without any format parameter. This can cause some issues when the
> >>> device name contains '%' characters.
> >>>
> >>> To avoid any potential issue, always use "%s" when using
> >>> drm_dev_set_unique() with dev_name().
> >
> > Not sure this is worth it really, normally people don't place % characters
> > into their device names, ever. And if they do it'll blow up. There's also
> > no security issue here since userspace can't set this name.
> >
> > If the maintainers of the affected drivers don't want this I won't merge
> > this patch.
>
> Actually I had the same opinion before I began to add __printf
> attributes and "%s" in several places in the kernel to make
> -Wformat-security useful. This led me to discover some funny issues
> like the one fixed by commit 3958b79266b1 ("configfs: fix kernel
> infoleak through user-controlled format string",
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3958b79266b14729edd61daf9dfb84de45f4ec6d
> ). The patch I sent is in fact a very small step towards making
> -Wformat-security useful again to detect "real" issues.
>
> Of course, if you do not feel it is worth it and believe that dev_name
> is fully controlled by trusted sources which will never introduce any %
> character, I understand your will of not merging my patch.
Ah, that's the context I was missing, that really should be in the commit
message. If this is part of an overall effort to enable something useful
it makes more sense to get it in.
On the patch itself it feels rather funny to do a "%s", str); combo, maybe
we should have a drm_dev_set_unique_static instead? Including kerneldoc
explaining why there's too.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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 03/10] i915: Replace "hweight8(dev_priv->info.subslice_7eu[i]) != 1" with "!is_power_of_2(dev_priv->info.subslice_7eu[i])"
On Sun, Dec 06, 2015 at 06:26:30PM +0800, Zhaoxiu Zeng wrote: > From: Zeng Zhaoxiu <zhaoxiu.z...@gmail.com> > > Signed-off-by: Zeng Zhaoxiu <zhaoxiu.z...@gmail.com> Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 9461a23..16c8cf1 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -27,6 +27,7 @@ > * > */ > > +#include > #include > #include "i915_drv.h" > #include > @@ -1000,7 +1001,7 @@ static int skl_tune_iz_hashing(struct intel_engine_cs > *ring) >* Only consider slices where one, and only one, subslice has 7 >* EUs >*/ > - if (hweight8(dev_priv->info.subslice_7eu[i]) != 1) > + if (!is_power_of_2(dev_priv->info.subslice_7eu[i])) > continue; > > /* > -- > 2.5.0 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] drm: do not use device name as a format string
On Mon, Dec 07, 2015 at 11:53:01AM +0200, Jani Nikula wrote:
> On Mon, 07 Dec 2015, Daniel Vetter <dan...@ffwll.ch> wrote:
> > On Sun, Dec 06, 2015 at 11:16:32AM +0100, Nicolas Iooss wrote:
> >> On 12/06/2015 10:35 AM, Daniel Vetter wrote:
> >> >> On 11/18/2015 06:58 PM, Nicolas Iooss wrote:
> >> >>> drm_dev_set_unique() formats its parameter using kvasprintf() but many
> >> >>> of its callers directly pass dev_name(dev) as printf format string,
> >> >>> without any format parameter. This can cause some issues when the
> >> >>> device name contains '%' characters.
> >> >>>
> >> >>> To avoid any potential issue, always use "%s" when using
> >> >>> drm_dev_set_unique() with dev_name().
> >> >
> >> > Not sure this is worth it really, normally people don't place %
> >> > characters
> >> > into their device names, ever. And if they do it'll blow up. There's also
> >> > no security issue here since userspace can't set this name.
> >> >
> >> > If the maintainers of the affected drivers don't want this I won't merge
> >> > this patch.
> >>
> >> Actually I had the same opinion before I began to add __printf
> >> attributes and "%s" in several places in the kernel to make
> >> -Wformat-security useful. This led me to discover some funny issues
> >> like the one fixed by commit 3958b79266b1 ("configfs: fix kernel
> >> infoleak through user-controlled format string",
> >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3958b79266b14729edd61daf9dfb84de45f4ec6d
> >> ). The patch I sent is in fact a very small step towards making
> >> -Wformat-security useful again to detect "real" issues.
> >>
> >> Of course, if you do not feel it is worth it and believe that dev_name
> >> is fully controlled by trusted sources which will never introduce any %
> >> character, I understand your will of not merging my patch.
> >
> > Ah, that's the context I was missing, that really should be in the commit
> > message. If this is part of an overall effort to enable something useful
> > it makes more sense to get it in.
> >
> > On the patch itself it feels rather funny to do a "%s", str); combo, maybe
> > we should have a drm_dev_set_unique_static instead? Including kerneldoc
> > explaining why there's too.
>
> No caller of drm_dev_set_unique() actually uses the formatting for
> anything... so you'd end up with drm_dev_set_unique_static() and an
> orphaned drm_dev_set_unique()...
Ok, then I guess we can just ditch the printf stuff from set_unique.
Nicolas, you're up for that?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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] drm/i915: constify intel_dvo_dev_ops structures
On Tue, Dec 08, 2015 at 09:55:27PM +0100, Julia Lawall wrote:
> The intel_dvo_dev_ops structures are never modified, so declare them as
> const.
>
> Done with the help of Coccinelle.
>
> Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
Queued for -next, thanks for the patch.
-Daniel
>
> ---
> drivers/gpu/drm/i915/dvo.h| 12 ++--
> drivers/gpu/drm/i915/dvo_ch7017.c |2 +-
> drivers/gpu/drm/i915/dvo_ch7xxx.c |2 +-
> drivers/gpu/drm/i915/dvo_ivch.c |2 +-
> drivers/gpu/drm/i915/dvo_ns2501.c |2 +-
> drivers/gpu/drm/i915/dvo_sil164.c |2 +-
> drivers/gpu/drm/i915/dvo_tfp410.c |2 +-
> 7 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/dvo.h b/drivers/gpu/drm/i915/dvo.h
> index 13dea42..5e6a301 100644
> --- a/drivers/gpu/drm/i915/dvo.h
> +++ b/drivers/gpu/drm/i915/dvo.h
> @@ -129,11 +129,11 @@ struct intel_dvo_dev_ops {
> void (*dump_regs)(struct intel_dvo_device *dvo);
> };
>
> -extern struct intel_dvo_dev_ops sil164_ops;
> -extern struct intel_dvo_dev_ops ch7xxx_ops;
> -extern struct intel_dvo_dev_ops ivch_ops;
> -extern struct intel_dvo_dev_ops tfp410_ops;
> -extern struct intel_dvo_dev_ops ch7017_ops;
> -extern struct intel_dvo_dev_ops ns2501_ops;
> +extern const struct intel_dvo_dev_ops sil164_ops;
> +extern const struct intel_dvo_dev_ops ch7xxx_ops;
> +extern const struct intel_dvo_dev_ops ivch_ops;
> +extern const struct intel_dvo_dev_ops tfp410_ops;
> +extern const struct intel_dvo_dev_ops ch7017_ops;
> +extern const struct intel_dvo_dev_ops ns2501_ops;
>
> #endif /* _INTEL_DVO_H */
> diff --git a/drivers/gpu/drm/i915/dvo_ch7017.c
> b/drivers/gpu/drm/i915/dvo_ch7017.c
> index cbb2202..b3c7c19 100644
> --- a/drivers/gpu/drm/i915/dvo_ch7017.c
> +++ b/drivers/gpu/drm/i915/dvo_ch7017.c
> @@ -402,7 +402,7 @@ static void ch7017_destroy(struct intel_dvo_device *dvo)
> }
> }
>
> -struct intel_dvo_dev_ops ch7017_ops = {
> +const struct intel_dvo_dev_ops ch7017_ops = {
> .init = ch7017_init,
> .detect = ch7017_detect,
> .mode_valid = ch7017_mode_valid,
> diff --git a/drivers/gpu/drm/i915/dvo_ch7xxx.c
> b/drivers/gpu/drm/i915/dvo_ch7xxx.c
> index 4b4acc1..44b3159 100644
> --- a/drivers/gpu/drm/i915/dvo_ch7xxx.c
> +++ b/drivers/gpu/drm/i915/dvo_ch7xxx.c
> @@ -356,7 +356,7 @@ static void ch7xxx_destroy(struct intel_dvo_device *dvo)
> }
> }
>
> -struct intel_dvo_dev_ops ch7xxx_ops = {
> +const struct intel_dvo_dev_ops ch7xxx_ops = {
> .init = ch7xxx_init,
> .detect = ch7xxx_detect,
> .mode_valid = ch7xxx_mode_valid,
> diff --git a/drivers/gpu/drm/i915/dvo_ivch.c b/drivers/gpu/drm/i915/dvo_ivch.c
> index ff9f1b0..4950b82 100644
> --- a/drivers/gpu/drm/i915/dvo_ivch.c
> +++ b/drivers/gpu/drm/i915/dvo_ivch.c
> @@ -490,7 +490,7 @@ static void ivch_destroy(struct intel_dvo_device *dvo)
> }
> }
>
> -struct intel_dvo_dev_ops ivch_ops = {
> +const struct intel_dvo_dev_ops ivch_ops = {
> .init = ivch_init,
> .dpms = ivch_dpms,
> .get_hw_state = ivch_get_hw_state,
> diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c
> b/drivers/gpu/drm/i915/dvo_ns2501.c
> index 063859f..2379c33 100644
> --- a/drivers/gpu/drm/i915/dvo_ns2501.c
> +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
> @@ -698,7 +698,7 @@ static void ns2501_destroy(struct intel_dvo_device *dvo)
> }
> }
>
> -struct intel_dvo_dev_ops ns2501_ops = {
> +const struct intel_dvo_dev_ops ns2501_ops = {
> .init = ns2501_init,
> .detect = ns2501_detect,
> .mode_valid = ns2501_mode_valid,
> diff --git a/drivers/gpu/drm/i915/dvo_sil164.c
> b/drivers/gpu/drm/i915/dvo_sil164.c
> index 26f13eb..1c1a067 100644
> --- a/drivers/gpu/drm/i915/dvo_sil164.c
> +++ b/drivers/gpu/drm/i915/dvo_sil164.c
> @@ -267,7 +267,7 @@ static void sil164_destroy(struct intel_dvo_device *dvo)
> }
> }
>
> -struct intel_dvo_dev_ops sil164_ops = {
> +const struct intel_dvo_dev_ops sil164_ops = {
> .init = sil164_init,
> .detect = sil164_detect,
> .mode_valid = sil164_mode_valid,
> diff --git a/drivers/gpu/drm/i915/dvo_tfp410.c
> b/drivers/gpu/drm/i915/dvo_tfp410.c
> index 6f1a0a6..31e181d 100644
> --- a/drivers/gpu/drm/i915/dvo_tfp410.c
> +++ b/drivers/gpu/drm/i915/dvo_tfp410.c
> @@ -306,7 +306,7 @@ static void tfp410_destroy(struct intel_dvo_device *dvo)
> }
> }
>
> -struct intel_dvo_dev_ops tfp410_ops = {
> +const struct intel_dvo_dev_ops tfp410_ops = {
> .init = tfp410_init,
> .detect = tfp410_detect,
> .mode_valid = tfp410_mode_valid,
>
> ___
Re: [PATCH RESEND 2/3] drm/i915: check for return value
On Wed, Dec 09, 2015 at 05:09:12PM +0530, Sudip Mukherjee wrote: > On Thu, Oct 08, 2015 at 04:29:31PM +0200, Daniel Vetter wrote: > > On Thu, Oct 08, 2015 at 07:28:00PM +0530, Sudip Mukherjee wrote: > > > We were not checking the return value of drm_encoder_init() which can > > > fail. And if it fails then we will be working with an uninitialized > > > encoder. > > > > > > Cc: Daniel Vetter <daniel.vet...@intel.com> > > > Cc: Jani Nikula <jani.nik...@linux.intel.com> > > > Signed-off-by: Sudip Mukherjee <su...@vectorindia.org> > > > > Queued for -next, thanks for the patch. > > -Daniel > > Hi Daniel, > It is still not there in linux-next. Still applies cleanly on > next-20151209. Sorry, this must have fallen through the cracks somehow. Applied now for real. Thanks, Daniel > > regards > sudip > > > > > > --- > > > > > > Sent on 27/07/2015 > > > > > > drivers/gpu/drm/i915/intel_dp.c | 6 -- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > b/drivers/gpu/drm/i915/intel_dp.c > > > index 18bcfbe..2a8b47e 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -6174,8 +6174,9 @@ intel_dp_init(struct drm_device *dev, int > > > output_reg, enum port port) > > > intel_encoder = _dig_port->base; > > > encoder = _encoder->base; > > > > > > - drm_encoder_init(dev, _encoder->base, _dp_enc_funcs, > > > - DRM_MODE_ENCODER_TMDS); > > > + if (drm_encoder_init(dev, _encoder->base, _dp_enc_funcs, > > > + DRM_MODE_ENCODER_TMDS)) > > > + goto err_encoder_init; > > > > > > intel_encoder->compute_config = intel_dp_compute_config; > > > intel_encoder->disable = intel_disable_dp; > > > @@ -6224,6 +6225,7 @@ intel_dp_init(struct drm_device *dev, int > > > output_reg, enum port port) > > > > > > err_init_connector: > > > drm_encoder_cleanup(encoder); > > > +err_encoder_init: > > > kfree(intel_connector); > > > err_connector_alloc: > > > kfree(intel_dig_port); > > > -- > > > 1.9.1 > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: [git pull] drm for 4.4-rc1
On Thu, Dec 10, 2015 at 04:04:20AM +0100, Stefan Lippers-Hollmann wrote: > Hi > > On 2015-11-09, Dave Airlie wrote: > [...] > The following changes since commit 06d1ee32a4d25356a710b49d5e95dbdd68bdf505: > > Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux > (2015-10-13 09:45:21 -0700) > > are available in the git repository at: > > git://people.freedesktop.org/~airlied/linux drm-next > > for you to fetch changes up to 816d2206f0f9953ca854e4ff1a2749a5cbd62715: > > Merge tag 'drm-intel-next-fixes-2015-11-06' of > git://anongit.freedesktop.org/drm-intel into drm-next (2015-11-07 17:16:59 > +1000) > [...] > > drm/i915: Check live status before reading edid > [...] > > This patch seems to introduce a regression for i915 in Linus' > v4.4-rc4-60-g9a0f76f, relative to v4.3 (and 4.3.1), on a sandy-bridge > (Intel DH67CL) system with a single HDMI connected monitor (Medion MD20094) > attached. Immediately after the modeswitch, the monitor switches off and > stays off. Nothing catches my eyes in dmesg or Xorg.0.log, but bisecting > the issue points me at: > > 237ed86c693d8a8e4db476976aeb30df4deac74b is the first bad commit > commit 237ed86c693d8a8e4db476976aeb30df4deac74b > Author: Sonika Jindal <sonika.jin...@intel.com> > Date: Tue Sep 15 09:44:20 2015 +0530 > > drm/i915: Check live status before reading edid > > The Bspec is very clear that Live status must be checked about before > trying to read EDID over DDC channel. This patch makes sure that HDMI > EDID is read only when live status is up. > > The live status doesn't seem to perform very consistent across various > platforms when tested with different monitors. The reason behind that is > some monitors are late to provide right voltage to set live_status up. > So, after getting the interrupt, for a small duration, live status reg > fluctuates, and then settles down showing the correct staus. > > This is explained here in, in a rough way: > HPD line > |\ T1 = Monitor Hotplug causing IRQ > | \__ > | | > | | > | | T2 = Live status is stable > | | _ > | | /| > Live status _|_|/ | > | | | > | | | > | | | > T0 T1 T2 > > (Between T1 and T2 Live status fluctuates or can be even low, depending on > the monitor) > > After several experiments, we have concluded that a max delay > of 30ms is enough to allow the live status to settle down with > most of the monitors. This total delay of 30ms has been split into > a resolution of 3 retries of 10ms each, for the better cases. > > This delay is kept at 30ms, keeping in consideration that, HDCP compliance > expect the HPD handler to respond a plug out in 100ms, by disabling port. > > v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions > to check digital port status. Adding a separate function to get bxt live > status (Daniel) > v3: Using intel_encoder->hpd_pin to check the live status (Siva) > Moving the live status read to intel_hdmi_probe and passing parameter > to read/not to read the edid. (me) > v4: > * Added live status check for all platforms using > intel_digital_port_connected. > * Rebased on top of Jani's DP cleanup series > * Some monitors take time in setting the live status. So retry for few > times if this is a connect HPD > v5: Removed extra "drm/i915" from commit message. Adding Shashank's sob > which was missed. > v6: Drop the (!detect_edid && !live_status check) check because for DDI > ports which are enumerated as hdmi as well as DP, we don't have a > mechanism to differentiate between DP and hdmi inside the encoder's > hot_plug. This leads to call to the hdmi's hot_plug hook for DP as well > as hdmi which leads to issues during unplug because of the above check. > v7: Make intel_digital_port_connected global in this patch, some > reformatting of while loop, adding a print when live status is not > up. (Rodrigo) > v8: Rebase it on nightly which involved skipping the hot_plug hook for now > and letting the live_status check happen in detect until the hpd handling > part is finalized (Daniel) > > Signed-off-by:
Re: [PATCH v2 1/1] drm: modes: fix DRM modes analysis regression
On Wed, Dec 09, 2015 at 08:11:39PM +0100, Corentin LABBE wrote:
> Le 09/12/2015 16:32, Jani Nikula a écrit :
> > On Wed, 09 Dec 2015, LABBE Corentin <clabbe.montj...@gmail.com> wrote:
> >> My latest commit introduce some case where a valid mode, could be
> >> rejected.
> >> simple_strtox functions stop at first non-digit character, but kstrtox not.
> >> So args like "video=HDMI-A-1:720x480-16@60" will be reject when checking
> >> 16@.
> >> The proper solution is to store digits in a specific buffer.
> >
> > Or to revert regressing commit...? Your original commit complicated the
> > already complicated function, and this one makes it more so. What is the
> > benefit?
> >
>
> The benefit is to remove a function marked obsolete who do not said if the
> conversation is successful or not.
> But I understand that the way I have done it was not the best one.
> If the maintainer want it, I will send a patch for reverting the first patch.
Given that nothin bad happens if we misparse the number (black screen) and
there's no screen resolution even close to the limit of what can be
parsed, I think reverting is the right action.
Carefully parsing userspace input makes sense. But this is the kernel
cmdline, if your attacker can change that you've lost no matter what.
Please send in the revert with a short summary of the discussion here.
Thanks, Daniel
>
> >> Fixes: 52157a4ca396 ("drm: modes: replace simple_strtoul by kstrtouint")
> >
> > For me the commit id is cc344980c76748e57c9c03100c2a14d36ab00334.
>
> Oups, I took commit id from my local git.
>
> Regards
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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] drm: modes: Revert cc344980c767 "replace simple_strtoul by kstrtouint"
On Fri, Dec 11, 2015 at 01:58:59PM +0100, LABBE Corentin wrote:
> My latest commit introduce some case where a valid mode, could be
> rejected.
> simple_strtox functions stop at first non-digit character, but kstrtox
> not.
> So args like "video=HDMI-A-1:720x480-16@60" will be reject when checking
> 16@.
>
> Discussions about this change comes to the conclusion that the best
> solution is to revert my commit cc344980c76748e57c9c03100c2a14d36ab00334.
>
> Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/drm_modes.c | 16 +---
> 1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index db36af7..3da1434 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1230,7 +1230,7 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
> unsigned int xres = 0, yres = 0, bpp = 32, refresh = 0;
> bool yres_specified = false, cvt = false, rb = false;
> bool interlace = false, margins = false, was_digit = false;
> - int i, err;
> + int i;
> enum drm_connector_force force = DRM_FORCE_UNSPECIFIED;
>
> #ifdef CONFIG_FB
> @@ -1250,9 +1250,7 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
> case '@':
> if (!refresh_specified && !bpp_specified &&
> !yres_specified && !cvt && !rb && was_digit) {
> - err = kstrtouint([i + 1], 10, );
> - if (err)
> - return false;
> + refresh = simple_strtol([i+1], NULL, 10);
> refresh_specified = true;
> was_digit = false;
> } else
> @@ -1261,9 +1259,7 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
> case '-':
> if (!bpp_specified && !yres_specified && !cvt &&
> !rb && was_digit) {
> - err = kstrtouint([i + 1], 10, );
> - if (err)
> - return false;
> + bpp = simple_strtol([i+1], NULL, 10);
> bpp_specified = true;
> was_digit = false;
> } else
> @@ -1271,9 +1267,7 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
> break;
> case 'x':
> if (!yres_specified && was_digit) {
> - err = kstrtouint([i + 1], 10, );
> - if (err)
> - return false;
> + yres = simple_strtol([i+1], NULL, 10);
> yres_specified = true;
> was_digit = false;
> } else
> @@ -1496,4 +1490,4 @@ int drm_mode_convert_umode(struct drm_display_mode *out,
>
> out:
> return ret;
> -}
> +}
> \ No newline at end of file
> --
> 2.4.10
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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] android: fix warning when releasing active sync point
ignaling)(struct fence *fence); > bool (*signaled)(struct fence *fence); > signed long (*wait)(struct fence *fence, bool intr, signed long > timeout); > void (*release)(struct fence *fence); > -- > 2.6.0.rc2.230.g3dd15c0 > > > -- > Dmitry > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] drm: Use the driver's gem_object_free function from CMA helpers.
On Mon, Dec 14, 2015 at 04:26:26PM -0800, Eric Anholt wrote:
> VC4 wraps the CMA objects in its own structures, so it needs to do its
> own teardown (waiting for GPU to finish, updating bo_stats tracking).
> The other CMA drivers are using drm_gem_cma_free_object as their
> gem_free_object, so this should be a no-op for them.
>
> Signed-off-by: Eric Anholt <e...@anholt.net>
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Since vc4 landed already I'll pull this into drm-misc.
-Daniel
> ---
> drivers/gpu/drm/drm_fb_cma_helper.c | 6 +++---
> drivers/gpu/drm/drm_gem_cma_helper.c | 4 ++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 5c1aca4..99b5673 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -279,7 +279,7 @@ static int drm_fbdev_cma_create(struct drm_fb_helper
> *helper,
> if (!fbi) {
> dev_err(dev->dev, "Failed to allocate framebuffer info.\n");
> ret = -ENOMEM;
> - goto err_drm_gem_cma_free_object;
> + goto err_gem_free_object;
> }
>
> fbdev_cma->fb = drm_fb_cma_alloc(dev, _cmd, , 1);
> @@ -322,8 +322,8 @@ err_drm_fb_cma_destroy:
> drm_fb_cma_destroy(fb);
> err_framebuffer_release:
> framebuffer_release(fbi);
> -err_drm_gem_cma_free_object:
> - drm_gem_cma_free_object(>base);
> +err_gem_free_object:
> + dev->driver->gem_free_object(>base);
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 7dcb43f..b409123 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -121,7 +121,7 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct
> drm_device *drm,
> return cma_obj;
>
> error:
> - drm_gem_cma_free_object(_obj->base);
> + drm->driver->gem_free_object(_obj->base);
> return ERR_PTR(ret);
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_create);
> @@ -171,7 +171,7 @@ drm_gem_cma_create_with_handle(struct drm_file *file_priv,
> return cma_obj;
>
> err_handle_create:
> - drm_gem_cma_free_object(gem_obj);
> + drm->driver->gem_free_object(gem_obj);
>
> return ERR_PTR(ret);
> }
> --
> 2.6.2
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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 0/8] drm/rockchip: covert to support atomic API
On Thu, Dec 17, 2015 at 01:03:53PM +0100, Heiko Stübner wrote: > Hi Mark, > > Am Donnerstag, 17. Dezember 2015, 18:32:31 schrieb Mark yao: > > On 2015年12月17日 18:00, Heiko Stübner wrote: > > > Am Donnerstag, 17. Dezember 2015, 11:01:07 schrieb Mark Yao: > > >> The series of patches coverting drm rockchip to atomic API, do some > > >> cleanup and some fixes on atomic side. > > [...] > > > > Plugging in HDMI after boot: > > > > > > [ 20.559534] [ cut here ] > > > [ 20.564412] WARNING: CPU: 3 PID: 74 at > > > drivers/gpu/drm/drm_atomic_helper.c:682 > > > drm_atomic_helper_update_legacy_modeset_state+0x6c/0x200() [ 20.576991] > > > Modules linked in: > > > [ 20.580175] CPU: 3 PID: 74 Comm: irq/51-ff98 Not tainted 4.4.0-rc5+ > > > #2787 [ 20.587502] Hardware name: Rockchip (Device Tree) > > > [ 20.592347] [] (unwind_backtrace) from [] > > > (show_stack+0x20/0x24) [ 20.600319] [] (show_stack) from > > > [] (dump_stack+0x84/0xb8) [ 20.607776] [] > > > (dump_stack) from [] (warn_slowpath_common+0x98/0xc4) [ > > > 20.616087] [] (warn_slowpath_common) from [] > > > (warn_slowpath_null+0x2c/0x34) [ 20.625106] [] > > > (warn_slowpath_null) from [] > > > (drm_atomic_helper_update_legacy_modeset_state+0x6c/0x200) [ 20.636382] > > > [] (drm_atomic_helper_update_legacy_modeset_state) from > > > [] (drm_atomic_helper_commit_modeset_disables+0x1e4/0x35c) [ > > > 20.649732] [] (drm_atomic_helper_commit_modeset_disables) from > > > [] (rockchip_atomic_commit_complete+0x34/0x108) [ 20.661789] > > > [] (rockchip_atomic_commit_complete) from [] > > > (rockchip_drm_atomic_commit+0x84/0xa0) [ 20.672442] [] > > > (rockchip_drm_atomic_commit) from [] > > > (drm_atomic_commit+0x60/0x70) [ 20.681711] [] > > > (drm_atomic_commit) from [] (restore_fbdev_mode+0x128/0x258) [ > > > 20.690418] [] (restore_fbdev_mode) from [] > > > (drm_fb_helper_restore_fbdev_mode_unlocked+0x3c/0x80) [ 20.701033] > > > [] (drm_fb_helper_restore_fbdev_mode_unlocked) from > > > [] (drm_fb_helper_set_par+0x50/0x60) [ 20.711907] > > > [] (drm_fb_helper_set_par) from [] > > > (drm_fb_helper_hotplug_event+0xc8/0xe0) [ 20.721569] [] > > > (drm_fb_helper_hotplug_event) from [] > > > (rockchip_drm_output_poll_changed+0x24/0x28) [ 20.732185] [] > > > (rockchip_drm_output_poll_changed) from [] > > > (drm_kms_helper_hotplug_event+0x34/0x38) [ 20.742891] [] > > > (drm_kms_helper_hotplug_event) from [] > > > (drm_helper_hpd_irq_event+0x120/0x150) [ 20.753078] [] > > > (drm_helper_hpd_irq_event) from [] (dw_hdmi_irq+0x14c/0x18c) [ > > > 20.761772] [] (dw_hdmi_irq) from [] > > > (irq_thread_fn+0x2c/0x44) [ 20.769350] [] (irq_thread_fn) > > > from [] (irq_thread+0x118/0x21c) [ 20.777019] [] > > > (irq_thread) from [] (kthread+0xf4/0x10c) [ 20.784083] > > > [] (kthread) from [] (ret_from_fork+0x14/0x24) [ > > > 20.791300] ---[ end trace 3deaa51d288067a0 ]--- > > > > Hi Heiko > > This issue can be explained from following thread: > > https://lkml.org/lkml/2015/11/16/498 > > > > You can fix it by following changes: > > > > @@ -1667,8 +1667,6 @@ static int dw_hdmi_register(struct drm_device > > *drm, struct dw_hdmi *hdmi) > > _hdmi_connector_funcs, > > DRM_MODE_CONNECTOR_HDMIA); > > > > - hdmi->connector.encoder = encoder; > > yep that fixes the warning. From the thread you mention, it looks like that > assignment should also be removed in the non-atomic case (i.MX)? > > Or if it is really necessary there could also wrap it in a conditional to > only > be set for the non-atomic case. It wont work with non-atomic, but it'll only break things from the 2nd modeset onwards. Someone needs to fix that bug, since afaiu the code it'd blow up for non-atomic if you remove it on even the first modeset. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 v2 2/2] drm: use dev_name as default unique name in drm_dev_alloc()
host1x_device *dev) > if (!drm) > return -ENOMEM; > > - drm_dev_set_unique(drm, dev_name(>dev)); > dev_set_drvdata(>dev, drm); > > err = drm_dev_register(drm, 0); > diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c > index d5db9e0f3b73..647772305e8f 100644 > --- a/drivers/gpu/drm/vc4/vc4_drv.c > +++ b/drivers/gpu/drm/vc4/vc4_drv.c > @@ -168,8 +168,6 @@ static int vc4_drm_bind(struct device *dev) > vc4->dev = drm; > drm->dev_private = vc4; > > - drm_dev_set_unique(drm, dev_name(dev)); > - > drm_mode_config_init(drm); > if (ret) > goto unref; > -- > 2.6.3 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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] android: fix warning when releasing active sync point
On Tue, Dec 15, 2015 at 11:08:01AM -0800, Dmitry Torokhov wrote:
> On Tue, Dec 15, 2015 at 11:00 AM, Gustavo Padovan <gust...@padovan.org> wrote:
> > 2015-12-15 Daniel Vetter <dan...@ffwll.ch>:
> >
> >> On Mon, Dec 14, 2015 at 05:29:55PM -0800, Dmitry Torokhov wrote:
> >> > Userspace can close the sync device while there are still active fence
> >> > points, in which case kernel produces the following warning:
> >> >
> >> > [ 43.853176] [ cut here ]
> >> > [ 43.857834] WARNING: CPU: 0 PID: 892 at
> >> > /mnt/host/source/src/third_party/kernel/v3.18/drivers/staging/android/sync.c:439
> >> > android_fence_release+0x88/0x104()
> >> > [ 43.871741] CPU: 0 PID: 892 Comm: Binder_5 Tainted: G U
> >> > 3.18.0-07661-g0550ce9 #1
> >> > [ 43.880176] Hardware name: Google Tegra210 Smaug Rev 1+ (DT)
> >> > [ 43.885834] Call trace:
> >> > [ 43.888294] [] dump_backtrace+0x0/0x10c
> >> > [ 43.893697] [] show_stack+0x10/0x1c
> >> > [ 43.898756] [] dump_stack+0x74/0xb8
> >> > [ 43.903814] [] warn_slowpath_common+0x84/0xb0
> >> > [ 43.909736] [] warn_slowpath_null+0x14/0x20
> >> > [ 43.915482] [] android_fence_release+0x84/0x104
> >> > [ 43.921582] [] fence_release+0x104/0x134
> >> > [ 43.927066] [] sync_fence_free+0x74/0x9c
> >> > [ 43.932552] [] sync_fence_release+0x34/0x48
> >> > [ 43.938304] [] __fput+0x100/0x1b8
> >> > [ 43.943185] [] fput+0x8/0x14
> >> > [ 43.947982] [] task_work_run+0xb0/0xe4
> >> > [ 43.953297] [] do_notify_resume+0x44/0x5c
> >> > [ 43.958867] ---[ end trace 5a2aa4027cc5d171 ]---
> >> >
> >> > Let's fix it by introducing a new optional callback (disable_signaling)
> >> > to fence operations so that drivers can do proper clean ups when we
> >> > remove last callback for given fence.
> >> >
> >> > Reviewed-by: Andrew Bresticker <abres...@chromium.org>
> >> > Signed-off-by: Dmitry Torokhov <d...@chromium.org>
> >> > ---
> >> > drivers/dma-buf/fence.c| 6 +-
> >> > drivers/staging/android/sync.c | 8
> >> > include/linux/fence.h | 2 ++
> >> > 3 files changed, 15 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
> >> > index 7b05dbe..0ed73ad 100644
> >> > --- a/drivers/dma-buf/fence.c
> >> > +++ b/drivers/dma-buf/fence.c
> >> > @@ -304,8 +304,12 @@ fence_remove_callback(struct fence *fence, struct
> >> > fence_cb *cb)
> >> > spin_lock_irqsave(fence->lock, flags);
> >> >
> >> > ret = !list_empty(>node);
> >> > - if (ret)
> >> > + if (ret) {
> >> > list_del_init(>node);
> >> > + if (list_empty(>cb_list))
> >> > + if (fence->ops->disable_signaling)
> >> > + fence->ops->disable_signaling(fence);
> >>
> >> What exactly is the bug here? A fence with no callbacks registered any
> >> more shouldn't have any problem. Why exactly does this blow up?
> >
> > The WARN_ON is probably this one:
> > https://android.googlesource.com/kernel/common/+/android-3.18/drivers/staging/android/sync.c#433
> >
> > I've been wondering in the last few days if this warning is really
> > necessary. If the user is closing a sync_timeline that has unsignalled
> > fences it should probably be aware of that already. Then I think it is
> > okay to remove the the sync_pt from the active_list at the release-time.
> > In fact I've already prepared a patch doing that. Thoughts?
> >
>
> Maybe, but you need to make sure that you only affecting your fences.
>
> My main objection is that still leaves fence_remove_callback() being
> not mirror image of fence_add_callback().
That's 100% intentional. I looked at the sync.c code a bit more and it
duplicates a bunch of the fence stuff still. We need to either merge that
code into the mainline struct fence logic, or remove it. There shouldn't
really be any need for the userspace ABI layer to keep track of active
fences at all. Worse, it means that you must use the sync_pt struct to be
able to export it to userspace, and can't just export any normal struct
fence object. That breaks the abstraction we're aiming for.
Imo just remove that WARN_ON for now.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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] android: fix warning when releasing active sync point
On Tue, Dec 15, 2015 at 09:22:56AM -0800, Dmitry Torokhov wrote: > On Tue, Dec 15, 2015 at 5:30 AM, Gustavo Padovan <gust...@padovan.org> wrote: > > 2015-12-14 Dmitry Torokhov <d...@chromium.org>: > > > >> Userspace can close the sync device while there are still active fence > >> points, in which case kernel produces the following warning: > >> > >> [ 43.853176] [ cut here ] > >> [ 43.857834] WARNING: CPU: 0 PID: 892 at > >> /mnt/host/source/src/third_party/kernel/v3.18/drivers/staging/android/sync.c:439 > >> android_fence_release+0x88/0x104() > >> [ 43.871741] CPU: 0 PID: 892 Comm: Binder_5 Tainted: G U > >> 3.18.0-07661-g0550ce9 #1 > >> [ 43.880176] Hardware name: Google Tegra210 Smaug Rev 1+ (DT) > >> [ 43.885834] Call trace: > >> [ 43.888294] [] dump_backtrace+0x0/0x10c > >> [ 43.893697] [] show_stack+0x10/0x1c > >> [ 43.898756] [] dump_stack+0x74/0xb8 > >> [ 43.903814] [] warn_slowpath_common+0x84/0xb0 > >> [ 43.909736] [] warn_slowpath_null+0x14/0x20 > >> [ 43.915482] [] android_fence_release+0x84/0x104 > >> [ 43.921582] [] fence_release+0x104/0x134 > >> [ 43.927066] [] sync_fence_free+0x74/0x9c > >> [ 43.932552] [] sync_fence_release+0x34/0x48 > >> [ 43.938304] [] __fput+0x100/0x1b8 > >> [ 43.943185] [] fput+0x8/0x14 > >> [ 43.947982] [] task_work_run+0xb0/0xe4 > >> [ 43.953297] [] do_notify_resume+0x44/0x5c > >> [ 43.958867] ---[ end trace 5a2aa4027cc5d171 ]--- > > > > This crash report seems to be for a 3.18 kernel. Can you reproduce it > > on upstream kernel as well? > > Unfortunately this board does not run upsrteam just yet, but looking > at the sync driver and fence code we are pretty much in sync with > upstream. Just to check: Is that with a proper hw driver, or using SW_SYNC? The later will get removed in upstream since it's a debug/validation only interface. Well, removed for drivers and production systems, the kselftests will use it. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 1/2] drm: make drm_dev_set_unique() not use a format string
On Wed, Dec 09, 2015 at 12:52:58AM +0100, Nicolas Iooss wrote:
> On 12/09/2015 12:28 AM, Emil Velikov wrote:
> > On 8 December 2015 at 22:12, Nicolas Iooss <nicolas.iooss_li...@m4x.org>
> > wrote:
> >> drm_dev_set_unique() uses a format string to define the unique name of a
> >> device. This feature is not used as currently all the calls to this
> >> function either use "%s" as a format string or directly use
> >> dev_name().
> >>
> >> Even though this second kind of call does not introduce security
> >> problems, because there cannot be "%" characters in dev_name() results,
> >> gcc issues a warning when building with -Wformat-security flag
> >> ("warning: format string is not a string literal (potentially
> >> insecure)"). This warning is useful to find real bugs like the one
> >> fixed by commit 3958b79266b1 ("configfs: fix kernel infoleak through
> >> user-controlled format string"). False positives which do not bring
> >> an extra value make the work of finding real bugs harder.
> >>
> >> Therefore remove the format-string feature from drm_dev_set_unique().
> >>
> >> Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
> >> ---
> >> drivers/gpu/drm/drm_drv.c | 11 +++
> >> drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
> >> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 +-
> >> include/drm/drmP.h | 2 +-
> >> 4 files changed, 6 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> >> index 7dd6728dd092..20eaa0aae205 100644
> >> --- a/drivers/gpu/drm/drm_drv.c
> >> +++ b/drivers/gpu/drm/drm_drv.c
> >> @@ -797,7 +797,7 @@ EXPORT_SYMBOL(drm_dev_unregister);
> >> /**
> >> * drm_dev_set_unique - Set the unique name of a DRM device
> >> * @dev: device of which to set the unique name
> >> - * @fmt: format string for unique name
> >> + * @name: unique name
> >> *
> >> * Sets the unique name of a DRM device using the specified format string
> >> and
> >> * a variable list of arguments. Drivers can use this at driver probe
> >> time if
> > You might want to also update the above hunk :-)
>
> Indeed, thanks! I will wait a little bit for other feedbacks, read all
> the comments/documentation to see if anything else needs an update and
> submit a v2.
fyi 4.5 window for drm is closing in the next few days (because holidays
and all that). Please resend soon, otherwise it might miss and get delayed
to 4.6.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: [git pull] drm for 4.4-rc1
On Mon, Dec 14, 2015 at 07:14:09AM +0100, Stefan Lippers-Hollmann wrote: > Hi > > On 2015-12-10, Daniel Vetter wrote: > > On Thu, Dec 10, 2015 at 04:04:20AM +0100, Stefan Lippers-Hollmann wrote: > > > On 2015-11-09, Dave Airlie wrote: > [...] > > > This patch seems to introduce a regression for i915 in Linus' > > > v4.4-rc4-60-g9a0f76f, relative to v4.3 (and 4.3.1), on a sandy-bridge > > > (Intel DH67CL) system with a single HDMI connected monitor (Medion > > > MD20094) > > > attached. Immediately after the modeswitch, the monitor switches off and > > > stays off. Nothing catches my eyes in dmesg or Xorg.0.log, but bisecting > > > the issue points me at: > > > > > > 237ed86c693d8a8e4db476976aeb30df4deac74b is the first bad commit > > > commit 237ed86c693d8a8e4db476976aeb30df4deac74b > > > Author: Sonika Jindal <sonika.jin...@intel.com> > > > Date: Tue Sep 15 09:44:20 2015 +0530 > > > > > > drm/i915: Check live status before reading edid > [...] > > This is strange, after connecting a different monitor (Fujitsu-Siemens > Scaleoview D19-1) as a second screen via DVI, both monitors came up fine > and even after removing it (and reverting everything to the status quo > ante), this HDMI connected Medion MD20094 monitor continues to work on > the previously broken sandy-bridge DH67CL mainboard. > Despite this problem being 100% reproducable and bisectable before, > including multiple power cycles and replacing the HDMI cable, I have not > been able to reproduce the issue again. > > > A few things to test: > > - How does that screen fare on a working machine? Would tell us if the > > issue is with the sink or the source. > > It is working fine on a Baytrail-D (ASRock Q1900DC-ITX) and an ivy-bridge > (Gigabyte GA-H77M-D3H r1.1) system - and now on the previously affected > sandy-bridge system (Intel DH67CL) as well. > > > - Please boot up with drm.debug=0xe on a working and broken kernel, grab > > dmesg for both. > > dmesg-v4.4-rc4-113-g0bd0f1e-working.gz is attached, unfortunately I'm no > longer able to reproduce the previous failure (tested with easy of the > bisection steps, identical kernel binaries as before, and v4.4-rc5 as > well), so I can't provide a broken trace. > > > - Extending the timeout magic might be worth a shot like in the below > > patch. Crank up retry if it doesn't help. > [...] > > I can only imagine that it was right beyond the brink of the timeout > before and something had a lasting, but probably subtile, effect on the > timing after temporarily connecting the second screen; it is working now. > > On 2015-12-10, Jani Nikula wrote: > [...] > > The very first thing to do is to ensure you've tried v4.4-rc4, which > > contains > > I tested both plain v4.4-rc4 and v4.4-rc4-60-g9a0f76f > > > commit 0f5a9be15797f78c9a34e432f26c796165b6e49a > > Author: Imre Deak <imre.d...@intel.com> > > Date: Fri Nov 27 18:55:29 2015 +0200 > > > > drm/i915: take a power domain reference while checking the HDMI live > > status > > both containing this patch. > > > Thanks a lot and sorry for your trouble, I'll report back if anything > changes - or if I can reproduce the problem again. Ah no worries, this happens fairly often ;-) Could be the cable had a bad day or something like that. Also someone from intel noticed that the delay isn't quite working like it should (we don't do the full 3x10ms delay), patch for that should go to upstream hopefuly soon. Anyway, thanks for reporting. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 v2] drm/i915: Send a hotplug event on resume instead of an IRQ event
On Tue, Dec 15, 2015 at 05:25:56PM -0500, Lyude wrote:
> We currently call drm_helper_hpd_irq_event() to handle reprobing
> displays on resume, however drm_helper_hpd_irq_event() only checks the
> status of hpd. HPD doesn't update if the displays connected changed
> before resuming the system, and as such causes us not to detect any
> connection changes after resume.
>
> This originally wasn't the case since drm would reset all of the
> connectors to unknown during resume and cause a reprobe (since the
> connector statuses would change), however this behavior changed in
> 5677d67ae394 ('drm: Stop resetting connector state to unknown').
>
> Signed-off-by: Lyude <cp...@redhat.com>
> ---
> I did some research into why setting the connector state to unknown was
> causing a reprobe and discovered that the fact that this reprobing ever
> worked at all was just a coincidence (i915's IRQ handler gets called by the
> irq event, notices the state of all the connectors changed since they were
> set to unknown, causes a reprobe). This seems to fix everything (including
> monitor detection in normal VTs), and I can't figure out any real use now
> for us to set everything to unknown so I'm going to go the route of just
> sending the hotplug event instead.
>
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 760e0ce..27a6c1b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -792,7 +792,7 @@ static int i915_drm_resume(struct drm_device *dev)
>* */
> intel_hpd_init(dev_priv);
> /* Config may have changed between suspend and resume */
> - drm_helper_hpd_irq_event(dev);
> + drm_kms_helper_hotplug_event(dev);
So there's a bunch of things broken in this area, but just from your
description it's unclear what exactly. Problems I'm aware of:
- hpd_irq_event() only reprobes connectors for which hpd is enabled, which
are not all of them. We broke this in
commit 816da85a0990c2b52cfffa77637d1c770d6790e9
Author: Daniel Vetter <daniel.vet...@ffwll.ch>
Date: Tue Oct 23 18:23:33 2012 +
drm: handle HPD and polled connectors separately
which is positively ages ago. Also note that any other driver not using
vt switching also suffers from this (atm radeon, nouveau, rockchip,
amdgpu). otoh at least from a quick those drivers also don't really
bother with reprobing on resume really.
- DP MST reprobing is busted in i915 since
commit e7d6f7d708290da1b7c92f533444b042c79412e0
Author: Dave Airlie <airl...@redhat.com>
Date: Mon Dec 8 13:23:37 2014 +1000
drm/i915: resume MST after reading back hw state
There's some bug reports iirc from Takashi and Ted Tso about this. Your
patch won't help for DP MST since we never reprobe MST state from
userspace, it only ever happens through interrupt handling. Well mostly,
if you completely unplug something then we detect that and throw all the
MST stuff over board, but if you e.g. just change a connection in your
dock or go from one dock to another we won't detect this.
Which one of the above is it? And to what extend do you care about this
working everywhere?
Sorry that I'm opening a can of worms here.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: [RFC PATCH] fbdev: add support for Sigma Designs' smp8xxxfb.ko
On Wed, Dec 30, 2015 at 12:32:00PM +0200, Tomi Valkeinen wrote: > > > On 30/12/15 11:31, Sebastian Frias wrote: > > Hi, > > > > On 12/30/2015 09:06 AM, Tomi Valkeinen wrote: > >> > >> Also note that I don't want new fbdev drivers into the mainline kernel. > >> You should implement a DRM based driver instead. > >> > > > > Thanks, is there a porting guide to go from fbdev to DRM? > > I don't think you should "port" the driver from fbdev to DRM, as the > frameworks are just so different. You should implement the driver from > scratch. Of course, the bits of code that actually touch the hardware > can possibly be copied directly. > > Kernel docs contain documentation about DRM, but I don't know if there's > really a "how to write a DRM driver" style documentation. There's an > active mailing list and irc channel, though. Laurent Pinchart has a presentation which gives a good overview over drm for display drivers: https://www.youtube.com/watch?v=5uHMpjz68HE DRM docs are at http://dri.freedesktop.org/docs/drm/ (this version is using asciidoc for more pretties, but you can also build it locally with make htmldocs and then look at it in Documentation/DocBook/drm/index.html). > > Does DRM provides a "fbdev" backward compatible API? Would that be > > feasible? > > DRM provides an fbdev "emulation". I think it's mainly aimed at > providing fb console, but many fbdev applications should work fine on > top of it. Modeset side should be full featured out of the box (i.e. you can change modes), drivers have the option to overallocate (to allow the fake page flipping using set_par) and adding 2d accel is possible. > > I did not find much about that. > > > > Currently our stack is something like: > > > >Qt -> eglfs -> Mali -> fbdev -> mem -> output > > (HW) (HW) > > > > We don't control the eglfs/Mali (GPU) part. > > From what I could see, Mali uses DRM with X11 which we do not need > > (note: I'm not a Mali expert and just took a quick look at the code so I > > may be wrong), which could be a problem. > > I'm not familiar with Mali, so I have no idea. > > > If "implement a DRM driver" is a lot of work, it would end up as a > > business decision and probably would not happen. > > True. It's, of course, up to you. If the fbdev driver works fine for you > and provides all the features, and you're happy with it, and there's no > requirement to get the driver to the mainline Linux, there's not much > point in going for a DRM driver. > > > Would you say there are good solid arguments to shake our current stack > > (other than for dusting it off)? > > Fbdev is the legacy framework, hopefully deprecated at some point, and > DRM is the current display framework. So DRM has much more features, is > actively developed, has a community that may help you with your issues, etc. > > From purely technical point of view, it depends on the hardware in > question. If the HW supports hardware overlays and multiple outputs, DRM > supports those fully, whereas fbdev does not. > > I'm not that familiar with the 3D side, but I think that can be > implemented properly with DRM, whereas on fbdev supporting 3D is always > more or less a hack. > > > By the way, does DRM improves 2D acceleration support over fbdev? > > I don't know enough about 2D acceleration to answer that. We've tried in-kernel accel of 2d fbdev ops in i915 and realized it's too much work and pretty pointless. But it's definitely possible to do that, on top of the provided fbdev emulation (e.g. gma500 has some scrolling accel tricks). Otherwise same as 3D really, using the split kernel/userspace driver approach. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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 2/9] drm/gma500: use to_pci_dev()
On Sun, Dec 27, 2015 at 06:45:58PM +0800, Geliang Tang wrote:
> Use to_pci_dev() instead of open-coding it.
>
> Signed-off-by: Geliang Tang <geliangt...@163.com>
Applied to drm-misc, thanks.
-Daniel
> ---
> drivers/gpu/drm/gma500/power.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c
> index b6b135f..bea8578 100644
> --- a/drivers/gpu/drm/gma500/power.c
> +++ b/drivers/gpu/drm/gma500/power.c
> @@ -187,7 +187,7 @@ static bool gma_resume_pci(struct pci_dev *pdev)
> */
> int gma_power_suspend(struct device *_dev)
> {
> - struct pci_dev *pdev = container_of(_dev, struct pci_dev, dev);
> + struct pci_dev *pdev = to_pci_dev(_dev);
> struct drm_device *dev = pci_get_drvdata(pdev);
> struct drm_psb_private *dev_priv = dev->dev_private;
>
> @@ -214,7 +214,7 @@ int gma_power_suspend(struct device *_dev)
> */
> int gma_power_resume(struct device *_dev)
> {
> - struct pci_dev *pdev = container_of(_dev, struct pci_dev, dev);
> + struct pci_dev *pdev = to_pci_dev(_dev);
> struct drm_device *dev = pci_get_drvdata(pdev);
>
> mutex_lock(_mutex);
> --
> 2.5.0
>
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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] drm/dp/mst: constify drm_dp_mst_topology_cbs structures
On Wed, Dec 30, 2015 at 10:20:30PM +0100, Julia Lawall wrote:
> The drm_dp_mst_topology_cbs structures are never modified, so declare them
> as const.
>
> Done with the help of Coccinelle.
>
> Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
Applied to drm-misc, thanks.
-Daniel
>
> ---
> drivers/gpu/drm/i915/intel_dp_mst.c|2 +-
> drivers/gpu/drm/radeon/radeon_dp_mst.c |2 +-
> include/drm/drm_dp_mst_helper.h|2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 74b5888..b37b859 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -421,7 +421,7 @@ struct drm_dp_payload {
> struct drm_dp_mst_topology_mgr {
>
> struct device *dev;
> - struct drm_dp_mst_topology_cbs *cbs;
> + const struct drm_dp_mst_topology_cbs *cbs;
> int max_dpcd_transaction_bytes;
> struct drm_dp_aux *aux; /* auxch for this topology mgr to use */
> int max_payloads;
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> index 94323f5..8a02225 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> @@ -329,7 +329,7 @@ static void radeon_dp_mst_hotplug(struct
> drm_dp_mst_topology_mgr *mgr)
> drm_kms_helper_hotplug_event(dev);
> }
>
> -struct drm_dp_mst_topology_cbs mst_cbs = {
> +const struct drm_dp_mst_topology_cbs mst_cbs = {
> .add_connector = radeon_dp_add_mst_connector,
> .register_connector = radeon_dp_register_mst_connector,
> .destroy_connector = radeon_dp_destroy_mst_connector,
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> b/drivers/gpu/drm/i915/intel_dp_mst.c
> index e2f515d..fa0dabf 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -534,7 +534,7 @@ static void intel_dp_mst_hotplug(struct
> drm_dp_mst_topology_mgr *mgr)
> drm_kms_helper_hotplug_event(dev);
> }
>
> -static struct drm_dp_mst_topology_cbs mst_cbs = {
> +static const struct drm_dp_mst_topology_cbs mst_cbs = {
> .add_connector = intel_dp_add_mst_connector,
> .register_connector = intel_dp_register_mst_connector,
> .destroy_connector = intel_dp_destroy_mst_connector,
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: [Intel-gfx] [PATCH] drm/i915: intel_hpd_init(): Don't check connectors for an encoder
On Tue, Jan 05, 2016 at 09:05:49PM +0100, Daniel Vetter wrote:
> On Tue, Jan 05, 2016 at 02:53:08PM -0500, Lyude wrote:
> > There's no reason to check for an active encoder when setting up hpd,
> > especially since a disconnected connector isn't going to have an encoder
> > in the first place. This fixes reprobing display connectors on resume.
> >
> > Signed-off-by: Lyude <cp...@redhat.com>
> > ---
> > After some talking with danvet on IRC, I learned that calling
> > drm_helper_hpd_irq_event() does actually trigger a full reprobe of each
> > connector's status. It turns out this is the actual reason reprobing on
> > resume
> > hasn't been working (this was observed on a T440s):
> >
> > - We call hpd_init()
> > - We check each connector for a couple of things before marking
> > connector->polled with DRM_CONNECTOR_POLL_HPD, one of which is an
> > active encoder. Of course, a disconnected port won't have an active
> > encoder, so we don't add the flag to any of the connector's.
> > - We call drm_helper_hpd_irq_event()
> > - drm_helper_irq_event() checks each connector for the
> > DRM_CONNECTOR_POLL_HPD flag. The only one that has it is eDP-1, so we
> > skip reprobing each connector except that one.
>
> The above explanation should be part of the commit message, otherwise it's
> hard to understand the impact of this fix.
>
> Fixes: 0e32b39ceed6 ("drm/i915: add DP 1.2 MST support (v0.7)")
> Cc: sta...@vger.kernel.org
>
> Unfortunately Dave doesn't explain in his MST commit why he's done this.
> Dave? I can't come up with a reason, but better to test that. Lyude?
>
> Anyway looks good to me.
>
> Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Need to retract my r-b. MST connectors don't have a fixed fake encoder,
and the intel_connector->encoder link is done at link time and not static.
We need to filter out MST connectors here, otherwise this will go boom.
While at it, can you please line-break this properly to appease
checkpatch?
-Daniel
>
> >
> > drivers/gpu/drm/i915/intel_hotplug.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > b/drivers/gpu/drm/i915/intel_hotplug.c
> > index b177857..18797bc 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -468,7 +468,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> > list_for_each_entry(connector, _config->connector_list, head) {
> > struct intel_connector *intel_connector =
> > to_intel_connector(connector);
> > connector->polled = intel_connector->polled;
> > - if (connector->encoder && !connector->polled &&
> > I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE)
> > + if (!connector->polled && I915_HAS_HOTPLUG(dev) &&
> > intel_connector->encoder->hpd_pin > HPD_NONE)
> > connector->polled = DRM_CONNECTOR_POLL_HPD;
> > if (intel_connector->mst_port)
> > connector->polled = DRM_CONNECTOR_POLL_HPD;
> > --
> > 2.5.0
> >
> > ___
> > Intel-gfx mailing list
> > intel-...@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: [Intel-gfx] [PATCH v2] drm/i915: intel_hpd_init(): Fix suspend/resume reprobing
On Tue, Jan 05, 2016 at 04:15:30PM -0500, Lyude wrote:
> This fixes reprobing of display connectors on resume. After some talking
> with danvet on IRC, I learned that calling drm_helper_hpd_irq_event() does
> actually trigger a full reprobe of each connector's status. It turns out
> this is the actual reason reprobing on resume hasn't been working (this was
> observed on a T440s):
>
> - We call hpd_init()
> - We check each connector for a couple of things before marking
> connector->polled with DRM_CONNECTOR_POLL_HPD, one of which is an
> active encoder. Of course, a disconnected port won't have an
> active encoder, so we don't add the flag to any of the
> connectors.
> - We call drm_helper_hpd_irq_event()
> - drm_helper_irq_event() checks each connector for the
> DRM_CONNECTOR_POLL_HPD flag. The only one that has it is eDP-1,
> so we skip reprobing each connector except for that one.
>
> In addition, we now avoid setting connector->polled to
> DRM_CONNECTOR_POLL_HPD for MST connectors, since their reprobing is handled
> by the mst helpers. This is probably what was originally intended to happen
> here anyway.
>
> Fixes: 0e32b39ceed6 ("drm/i915: add DP 1.2 MST support (v0.7)")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Lyude <cp...@redhat.com>
> ---
> Changes
> * Use the explanation of the issue as the commit message instead
> * Change the title of the commit, since this does more then just stop a check
> for an encoder now
> * Add "Fixes" line for the patch that introduced this regression
> * Don't enable DRM_CONNECTOR_POLL_HPD for mst connectors
For drm/i915 I prefer when the patch changelog is above the s-o-b section,
with v2: v3: ... headings.
>
> drivers/gpu/drm/i915/intel_hotplug.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index b177857..51ecf0b4 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -468,9 +468,9 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> list_for_each_entry(connector, _config->connector_list, head) {
> struct intel_connector *intel_connector =
> to_intel_connector(connector);
> connector->polled = intel_connector->polled;
> - if (connector->encoder && !connector->polled &&
> I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE)
> - connector->polled = DRM_CONNECTOR_POLL_HPD;
> - if (intel_connector->mst_port)
> + if (!connector->polled && !intel_connector->mst_port &&
> + I915_HAS_HOTPLUG(dev) &&
> + intel_connector->encoder->hpd_pin > HPD_NONE)
> connector->polled = DRM_CONNECTOR_POLL_HPD;
Hm, on 2nd thought we could clarify this further like this:
list_for_each_entry(...) {
...
/* MST has a dynamic intel_connector->encoder and it's
* reprobing is all handled by the MST helpers. */
if (intel_connector->mst_port)
continue;
if (!connector->polled && I915_HAS_HOTPLUG(dev) &&
intel_connector->encoder->hpd_pin > HPD_NONE)
connector->polled = DRM_CONNECTOR_POLL_HPD;
}
That yields even tidier code and gives us a place for a comment. Can you
pls respin once more?
Thanks, Daniel
> }
>
> --
> 2.5.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: drm/i915/intel_drv.h - RPM wakelock ref not held during HW access
_cgroup_shrink_node_zone+0x290/0x290 > [Tue Jan 5 15:15:44 2016] [] ? > __this_cpu_preempt_check+0x13/0x20 > [Tue Jan 5 15:15:44 2016] [] ? > _raw_spin_unlock_irq+0x38/0x50 > [Tue Jan 5 15:15:44 2016] [] ? > wake_atomic_t_function+0x270/0x270 > [Tue Jan 5 15:15:44 2016] [] ? > mem_cgroup_shrink_node_zone+0x290/0x290 > [Tue Jan 5 15:15:44 2016] [] kthread+0x218/0x2e0 > [Tue Jan 5 15:15:44 2016] [] ? > kthread_create_on_node+0x460/0x460 > [Tue Jan 5 15:15:44 2016] [] ? > kthread_create_on_node+0x460/0x460 > [Tue Jan 5 15:15:44 2016] [] ret_from_fork+0x3f/0x70 > [Tue Jan 5 15:15:44 2016] [] ? > kthread_create_on_node+0x460/0x460 > [Tue Jan 5 15:15:44 2016] ---[ end trace 074b4bd8168aac27 ]--- > >In attachment all the hardware details. Also you can find compiled kernel > here: >http://mail.gelma.net/i915 > > Thanks a lot for your work, > Andrea > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- 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: [Intel-gfx] [PATCH v3] drm/i915: intel_hpd_init(): Fix suspend/resume reprobing
On Thu, Jan 07, 2016 at 10:43:28AM -0500, Lyude wrote:
> This fixes reprobing of display connectors on resume. After some
> talking with danvet on IRC, I learned that calling
> drm_helper_hpd_irq_event() does actually trigger a full reprobe of each
> connector's status. It turns out this is the actual reason reprobing on
> resume hasn't been working (this was observed on a T440s):
>
> - We call hpd_init()
> - We check each connector for a couple of things before marking
> connector->polled with DRM_CONNECTOR_POLL_HPD, one of which is an
> active encoder. Of course, a disconnected port won't have an
> active encoder, so we don't add the flag to any of the
> connectors.
> - We call drm_helper_hpd_irq_event()
> - drm_helper_irq_event() checks each connector for the
> DRM_CONNECTOR_POLL_HPD flag. The only one that has it is eDP-1,
> so we skip reprobing each connector except that one.
>
> In addition, we also now avoid setting connector->polled to
> DRM_CONNECTOR_POLL_HPD for MST connectors, since their reprobing is
> handled by the mst helpers. This is probably what was originally
> intended to happen here.
>
> Changes since V1:
> * Use the explanation of the issue as the commit message instead
> * Change the title of the commit, since this does more then just stop a
> check for an encoder now
> * Add "Fixes" line for the patch that introduced this regression
> * Don't enable DRM_CONNECTOR_POLL_HPD for mst connectors
>
> Changes since V2:
> * Put patch changelog above Signed-off-by
> * Follow Daniel Vetter's suggestion for making the code here a bit more
> legible
>
> Fixes: 0e32b39ceed6 ("drm/i915: add DP 1.2 MST support (v0.7)")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Lyude <cp...@redhat.com>
Awesome, and merged to drm-intel.
Thanks, Daniel
> ---
> drivers/gpu/drm/i915/intel_hotplug.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index b177857..d7a6437 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -468,9 +468,14 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> list_for_each_entry(connector, _config->connector_list, head) {
> struct intel_connector *intel_connector =
> to_intel_connector(connector);
> connector->polled = intel_connector->polled;
> - if (connector->encoder && !connector->polled &&
> I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE)
> - connector->polled = DRM_CONNECTOR_POLL_HPD;
> +
> + /* MST has a dynamic intel_connector->encoder and it's reprobing
> + * is all handled by the MST helpers. */
> if (intel_connector->mst_port)
> + continue;
> +
> + if (!connector->polled && I915_HAS_HOTPLUG(dev) &&
> + intel_connector->encoder->hpd_pin > HPD_NONE)
> connector->polled = DRM_CONNECTOR_POLL_HPD;
> }
>
> --
> 2.5.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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: [Intel-gfx] [PATCH] drm/i915: intel_hpd_init(): Don't check connectors for an encoder
On Tue, Jan 05, 2016 at 02:53:08PM -0500, Lyude wrote:
> There's no reason to check for an active encoder when setting up hpd,
> especially since a disconnected connector isn't going to have an encoder
> in the first place. This fixes reprobing display connectors on resume.
>
> Signed-off-by: Lyude <cp...@redhat.com>
> ---
> After some talking with danvet on IRC, I learned that calling
> drm_helper_hpd_irq_event() does actually trigger a full reprobe of each
> connector's status. It turns out this is the actual reason reprobing on resume
> hasn't been working (this was observed on a T440s):
>
> - We call hpd_init()
> - We check each connector for a couple of things before marking
> connector->polled with DRM_CONNECTOR_POLL_HPD, one of which is an
> active encoder. Of course, a disconnected port won't have an active
> encoder, so we don't add the flag to any of the connector's.
> - We call drm_helper_hpd_irq_event()
> - drm_helper_irq_event() checks each connector for the
> DRM_CONNECTOR_POLL_HPD flag. The only one that has it is eDP-1, so we
> skip reprobing each connector except that one.
The above explanation should be part of the commit message, otherwise it's
hard to understand the impact of this fix.
Fixes: 0e32b39ceed6 ("drm/i915: add DP 1.2 MST support (v0.7)")
Cc: sta...@vger.kernel.org
Unfortunately Dave doesn't explain in his MST commit why he's done this.
Dave? I can't come up with a reason, but better to test that. Lyude?
Anyway looks good to me.
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
>
> drivers/gpu/drm/i915/intel_hotplug.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index b177857..18797bc 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -468,7 +468,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> list_for_each_entry(connector, _config->connector_list, head) {
> struct intel_connector *intel_connector =
> to_intel_connector(connector);
> connector->polled = intel_connector->polled;
> - if (connector->encoder && !connector->polled &&
> I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE)
> + if (!connector->polled && I915_HAS_HOTPLUG(dev) &&
> intel_connector->encoder->hpd_pin > HPD_NONE)
> connector->polled = DRM_CONNECTOR_POLL_HPD;
> if (intel_connector->mst_port)
> connector->polled = DRM_CONNECTOR_POLL_HPD;
> --
> 2.5.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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/
