Re: [PATCH v2] drm/msm/dpu: add display port support in DPU
Hi Jeykumar,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on robclark/msm-next]
[also build test ERROR on v4.20-rc4 next-20181130]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Jeykumar-Sankaran/drm-msm-dpu-add-display-port-support-in-DPU/20181130-230021
base: git://people.freedesktop.org/~robclark/linux msm-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm
All errors (new ones prefixed by >>):
drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c: In function
'_dpu_kms_initialize_displayport':
>> drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c:483:13: error: 'struct
>> msm_drm_private' has no member named 'dp'; did you mean 'edp'?
if (!priv->dp)
^~
edp
>> drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c:492:7: error: implicit declaration
>> of function 'msm_dp_modeset_init'; did you mean 'msm_edp_modeset_init'?
>> [-Werror=implicit-function-declaration]
rc = msm_dp_modeset_init(priv->dp, dev, encoder);
^~~
msm_edp_modeset_init
drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c:492:33: error: 'struct
msm_drm_private' has no member named 'dp'; did you mean 'edp'?
rc = msm_dp_modeset_init(priv->dp, dev, encoder);
^~
edp
cc1: some warnings being treated as errors
vim +483 drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c
475
476 static void _dpu_kms_initialize_displayport(struct drm_device *dev,
477 struct msm_drm_private
*priv,
478 struct dpu_kms *dpu_kms)
479 {
480 struct drm_encoder *encoder = NULL;
481 int rc;
482
> 483 if (!priv->dp)
484 return;
485
486 encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
487 if (IS_ERR(encoder)) {
488 DPU_ERROR("encoder init failed for dsi display\n");
489 return;
490 }
491
> 492 rc = msm_dp_modeset_init(priv->dp, dev, encoder);
493 if (rc) {
494 DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
495 drm_encoder_cleanup(encoder);
496 return;
497 }
498
499 priv->encoders[priv->num_encoders++] = encoder;
500 }
501
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108814] [radeonsi] page fault, umr dump
https://bugs.freedesktop.org/show_bug.cgi?id=108814 Domen changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 08:39:01PM +, Abuse wrote: > On Friday, 30 November 2018 20:35:07 GMT David Miller wrote: > > From: Jens Axboe > > Date: Fri, 30 Nov 2018 13:12:26 -0700 > > > > > On 11/30/18 12:56 PM, Davidlohr Bueso wrote: > > >> On Fri, 30 Nov 2018, Kees Cook wrote: > > >> > > >>> On Fri, Nov 30, 2018 at 11:27 AM Jarkko Sakkinen > > >>> wrote: > > > > In order to comply with the CoC, replace with a hug. > > >> > > >> I hope this is some kind of joke. How would anyone get offended by > > >> reading > > >> technical comments? This is all beyond me... > > > > > > Agree, this is insanity. > > > > And even worse it is censorship. > > > > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > Fuck > > I assume I will now be barred. > > Thank you for taking the opportunity to practice free speech in the always so welcoming and inclusive on-line world :-) Now I'll just have to find my notebook and write this prose down so that I'll never forget it. Thanks again. /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v3 03/19] kunit: test: add string_stream a std::stream like string builder
On Fri, Nov 30, 2018 at 06:14:17PM -0800, Brendan Higgins wrote: > On Thu, Nov 29, 2018 at 7:29 PM Luis Chamberlain wrote: > > > > On Wed, Nov 28, 2018 at 11:36:20AM -0800, Brendan Higgins wrote: > > > A number of test features need to do pretty complicated string printing > > > where it may not be possible to rely on a single preallocated string > > > with parameters. > > > > > > So provide a library for constructing the string as you go similar to > > > C++'s std::string. > > > > Hrm, what's the potential for such thing actually being eventually > > generically useful for printk folks, I wonder? Petr? > > Are you saying you think this is applicable for other things? Yes. > This doesn't belong here. Luis ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v3 01/19] kunit: test: add KUnit test runner core
On Fri, Nov 30, 2018 at 06:08:36PM -0800, Brendan Higgins wrote:
> On Thu, Nov 29, 2018 at 7:28 PM Luis Chamberlain wrote:
> >
> > > +static void kunit_run_case_internal(struct kunit *test,
> > > + struct kunit_module *module,
> > > + struct kunit_case *test_case)
> > > +{
> > > + int ret;
> > > +
> > > + if (module->init) {
> > > + ret = module->init(test);
> > > + if (ret) {
> > > + kunit_err(test, "failed to initialize: %d", ret);
> > > + kunit_set_success(test, false);
> > > + return;
> > > + }
> > > + }
> > > +
> > > + test_case->run_case(test);
> > > +}
> >
> > <-- snip -->
> >
> > > +static bool kunit_run_case(struct kunit *test,
> > > +struct kunit_module *module,
> > > +struct kunit_case *test_case)
> > > +{
> > > + kunit_set_success(test, true);
> > > +
> > > + kunit_run_case_internal(test, module, test_case);
> > > + kunit_run_case_cleanup(test, module, test_case);
> > > +
> > > + return kunit_get_success(test);
> > > +}
> >
> > So we are running the module->init() for each test case... is that
> > correct? Shouldn't the init run once? Also, typically init calls are
>
> Yep, it's correct. `module->init()` should run once before every test
> case, reason being that the kunit_module serves as a test fixture in
> which each test cases should be run completely independently of every
> other.
Shouldn't the init be test_case specific as well? Right now we just
past the struct kunit, but not the struct kunit_case. I though that
that the struct kunit_case was where we'd customize each specific
test case as we see fit for each test case. If not, how would we
do say, a different type of initialization for a different type of
test (for the same unit)?
> init and exit is supposed to allow code common to all test
> cases to run since it is so common to have dependencies needed for a
> test to be common to every test case.
Sure things in common make sense, however the differntiating aspects
seem important as well on init? Or should the author be doing all
custom specific initializations on run_case() instead?
Luis
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v3 01/19] kunit: test: add KUnit test runner core
On Wed, Nov 28, 2018 at 11:36:18AM -0800, Brendan Higgins wrote:
> +int kunit_run_tests(struct kunit_module *module)
> +{
> + bool all_passed = true, success;
> + struct kunit_case *test_case;
> + struct kunit test;
> + int ret;
> +
> + ret = kunit_init_test(, module->name);
> + if (ret)
> + return ret;
> +
> + for (test_case = module->test_cases; test_case->run_case; test_case++) {
> + success = kunit_run_case(, module, test_case);
We are running test cases serially, why not address testing
asynchronously, this way tests can also be paralellized when possible,
therefore decreasing test time even further.
Would that mess up the printing/log stuff somehow?
Luis
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v3 01/19] kunit: test: add KUnit test runner core
On Fri, Nov 30, 2018 at 05:51:11PM -0800, Brendan Higgins wrote:
> On Thu, Nov 29, 2018 at 7:14 PM Luis Chamberlain wrote:
> >
> > On Wed, Nov 28, 2018 at 11:36:18AM -0800, Brendan Higgins wrote:
> > > +#define module_test(module) \
> > > + static int module_kunit_init##module(void) \
> > > + { \
> > > + return kunit_run_tests(); \
> > > + } \
> > > + late_initcall(module_kunit_init##module)
> >
> > Here in lies an assumption that suffices. I'm inclined to believe we
> > need new initcall level here so to ensure we *do* run after all the
> > respective kernels iniut calls. Otherwise we're left at the whims of
> > link order for kunit. For instance if a kunit test relies on frameworks
> > which are also late_initcall() we'd have complete incompatibility with
> > anything linked *after* kunit.
>
> Yep, I have some patches that address this, but I thought this is
> sufficient for the initial patchset (I figured that's the type of
> thing that people will have opinions about so best to get it out of
> the critical path). Do you want me to add those in the next revision?
>
> >
> > > diff --git a/kunit/Kconfig b/kunit/Kconfig
> > > new file mode 100644
> > > index 0..49b44c4f6630a
> > > --- /dev/null
> > > +++ b/kunit/Kconfig
> > > @@ -0,0 +1,17 @@
> > > +#
> > > +# KUnit base configuration
> > > +#
> > > +
> > > +menu "KUnit support"
> > > +
> > > +config KUNIT
> > > + bool "Enable support for unit tests (KUnit)"
> > > + depends on UML
> >
> > Consider using:
> >
> > if UML
> >...
> > endif
> >
> > That allows the depends to be done once.
>
> If you want to eliminate depends, wouldn't it be best to have KUNIT
> depend on whatever it needs, and then do `if KUNIT` below that? That
> seems cleaner over the long term. Anyway, Kees actually asked me to
> change it to the way it is now; I really don't care either way.
Yes, that works better. The idea is to just avoid having to write in
depends on over and over again.
> > I'm a bit conflicted here. This currently depends on UML but yet you
> > noted on RFC v2 that your intention is to liberate kunit from UML and
> > ideally allow unit tests to depend only on userspace. I've addressed
> > tests using both selftests kernels drivers and also re-written kernel
> > APIs to userspace to test there. I think we may need to live with both.
>
> I am not entirely opposed. The greater isolation we can achieve, the
> fewer dependencies, and barriers to setting up test fixtures the
> better. I think the best way to do that in most cases is allowing
> minimal test binaries to be built that have the absolute minimum
> amount of code necessary to test the desired property. That being
> said, integration tests are a thing and drawing a line between them
> and unit tests is not always possible, so supporting other
> architectures might be necessary.
Then lets pave the way for it to be done easily.
> > Then for the UML stuff, I think if we *really* accept that UML will
> > always be a viable option we should probably consider now throwing these
> > things under drivers/platform/uml/. This follows the pattern of arch
> > specific drivers. Whether or not we end up with a complete userspace
> > component independent of UML may implicate having a shared component
> > somewhere else.
>
> Fair enough. What specifically are you suggesting should go in
> `drivers/platform/uml`? Just the bits that are completely tied to UML
> or a concrete architecture?
The bits that are UML specific. As I see it, with the above intention
clarified, kunit is a framework for architectures and UML is supported
first. The code doesn't currently reflect this.
> > Likewise, I realize the goal is to *avoid* using a virtual machine for
> > these tests, but would it in any way make sense to share kunit to be
> > supported for other architectures to allow easier-to-write tests as
> > well?
>
> You are not the first person to ask for this.
>
> For the vast majority of tests, I think we can (and consequently
> should) make them run without any external dependencies. Doing so
> makes it such that someone can run a test without knowing anything
> about it, which allows you to do a lot of things. For one, I, as a
> developer, don't have to hunt down somebody's QEMU patches, or
> whatever. But it also means I, as someone maintaining part of the
> kernel, can make nice test runners and build things like presubmit
> servers on top of them.
>
> Nevertheless, I accept that there are things which are just easier to
> do with hardware or a VM (for integration tests it is necessary).
> Still, I think we need to make sure the vast majority of unit tests do
> not depend on real hardware or a VM.
When possible, sure.
Luis
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/1] drm: msm: Replace dma_map_sg with dma_sync_sg*
On Thu, Nov 29, 2018 at 4:23 PM Tomasz Figa wrote: > > On Thu, Nov 29, 2018 at 12:03 PM Robin Murphy wrote: > > > > On 29/11/2018 19:57, Tomasz Figa wrote: > > > On Thu, Nov 29, 2018 at 11:40 AM Jordan Crouse > > > wrote: > > >> > > >> On Thu, Nov 29, 2018 at 01:48:15PM -0500, Rob Clark wrote: > > >>> On Thu, Nov 29, 2018 at 10:54 AM Christoph Hellwig wrote: > > > > On Thu, Nov 29, 2018 at 09:42:50AM -0500, Rob Clark wrote: > > > Maybe the thing we need to do is just implement a blacklist of > > > compatible strings for devices which should skip the automatic > > > iommu/dma hookup. Maybe a bit ugly, but it would also solve a problem > > > preventing us from enabling per-process pagetables for a5xx (where we > > > need to control the domain/context-bank that is allocated by the dma > > > api). > > > > You can detach from the dma map attachment using > > arm_iommu_detach_device, > > which a few drm drivers do, but I don't think this is the problem. > > >>> > > >>> I think even with detach, we wouldn't end up with the context-bank > > >>> that the gpu firmware was hard-coded to expect, and so it would > > >>> overwrite the incorrect page table address register. (I could be > > >>> mis-remembering that, Jordan spent more time looking at that. But it > > >>> was something along those lines.) > > >> > > >> Right - basically the DMA domain steals context bank 0 and the GPU is > > >> hard coded > > >> to use that context bank for pagetable switching. > > >> > > >> I believe the Tegra guys also had a similar problem with a hard coded > > >> context > > >> bank. > > > > AIUI, they don't need a specific hardware context, they just need to > > know which one they're actually using, which the domain abstraction hides. > > > > > Wait, if we detach the GPU/display struct device from the default > > > domain and attach it to a newly allocated domain, wouldn't the newly > > > allocated domain use the context bank we need? Note that we're already > > > > The arm-smmu driver doesn't, but there's no fundamental reason it > > couldn't. That should just need code to refcount domain users and > > release hardware contexts for domains with no devices currently attached. > > > > Robin. > > > > > doing that, except that we're doing it behind the back of the DMA > > > mapping subsystem, so that it keeps using the IOMMU version of the DMA > > > ops for the device and doing any mapping operations on the default > > > domain. If we ask the DMA mapping to detach, wouldn't it essentially > > > solve the problem? > > Thanks Robin. > > Still, my point is that the MSM DRM driver attaches the GPU struct > device to a new domain it allocates using iommu_domain_alloc() and it > seems to work fine, so I believe it's not the problem we're looking > into with this patch. Could we just make the MSM DRM call arch_teardown_dma_ops() and then arch_setup_dma_ops() with the `iommu` argument set to NULL and be done with it? Best regards, Tomasz ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/6] drm/v3d: Document cache flushing ABI.
Right now, userspace doesn't do any L2T writes, but we should lay out
our expectations for how it works.
Signed-off-by: Eric Anholt
---
include/uapi/drm/v3d_drm.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h
index 35c7d813c66e..95b8f8e82ea5 100644
--- a/include/uapi/drm/v3d_drm.h
+++ b/include/uapi/drm/v3d_drm.h
@@ -52,6 +52,13 @@ extern "C" {
*
* This asks the kernel to have the GPU execute an optional binner
* command list, and a render command list.
+ *
+ * The caches (L1T, slice, and L2T) will be flushed before the job
+ * executes. The TLB writes are guaranteed to have been flushed by
+ * the time the render done IRQ happens, which is the trigger for
+ * out_sync. Any dirtying of cachelines by the job (only possible
+ * using TMU writes) must be flushed by the caller using the CL's
+ * cache flush commands.
*/
struct drm_v3d_submit_cl {
/* Pointer to the binner command list.
--
2.20.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/6] drm/v3d: Add more tracepoints for V3D GPU rendering.
The core scheduler tells us when the job is pushed to the scheduler's
queue, and I had the job_run functions saying when they actually queue
the job to the hardware. By adding tracepoints for the very top of
the ioctls and the IRQs signaling job completion, "perf record -a -e
v3d:.\* -e gpu_scheduler:.\* ; perf script" gets you a pretty
decent timeline.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_gem.c | 4 ++
drivers/gpu/drm/v3d/v3d_irq.c | 19 +-
drivers/gpu/drm/v3d/v3d_trace.h | 101
3 files changed, 121 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 0bd6892e3044..2f82e2724b1f 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -484,6 +484,8 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
struct drm_syncobj *sync_out;
int ret = 0;
+ trace_v3d_submit_cl_ioctl(>drm, args->rcl_start, args->rcl_end);
+
if (args->pad != 0) {
DRM_INFO("pad must be zero: %d\n", args->pad);
return -EINVAL;
@@ -611,6 +613,8 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
int ret = 0;
int bo_count;
+ trace_v3d_submit_tfu_ioctl(>drm, args->iia);
+
job = kcalloc(1, sizeof(*job), GFP_KERNEL);
if (!job)
return -ENOMEM;
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index dd7a7b0bd5a1..69338da70ddc 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -15,6 +15,7 @@
#include "v3d_drv.h"
#include "v3d_regs.h"
+#include "v3d_trace.h"
#define V3D_CORE_IRQS ((u32)(V3D_INT_OUTOMEM | \
V3D_INT_FLDONE | \
@@ -88,12 +89,20 @@ v3d_irq(int irq, void *arg)
}
if (intsts & V3D_INT_FLDONE) {
- dma_fence_signal(v3d->bin_job->bin.done_fence);
+ struct v3d_fence *fence =
+ to_v3d_fence(v3d->bin_job->bin.done_fence);
+
+ trace_v3d_bcl_irq(>drm, fence->seqno);
+ dma_fence_signal(>base);
status = IRQ_HANDLED;
}
if (intsts & V3D_INT_FRDONE) {
- dma_fence_signal(v3d->render_job->render.done_fence);
+ struct v3d_fence *fence =
+ to_v3d_fence(v3d->render_job->render.done_fence);
+
+ trace_v3d_rcl_irq(>drm, fence->seqno);
+ dma_fence_signal(>base);
status = IRQ_HANDLED;
}
@@ -119,7 +128,11 @@ v3d_hub_irq(int irq, void *arg)
V3D_WRITE(V3D_HUB_INT_CLR, intsts);
if (intsts & V3D_HUB_INT_TFUC) {
- dma_fence_signal(v3d->tfu_job->done_fence);
+ struct v3d_fence *fence =
+ to_v3d_fence(v3d->tfu_job->done_fence);
+
+ trace_v3d_tfu_irq(>drm, fence->seqno);
+ dma_fence_signal(>base);
status = IRQ_HANDLED;
}
diff --git a/drivers/gpu/drm/v3d/v3d_trace.h b/drivers/gpu/drm/v3d/v3d_trace.h
index f54ed9cd3444..edd984afa33f 100644
--- a/drivers/gpu/drm/v3d/v3d_trace.h
+++ b/drivers/gpu/drm/v3d/v3d_trace.h
@@ -12,6 +12,28 @@
#define TRACE_SYSTEM v3d
#define TRACE_INCLUDE_FILE v3d_trace
+TRACE_EVENT(v3d_submit_cl_ioctl,
+ TP_PROTO(struct drm_device *dev, u32 ct1qba, u32 ct1qea),
+ TP_ARGS(dev, ct1qba, ct1qea),
+
+ TP_STRUCT__entry(
+__field(u32, dev)
+__field(u32, ct1qba)
+__field(u32, ct1qea)
+),
+
+ TP_fast_assign(
+ __entry->dev = dev->primary->index;
+ __entry->ct1qba = ct1qba;
+ __entry->ct1qea = ct1qea;
+ ),
+
+ TP_printk("dev=%u, RCL 0x%08x..0x%08x",
+ __entry->dev,
+ __entry->ct1qba,
+ __entry->ct1qea)
+);
+
TRACE_EVENT(v3d_submit_cl,
TP_PROTO(struct drm_device *dev, bool is_render,
uint64_t seqno,
@@ -42,6 +64,85 @@ TRACE_EVENT(v3d_submit_cl,
__entry->ctnqea)
);
+TRACE_EVENT(v3d_bcl_irq,
+ TP_PROTO(struct drm_device *dev,
+uint64_t seqno),
+ TP_ARGS(dev, seqno),
+
+ TP_STRUCT__entry(
+__field(u32, dev)
+__field(u64, seqno)
+),
+
+ TP_fast_assign(
+ __entry->dev = dev->primary->index;
+ __entry->seqno = seqno;
+ ),
+
+ TP_printk("dev=%u, seqno=%llu",
+ __entry->dev,
+ __entry->seqno)
+);
+
+TRACE_EVENT(v3d_rcl_irq,
+ TP_PROTO(struct drm_device *dev,
+uint64_t
[PATCH 6/6] drm/v3d: Add missing fence timeline name for TFU.
We shouldn't be returning v3d-render in for our new queue.
Signed-off-by: Eric Anholt
Fixes: 83d5139982db ("drm/v3d: Add support for submitting jobs to the TFU.")
---
drivers/gpu/drm/v3d/v3d_fence.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_fence.c b/drivers/gpu/drm/v3d/v3d_fence.c
index 50bfcf9a8a1a..b0a2a1ae2eb1 100644
--- a/drivers/gpu/drm/v3d/v3d_fence.c
+++ b/drivers/gpu/drm/v3d/v3d_fence.c
@@ -29,10 +29,16 @@ static const char *v3d_fence_get_timeline_name(struct
dma_fence *fence)
{
struct v3d_fence *f = to_v3d_fence(fence);
- if (f->queue == V3D_BIN)
+ switch (f->queue) {
+ case V3D_BIN:
return "v3d-bin";
- else
+ case V3D_RENDER:
return "v3d-render";
+ case V3D_TFU:
+ return "v3d-tfu";
+ default:
+ return NULL;
+ }
}
const struct dma_fence_ops v3d_fence_ops = {
--
2.20.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/6] drm/v3d: Drop unused v3d_flush_caches().
Now that I've specified how the end-of-pipeline flushing should work,
we're never going to use this function.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_drv.h | 1 -
drivers/gpu/drm/v3d/v3d_gem.c | 21 -
2 files changed, 22 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index bcd3d567bec2..239b56d76f3e 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -314,7 +314,6 @@ void v3d_exec_put(struct v3d_exec_info *exec);
void v3d_tfu_job_put(struct v3d_tfu_job *exec);
void v3d_reset(struct v3d_dev *v3d);
void v3d_invalidate_caches(struct v3d_dev *v3d);
-void v3d_flush_caches(struct v3d_dev *v3d);
/* v3d_irq.c */
void v3d_irq_init(struct v3d_dev *v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 8b4af512450f..34103205b7cb 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -175,20 +175,6 @@ v3d_invalidate_slices(struct v3d_dev *v3d, int core)
V3D_SET_FIELD(0xf, V3D_SLCACTL_ICC));
}
-/* Invalidates texture L2 cachelines */
-static void
-v3d_invalidate_l2t(struct v3d_dev *v3d, int core)
-{
- V3D_CORE_WRITE(core,
- V3D_CTL_L2TCACTL,
- V3D_L2TCACTL_L2TFLS |
- V3D_SET_FIELD(V3D_L2TCACTL_FLM_CLEAR, V3D_L2TCACTL_FLM));
- if (wait_for(!(V3D_CORE_READ(core, V3D_CTL_L2TCACTL) &
- V3D_L2TCACTL_L2TFLS), 100)) {
- DRM_ERROR("Timeout waiting for L2T invalidate\n");
- }
-}
-
void
v3d_invalidate_caches(struct v3d_dev *v3d)
{
@@ -199,13 +185,6 @@ v3d_invalidate_caches(struct v3d_dev *v3d)
v3d_flush_l2t(v3d, 0);
}
-void
-v3d_flush_caches(struct v3d_dev *v3d)
-{
- v3d_invalidate_l1td(v3d, 0);
- v3d_invalidate_l2t(v3d, 0);
-}
-
static void
v3d_attach_object_fences(struct v3d_bo **bos, int bo_count,
struct dma_fence *fence)
--
2.20.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/6] drm/v3d: Drop the wait for L2T flush to complete.
According to Dave, once you've started an L2T flush, all L2T accesses
will be blocked until the flush completes. This fixes a consistent
3-4ms stall between the ioctl and running the job, and 3DMMES Taiji
goes from 27fps to 110fps.
Signed-off-by: Eric Anholt
Fixes: 57692c94dcbe ("drm/v3d: Introduce a new DRM driver for Broadcom V3D
V3.x+")
---
drivers/gpu/drm/v3d/v3d_gem.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index cc4d025b01e0..0bd6892e3044 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -146,10 +146,6 @@ v3d_flush_l2t(struct v3d_dev *v3d, int core)
V3D_CORE_WRITE(core, V3D_CTL_L2TCACTL,
V3D_L2TCACTL_L2TFLS |
V3D_SET_FIELD(V3D_L2TCACTL_FLM_FLUSH, V3D_L2TCACTL_FLM));
- if (wait_for(!(V3D_CORE_READ(core, V3D_CTL_L2TCACTL) &
- V3D_L2TCACTL_L2TFLS), 100)) {
- DRM_ERROR("Timeout waiting for L2T flush\n");
- }
}
/* Invalidates the slice caches. These are read-only caches. */
--
2.20.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/6] drm/v3d: Don't bother flushing L1TD at job start.
This is the write combiner for TMU writes. You're supposed to flush
that at job end if you had dirtied any cachelines. Flushing it at job
start then doesn't make any sense.
Signed-off-by: Eric Anholt
Fixes: 57692c94dcbe ("drm/v3d: Introduce a new DRM driver for Broadcom V3D
V3.x+")
---
drivers/gpu/drm/v3d/v3d_gem.c | 12
1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 34103205b7cb..cc4d025b01e0 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -139,22 +139,10 @@ v3d_invalidate_l2(struct v3d_dev *v3d, int core)
V3D_L2CACTL_L2CENA);
}
-static void
-v3d_invalidate_l1td(struct v3d_dev *v3d, int core)
-{
- V3D_CORE_WRITE(core, V3D_CTL_L2TCACTL, V3D_L2TCACTL_TMUWCF);
- if (wait_for(!(V3D_CORE_READ(core, V3D_CTL_L2TCACTL) &
- V3D_L2TCACTL_L2TFLS), 100)) {
- DRM_ERROR("Timeout waiting for L1T write combiner flush\n");
- }
-}
-
/* Invalidates texture L2 cachelines */
static void
v3d_flush_l2t(struct v3d_dev *v3d, int core)
{
- v3d_invalidate_l1td(v3d, core);
-
V3D_CORE_WRITE(core, V3D_CTL_L2TCACTL,
V3D_L2TCACTL_L2TFLS |
V3D_SET_FIELD(V3D_L2TCACTL_FLM_FLUSH, V3D_L2TCACTL_FLM));
--
2.20.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 6/8] arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY
Add 'xo_board' as ref clock for the DSI PHYs, it was previously hardcoded in the PLL 'driver' for the 28nm PHY. Signed-off-by: Matthias Kaehlcke Reviewed-by: Douglas Anderson --- Changes in v3: - added 'Reviewed-by: Douglas Anderson ' tag Changes in v2: - patch added to the series --- arch/arm64/boot/dts/qcom/msm8916.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi index d302d8d639a12..89f30f34ff896 100644 --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi @@ -959,8 +959,9 @@ #clock-cells = <1>; #phy-cells = <0>; - clocks = < GCC_MDSS_AHB_CLK>; - clock-names = "iface"; + clocks = < GCC_MDSS_AHB_CLK>, +<_board>; + clock-names = "iface", "ref"; }; }; -- 2.20.0.rc1.387.gf8505762e3-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/8] dt-bindings: msm/dsi: Add ref clock for PHYs
Allow the PHY drivers to get the ref clock from the DT. Signed-off-by: Matthias Kaehlcke --- Changes in V3: - added note that the ref clock is only required for new DTS files/entries Changes in v2: - add the ref clock for all PHYs, not only the 10nm one - updated commit message --- Documentation/devicetree/bindings/display/msm/dsi.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index dfc743219bd88..9ae9469427207 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -106,6 +106,7 @@ Required properties: - clocks: Phandles to device clocks. See [1] for details on clock bindings. - clock-names: the following clocks are required: * "iface" + * "ref" (only required for new DTS files/entries) For 28nm HPM/LP, 28nm 8960 PHYs: - vddio-supply: phandle to vdd-io regulator device node For 20nm PHY: -- 2.20.0.rc1.387.gf8505762e3-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 7/8] arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs
Add 'bi_tcxo' as ref clock for the DSI PHYs, it was previously
hardcoded in the PLL 'driver' for the 10nm PHY.
Signed-off-by: Matthias Kaehlcke
Reviewed-by: Douglas Anderson
---
based on "[v4,1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file"
(https://patchwork.kernel.org/patch/10666253/)
Changes in v3:
- added 'Reviewed-by: Douglas Anderson ' tag
Changes in v2:
- patch added to the series
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 5728b4cfae269..cdb5a9bb23e69 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1372,8 +1372,9 @@
#clock-cells = <1>;
#phy-cells = <0>;
- clocks = < DISP_CC_MDSS_AHB_CLK>;
- clock-names = "iface";
+ clocks = < DISP_CC_MDSS_AHB_CLK>,
+< RPMH_CXO_CLK>;
+ clock-names = "iface", "ref";
};
dsi1: dsi@ae96000 {
@@ -1434,8 +1435,9 @@
#clock-cells = <1>;
#phy-cells = <0>;
- clocks = < DISP_CC_MDSS_AHB_CLK>;
- clock-names = "iface";
+ clocks = < DISP_CC_MDSS_AHB_CLK>,
+< RPMH_CXO_CLK>;
+ clock-names = "iface", "ref";
};
};
--
2.20.0.rc1.387.gf8505762e3-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 8/8] ARM: dts: qcom-apq8064: Set 'xo_board' as ref clock of the DSI PHY
Add 'xo_board' as ref clock for the DSI PHY, it was previously hardcoded in the PLL 'driver' for the 28nm 8960 PHY. Signed-off-by: Matthias Kaehlcke --- Changes in v3: - patch added to the series --- arch/arm/boot/dts/qcom-apq8064.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi index 48c3cf4276101..d337ae9326cd8 100644 --- a/arch/arm/boot/dts/qcom-apq8064.dtsi +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi @@ -1338,8 +1338,9 @@ <0x04700300 0x200>, <0x04700500 0x5c>; reg-names = "dsi_pll", "dsi_phy", "dsi_phy_regulator"; - clock-names = "iface_clk"; - clocks = < DSI_M_AHB_CLK>; + clock-names = "iface_clk", "ref"; + clocks = < DSI_M_AHB_CLK>, +<_board>; }; -- 2.20.0.rc1.387.gf8505762e3-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/8] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT
Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate. Use default values if the ref
clock is not specified.
Signed-off-by: Matthias Kaehlcke
---
Changes in v3:
- use default name and rate if the ref clock is not specified
in the DT
- store vco_ref_clk_name instead of vco_ref_clk
- fixed check for EPROBE_DEFER
- renamed VCO_REF_CLK_RATE to VCO_REF_CLK_DEFAULT_RATE
Changes in v2:
- patch added to the series
---
.../gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c | 28 +++
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
index 49008451085b8..3af678d3317f6 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
@@ -47,9 +47,9 @@
#define NUM_PROVIDED_CLKS 2
-#define VCO_REF_CLK_RATE 2700
-#define VCO_MIN_RATE 6
-#define VCO_MAX_RATE 12
+#define VCO_REF_CLK_DEFAULT_RATE 2700
+#define VCO_MIN_RATE 6
+#define VCO_MAX_RATE 12
#define DSI_BYTE_PLL_CLK 0
#define DSI_PIXEL_PLL_CLK 1
@@ -75,6 +75,8 @@ struct dsi_pll_28nm {
struct platform_device *pdev;
void __iomem *mmio;
+ const char *vco_ref_clk_name;
+
/* custom byte clock divider */
struct clk_bytediv *bytediv;
@@ -125,7 +127,10 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw *hw,
unsigned long rate,
DBG("rate=%lu, parent's=%lu", rate, parent_rate);
temp = rate / 10;
- val = VCO_REF_CLK_RATE / 10;
+ if (parent_rate)
+ val = parent_rate / 10;
+ else
+ val = VCO_REF_CLK_DEFAULT_RATE / 10;
fb_divider = (temp * VCO_PREF_DIV_RATIO) / val;
fb_divider = fb_divider / 2 - 1;
pll_write(base + REG_DSI_28nm_8960_PHY_PLL_CTRL_1,
@@ -410,7 +415,7 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm)
{
char *clk_name, *parent_name, *vco_name;
struct clk_init_data vco_init = {
- .parent_names = (const char *[]){ "pxo" },
+ .parent_names = _28nm->vco_ref_clk_name,
.num_parents = 1,
.flags = CLK_IGNORE_UNUSED,
.ops = _ops_dsi_pll_28nm_vco,
@@ -494,6 +499,7 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_8960_init(struct
platform_device *pdev,
{
struct dsi_pll_28nm *pll_28nm;
struct msm_dsi_pll *pll;
+ struct clk *vco_ref_clk;
int ret;
if (!pdev)
@@ -506,6 +512,18 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_8960_init(struct
platform_device *pdev,
pll_28nm->pdev = pdev;
pll_28nm->id = id + 1;
+ vco_ref_clk = devm_clk_get(>dev, "ref");
+ if (!IS_ERR(vco_ref_clk)) {
+ pll_28nm->vco_ref_clk_name = __clk_get_name(vco_ref_clk);
+ } else {
+ ret = PTR_ERR(vco_ref_clk);
+ if (ret == -EPROBE_DEFER)
+ return ERR_PTR(ret);
+
+ dev_warn(>dev, "'ref' clock is not specified, using
default name\n");
+ pll_28nm->vco_ref_clk_name = "pxo";
+ }
+
pll_28nm->mmio = msm_ioremap(pdev, "dsi_pll", "DSI_PLL");
if (IS_ERR_OR_NULL(pll_28nm->mmio)) {
dev_err(>dev, "%s: failed to map pll base\n", __func__);
--
2.20.0.rc1.387.gf8505762e3-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 5/8] drm/msm/dsi: 10nm PHY: Get ref clock from the DT
Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.
Note: This change could break old out-of-tree DTS files that
use the 10nm PHY
Signed-off-by: Matthias Kaehlcke
Reviewed-by: Douglas Anderson
---
Changes in v3:
- fixed check for EPROBE_DEFER
- added note to commit message about breaking old DTS files
- added 'Reviewed-by: Douglas Anderson ' tag
Changes in v2:
- remove anonymous array in clk_init_data assignment
- log error code if devm_clk_get() fails
- don't log devm_clk_get() failures for -EPROBE_DEFER
- updated commit message
---
drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
index 4c03f0b7343ed..2d23372acd20d 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
@@ -91,6 +91,7 @@ struct dsi_pll_10nm {
void __iomem *phy_cmn_mmio;
void __iomem *mmio;
+ struct clk *vco_ref_clk;
u64 vco_ref_clk_rate;
u64 vco_current_rate;
@@ -629,8 +630,9 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm)
{
char clk_name[32], parent[32], vco_name[32];
char parent2[32], parent3[32], parent4[32];
+ const char *ref_clk_name = __clk_get_name(pll_10nm->vco_ref_clk);
struct clk_init_data vco_init = {
- .parent_names = (const char *[]){ "xo" },
+ .parent_names = _clk_name,
.num_parents = 1,
.name = vco_name,
.flags = CLK_IGNORE_UNUSED,
@@ -786,6 +788,15 @@ struct msm_dsi_pll *msm_dsi_pll_10nm_init(struct
platform_device *pdev, int id)
pll_10nm->id = id;
pll_10nm_list[id] = pll_10nm;
+ pll_10nm->vco_ref_clk = devm_clk_get(>dev, "ref");
+ if (IS_ERR(pll_10nm->vco_ref_clk)) {
+ ret = PTR_ERR(pll_10nm->vco_ref_clk);
+ if (ret != -EPROBE_DEFER)
+ dev_err(>dev, "couldn't get 'ref' clock: %d\n",
+ ret);
+ return ERR_PTR(ret);
+ }
+
pll_10nm->phy_cmn_mmio = msm_ioremap(pdev, "dsi_phy", "DSI_PHY");
if (IS_ERR_OR_NULL(pll_10nm->phy_cmn_mmio)) {
dev_err(>dev, "failed to map CMN PHY base\n");
--
2.20.0.rc1.387.gf8505762e3-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/8] drm/msm/dsi: 28nm PHY: Get ref clock from the DT
Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate. Use default values if the ref
clock is not specified.
Signed-off-by: Matthias Kaehlcke
---
Changes in v3:
- use default name and rate if the ref clock is not specified
in the DT
- store vco_ref_clk_name instead of vco_ref_clk
- dsi_pll_28nm_clk_set_rate: changed data type of ref_clk_rate to
unsigned long
- fixed check for EPROBE_DEFER
- renamed VCO_REF_CLK_RATE to VCO_REF_CLK_DEFAULT_RATE
Changes in v2:
- patch added to the series
---
drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c | 35 --
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
index 26e3a01a99c2b..4a84c69ca0b2b 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c
@@ -40,7 +40,7 @@
#define NUM_PROVIDED_CLKS 2
-#define VCO_REF_CLK_RATE 1920
+#define VCO_REF_CLK_DEFAULT_RATE 1920
#define VCO_MIN_RATE 35000
#define VCO_MAX_RATE 75000
@@ -81,6 +81,7 @@ struct dsi_pll_28nm {
struct platform_device *pdev;
void __iomem *mmio;
+ const char *vco_ref_clk_name;
int vco_delay;
/* private clocks: */
@@ -139,6 +140,8 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw *hw,
unsigned long rate,
struct dsi_pll_28nm *pll_28nm = to_pll_28nm(pll);
struct device *dev = _28nm->pdev->dev;
void __iomem *base = pll_28nm->mmio;
+ unsigned long ref_clk_rate = parent_rate ?
+ parent_rate : VCO_REF_CLK_DEFAULT_RATE;
unsigned long div_fbx1000, gen_vco_clk;
u32 refclk_cfg, frac_n_mode, frac_n_value;
u32 sdm_cfg0, sdm_cfg1, sdm_cfg2, sdm_cfg3;
@@ -166,17 +169,17 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw *hw,
unsigned long rate,
pll_write(base + REG_DSI_28nm_PHY_PLL_LPFC1_CFG, 0x70);
pll_write(base + REG_DSI_28nm_PHY_PLL_LPFC2_CFG, 0x15);
- rem = rate % VCO_REF_CLK_RATE;
+ rem = rate % ref_clk_rate;
if (rem) {
refclk_cfg = DSI_28nm_PHY_PLL_REFCLK_CFG_DBLR;
frac_n_mode = 1;
- div_fbx1000 = rate / (VCO_REF_CLK_RATE / 500);
- gen_vco_clk = div_fbx1000 * (VCO_REF_CLK_RATE / 500);
+ div_fbx1000 = rate / (ref_clk_rate / 500);
+ gen_vco_clk = div_fbx1000 * (ref_clk_rate / 500);
} else {
refclk_cfg = 0x0;
frac_n_mode = 0;
- div_fbx1000 = rate / (VCO_REF_CLK_RATE / 1000);
- gen_vco_clk = div_fbx1000 * (VCO_REF_CLK_RATE / 1000);
+ div_fbx1000 = rate / (ref_clk_rate / 1000);
+ gen_vco_clk = div_fbx1000 * (ref_clk_rate / 1000);
}
DBG("refclk_cfg = %d", refclk_cfg);
@@ -265,7 +268,8 @@ static unsigned long dsi_pll_28nm_clk_recalc_rate(struct
clk_hw *hw,
void __iomem *base = pll_28nm->mmio;
u32 sdm0, doubler, sdm_byp_div;
u32 sdm_dc_off, sdm_freq_seed, sdm2, sdm3;
- u32 ref_clk = VCO_REF_CLK_RATE;
+ u32 ref_clk = parent_rate ?
+ parent_rate : VCO_REF_CLK_DEFAULT_RATE;
unsigned long vco_rate;
VERB("parent_rate=%lu", parent_rate);
@@ -273,7 +277,7 @@ static unsigned long dsi_pll_28nm_clk_recalc_rate(struct
clk_hw *hw,
/* Check to see if the ref clk doubler is enabled */
doubler = pll_read(base + REG_DSI_28nm_PHY_PLL_REFCLK_CFG) &
DSI_28nm_PHY_PLL_REFCLK_CFG_DBLR;
- ref_clk += (doubler * VCO_REF_CLK_RATE);
+ ref_clk += (doubler * ref_clk);
/* see if it is integer mode or sdm mode */
sdm0 = pll_read(base + REG_DSI_28nm_PHY_PLL_SDM_CFG0);
@@ -518,7 +522,7 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm)
{
char clk_name[32], parent1[32], parent2[32], vco_name[32];
struct clk_init_data vco_init = {
- .parent_names = (const char *[]){ "xo" },
+ .parent_names = _28nm->vco_ref_clk_name,
.num_parents = 1,
.name = vco_name,
.flags = CLK_IGNORE_UNUSED,
@@ -593,6 +597,7 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_init(struct
platform_device *pdev,
{
struct dsi_pll_28nm *pll_28nm;
struct msm_dsi_pll *pll;
+ struct clk *vco_ref_clk;
int ret;
if (!pdev)
@@ -605,6 +610,18 @@ struct msm_dsi_pll *msm_dsi_pll_28nm_init(struct
platform_device *pdev,
pll_28nm->pdev = pdev;
pll_28nm->id = id;
+ vco_ref_clk = devm_clk_get(>dev, "ref");
+ if (!IS_ERR(vco_ref_clk)) {
+ pll_28nm->vco_ref_clk_name = __clk_get_name(vco_ref_clk);
+ } else {
+ ret = PTR_ERR(vco_ref_clk);
+ if (ret == -EPROBE_DEFER)
+ ERR_PTR(ret);
+
+
[PATCH v3 0/8] drm/msm/dsi: Get PHY ref clocks from the DT
The MSM DSI PHY drivers currently hardcode the name and the rate of the PHY ref clock. Get the ref clock from the device tree instead. Note: testing of this series was limited to SDM845 and the 10nm PHY Major changes in v3: - keep supporting DTs without ref clock for the 28nm and the 28nm 8960 PHYs - added patch to add ref clock to qcom-apq8064.dtsi Major changes in v2: - apply to all MSM DSI PHY drivers, not only 10nm Matthias Kaehlcke (8): dt-bindings: msm/dsi: Add ref clock for PHYs drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT drm/msm/dsi: 28nm PHY: Get ref clock from the DT drm/msm/dsi: 14nm PHY: Get ref clock from the DT drm/msm/dsi: 10nm PHY: Get ref clock from the DT arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs ARM: dts: qcom-apq8064: Set 'xo_board' as ref clock of the DSI PHY .../devicetree/bindings/display/msm/dsi.txt | 1 + arch/arm/boot/dts/qcom-apq8064.dtsi | 5 +-- arch/arm64/boot/dts/qcom/msm8916.dtsi | 5 +-- arch/arm64/boot/dts/qcom/sdm845.dtsi | 10 +++--- drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c| 13 ++- drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c| 16 +++-- drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c| 35 ++- .../gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c | 28 --- 8 files changed, 87 insertions(+), 26 deletions(-) -- 2.20.0.rc1.387.gf8505762e3-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/8] drm/msm/dsi: 14nm PHY: Get ref clock from the DT
Get the ref clock of the PHY from the device tree instead of
hardcoding its name and rate.
Note: This change could break old out-of-tree DTS files that
use the 14nm PHY.
Signed-off-by: Matthias Kaehlcke
Reviewed-by: Douglas Anderson
---
Changes in v3:
- fixed check for EPROBE_DEFER
- added note to commit message about breaking old DTS files
- added 'Reviewed-by: Douglas Anderson ' tag
Changes in v2:
- patch added to the series
---
drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
index 71fe60e5f01f1..032bf3e8614bd 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
@@ -40,7 +40,6 @@
#define NUM_PROVIDED_CLKS 2
-#define VCO_REF_CLK_RATE 1920
#define VCO_MIN_RATE 13UL
#define VCO_MAX_RATE 26UL
@@ -139,6 +138,7 @@ struct dsi_pll_14nm {
/* protects REG_DSI_14nm_PHY_CMN_CLK_CFG0 register */
spinlock_t postdiv_lock;
+ struct clk *vco_ref_clk;
u64 vco_current_rate;
u64 vco_ref_clk_rate;
@@ -591,7 +591,7 @@ static int dsi_pll_14nm_vco_set_rate(struct clk_hw *hw,
unsigned long rate,
parent_rate);
pll_14nm->vco_current_rate = rate;
- pll_14nm->vco_ref_clk_rate = VCO_REF_CLK_RATE;
+ pll_14nm->vco_ref_clk_rate = parent_rate;
dsi_pll_14nm_input_init(pll_14nm);
@@ -950,8 +950,9 @@ static struct clk_hw *pll_14nm_postdiv_register(struct
dsi_pll_14nm *pll_14nm,
static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm)
{
char clk_name[32], parent[32], vco_name[32];
+ const char *ref_clk_name = __clk_get_name(pll_14nm->vco_ref_clk);
struct clk_init_data vco_init = {
- .parent_names = (const char *[]){ "xo" },
+ .parent_names = _clk_name,
.num_parents = 1,
.name = vco_name,
.flags = CLK_IGNORE_UNUSED,
@@ -1065,6 +1066,15 @@ struct msm_dsi_pll *msm_dsi_pll_14nm_init(struct
platform_device *pdev, int id)
pll_14nm->id = id;
pll_14nm_list[id] = pll_14nm;
+ pll_14nm->vco_ref_clk = devm_clk_get(>dev, "ref");
+ if (IS_ERR(pll_14nm->vco_ref_clk)) {
+ ret = PTR_ERR(pll_14nm->vco_ref_clk);
+ if (ret != -EPROBE_DEFER)
+ dev_err(>dev, "couldn't get 'ref' clock: %d\n",
+ ret);
+ return ERR_PTR(ret);
+ }
+
pll_14nm->phy_cmn_mmio = msm_ioremap(pdev, "dsi_phy", "DSI_PHY");
if (IS_ERR_OR_NULL(pll_14nm->phy_cmn_mmio)) {
dev_err(>dev, "failed to map CMN PHY base\n");
--
2.20.0.rc1.387.gf8505762e3-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 07/11] drm/i915/psr: Check if resolution is supported by default SU granularity
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
> Selective updates have a default granularity requirements as stated
> by eDP spec
Needs reference to the location in the spec.
> , so check if HW can match those requirements before
> enable PSR2.
typo: enabling*
>
> Cc: Dhinakaran Pandiyan
> Cc: Rodrigo Vivi
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/intel_psr.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index c4a8f476eea9..282ff1bc68a7 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -539,6 +539,18 @@ static bool intel_psr2_config_valid(struct
> intel_dp *intel_dp,
> return false;
> }
>
> + /* HW will always send full lines in SU blocks, so X will
s/X/starting X coordinate
> + * always be 0 and we only need to check the width to validate
> + * horizontal granularity.
> + * About vertical granularity HW works by SU blocks starting
> + * at each 4 lines with height of 4 lines, what eDP states
> + * that sink should support.
How about rewriting this as -
"HW sends SU blocks of size four scan lines, which means the starting X
coordinate and Y granularity requirements will always be met. We only
need to validate the SU block width is a multiple of 4."?
> + */
> + if (crtc_hdisplay % 4) {
> + DRM_DEBUG_KMS("PSR2 not enabled, default SU granularity
> not match\n");
"PSR2 not enabled, hdisplay(%d) not multiple of 4\n"
With nits addressed,
Reviewed-by: Dhinakaran Pandiyan
> + return false;
> + }
> +
> return true;
> }
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH v2 5/5] drm/msm: subclass work object for vblank events
On 2018-11-30 12:07, Sean Paul wrote:
On Fri, Nov 30, 2018 at 11:45:55AM -0800, Jeykumar Sankaran wrote:
On 2018-11-29 14:15, Sean Paul wrote:
> On Tue, Nov 20, 2018 at 02:04:14PM -0800, Jeykumar Sankaran wrote:
> > On 2018-11-07 07:55, Sean Paul wrote:
> > > On Tue, Nov 06, 2018 at 02:36:30PM -0800, Jeykumar Sankaran wrote:
> > > > msm maintains a separate structure to define vblank
> > > > work definitions and a list to track events submitted
> > > > to the workqueue. We can avoid this redundant list
> > > > and its protection mechanism, if we subclass the
> > > > work object to encapsulate vblank event parameters.
> > > >
> > > > changes in v2:
> > > > - subclass optimization on system wq (Sean Paul)
> > >
> > > I wouldn't do it like this, tbh. One problem is that you've lost
your
> > > flush() on
> > > unbind, so there's no way to know if you have workers in the wild
> > > waiting
> > > to
> > > enable/disable vblank.
> > >
> > > Another issues is that AFAICT, we don't need a queue of
> > > enables/disables,
> > > but
> > > rather just the last requested state (ie: should we be on or off).
So
> > > things
> > > don't need to be this complicated (and we're possibly thrashing
vblank
> > > on/off
> > > for no reason).
> > >
> > > I'm still of the mind that you should just make this synchronous
and
> be
> > > done
> > > with the threads (especially since we're still
uncovering/introducing
> > > races!).
> > >
> > While scoping out the effort to make vblank events synchronous, I
> > found
> > that the spinlock locking order of vblank request sequence and
vblank
> > callback
> > sequences are the opposite.
> >
> > In DPU, drm_vblank_enable acquires vblank_time_lock before
registering
> > the crtc to encoder which happens after acquiring encoder_spinlock.
> > But
> > the vblank_callback acquires encoder_spinlock before accessing the
> > registered
> > crtc and calling into drm_vblank_handler which tries to acquire
> > vblank_time_lock.
> > Acquiring both vblank_time_lock and encoder_spinlock in the same
> > thread
> > is leading to deadlock.
>
> Hmm, I'm not sure I follow. Are you seeing issues where irq overlaps
> with
> enable/disable? I hacked in sync vblank enable/disable quickly to see
if
> I
> could
> reproduce what you're seeing, but things seemed well behaved.
>
The race is between drm_vblank_get/put and vblank_handler contexts.
When made synchronous:
while calling drm_vblank_get, the callstack looks like below:
drm_vblank_get -> drm_vblank_enable (acquires vblank_time_lock) ->
__enable_vblank -> dpu_crtc_vblank ->
dpu_encoder_toggle_vblank_for_crtc
(tries to acquire enc_spinlock)
In vblank handler, the call stack will be:
dpu_encoder_phys_vid_vblank_irq -> dpu_encoder_vblank_callback
(acquires
enc_spinlock) -> dpu_crtc_vblank_callback -> drm_handle_vblank (tries
to
acquire vblank_time_lock)
Hmm, I'm not sure how this can happen. We acquire and release the
enc_spinlock
before enabling the irq, yes we will hold on to the vbl_time_lock, but
we
shouldn't be trying to reacquire an encoder's spinlock after we've
enabled
it.
In the synchronous approach dpu_encoder_toggle_vblank_for_crtc(which
acquires the enc_spinlock) will be called while we
are holding the vbl_time_lock.
I don't know how that can deadlock, since we should never be running
enable and
the handler concurrently.
I agree that vblank_irq handler should not be running before the enable
sequence. But
don't you expect the handler to be running while calling the
vblank_disable sequence?
vbl disable will try to acquire the locks in the opposite order to that
of irq_handler and the
same issue is bound to happen.
With your patch, you should be able to simulate this deadlock if you can
inject a delay
by adding a pr_err log in vblank_ctrl_queue_work
Thanks,
Jeykumar S.
The only thing I can think of is that the vblank interrupts are firing
after
vblank has been disabled? In that case, it seems like we should
properly
flush
them.
Sean
> I do see that there is a chance to call drm_handle_vblank() while
> holding
> enc_spinlock, but couldn't find any obvious lock recursion there.
>
> Maybe a callstack or lockdep splat would help?
>
> Sean
>
>
> Here's my hack to bypass the display thread:
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c
> b/drivers/gpu/drm/msm/msm_drv.c
> index 9c9f7ff6960b38..5a3cac5825319e 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -242,24 +242,19 @@ static void vblank_ctrl_worker(struct
kthread_work
> *work)
> static int vblank_ctrl_queue_work(struct msm_drm_private *priv,
>int crtc_id, bool enable)
> {
> + struct msm_kms *kms = priv->kms;
>struct msm_vblank_ctrl *vbl_ctrl = >vblank_ctrl;
> - struct vblank_event *vbl_ev;
>unsigned long flags;
>
> - vbl_ev = kzalloc(sizeof(*vbl_ev), GFP_ATOMIC);
> - if (!vbl_ev)
> - return -ENOMEM;
> + spin_lock_irqsave(_ctrl->lock, flags);
>
Re: [PATCH v2 06/11] drm: Add the PSR SU granularity registers offsets
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote: > Source is required to comply to sink SU granularity when > DP_PSR2_SU_GRANULARITY_REQUIRED is set in DP_PSR_CAPS, > so adding the registers offsets. > > v2: Also adding DP_PSR2_SU_Y_GRANULARITY(Rodrigo) > > Cc: Dhinakaran Pandiyan > Cc: Rodrigo Vivi > Signed-off-by: José Roberto de Souza > --- > include/drm/drm_dp_helper.h | 4 > 1 file changed, 4 insertions(+) > > diff --git a/include/drm/drm_dp_helper.h > b/include/drm/drm_dp_helper.h > index 047314ce25d6..0e04b2db3dde 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -314,6 +314,10 @@ > # define DP_PSR_SETUP_TIME_SHIFT1 > # define DP_PSR2_SU_Y_COORDINATE_REQUIRED (1 << 4) /* eDP 1.4a */ > # define DP_PSR2_SU_GRANULARITY_REQUIRED(1 << 5) /* eDP 1.4b */ > + > +#define DP_PSR2_SU_X_GRANULARITY 0x072 /* eDP 1.4b */ > +#define DP_PSR2_SU_Y_GRANULARITY 0x074 /* eDP 1.4b */ Definitions above use spaces instead of tabs, so it'd have been good to be consistent. But, there are places in the file where tabs are used too, so will leave it to you if you want to switch. > + Verified against eDP spec 1.4b Reviewed-by: Dhinakaran Pandiyan > /* > * 0x80-0x8f describe downstream port capabilities, but there are > two layouts > * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set. If it > was not, ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 04/11] drm/i915/psr: Enable sink to trigger a interruption on PSR2 CRC mismatch
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
> eDP spec states 2 different bits to enable sink to trigger a
> interruption when there is a CRC mismatch.
> DP_PSR_CRC_VERIFICATION is for PSR only and
> DP_PSR_IRQ_HPD_WITH_CRC_ERRORS is for PSR2 only.
With PSR short pulse handling implemented, I think we are ready for
this.
Reviewed-by: Dhinakaran Pandiyan
>
> Cc: Dhinakaran Pandiyan
> Reviewed-by: Rodrigo Vivi
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/intel_psr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index b04472e637c8..77162c469079 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -394,7 +394,7 @@ static void intel_psr_enable_sink(struct intel_dp
> *intel_dp)
> if (dev_priv->psr.psr2_enabled) {
> drm_dp_dpcd_writeb(_dp->aux,
> DP_RECEIVER_ALPM_CONFIG,
> DP_ALPM_ENABLE);
> - dpcd_val |= DP_PSR_ENABLE_PSR2;
> + dpcd_val |= DP_PSR_ENABLE_PSR2 |
> DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
> } else {
> if (dev_priv->psr.link_standby)
> dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 03/11] drm/i915/psr: Set PSR CRC verification bit in sink inside PSR1 block
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
> As we have a else block for the 'if (dev_priv->psr.psr2_enabled) {'
> and this bit is only set for PSR1 move it to that block to make it
> more easy to read.
>
> Cc: Dhinakaran Pandiyan
> Cc: Rodrigo Vivi
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/intel_psr.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 8515f4a6f4f1..b04472e637c8 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -398,10 +398,11 @@ static void intel_psr_enable_sink(struct
> intel_dp *intel_dp)
> } else {
> if (dev_priv->psr.link_standby)
> dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> +
> + if (INTEL_GEN(dev_priv) >= 8)
> + dpcd_val |= DP_PSR_CRC_VERIFICATION;
> }
>
> - if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> - dpcd_val |= DP_PSR_CRC_VERIFICATION;
> drm_dp_dpcd_writeb(_dp->aux, DP_PSR_EN_CFG, dpcd_val);
>
> drm_dp_dpcd_writeb(_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
Do we need this DPCD write? The panel should already be awake by this
point, I think it's worth removing it if there's no regression.
Your change in this patch looks good, so
Reviewed-by: Dhinakaran Pandiyan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 02/11] drm/i915/psr: Don't tell sink that main link will be active while is active PSR2
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
> For PSR2 there is no register to tell HW to keep main link enabled
Right, there is no bit in PSR2_CTL
Reviewed-by: Dhinakaran Pandiyan
> while PSR2 is active, so don't configure sink DPCD with a
> misleading value.
>
> v2: Moving the set of DP_PSR_CRC_VERIFICATION to the else block
> of 'if (dev_priv->psr.psr2_enabled)' to another patch. (Rodrigo)
>
> Cc: Dhinakaran Pandiyan
> Cc: Rodrigo Vivi
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/intel_psr.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 40ca6cc43cc4..8515f4a6f4f1 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -395,10 +395,11 @@ static void intel_psr_enable_sink(struct
> intel_dp *intel_dp)
> drm_dp_dpcd_writeb(_dp->aux,
> DP_RECEIVER_ALPM_CONFIG,
> DP_ALPM_ENABLE);
> dpcd_val |= DP_PSR_ENABLE_PSR2;
> + } else {
> + if (dev_priv->psr.link_standby)
> + dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> }
>
> - if (dev_priv->psr.link_standby)
> - dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> dpcd_val |= DP_PSR_CRC_VERIFICATION;
> drm_dp_dpcd_writeb(_dp->aux, DP_PSR_EN_CFG, dpcd_val);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 01/11] drm/i915: Disable PSR in Apple panels
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
> i915 yet don't support PSR in Apple panels, so lets keep it disabled
> while we work on that.
>
> v2: Renamed DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED to
> DP_DPCD_QUIRK_NO_PSR (Ville)
>
> Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW)
> Cc: Ville Syrjälä
> Cc: Rodrigo Vivi
> Cc: Dhinakaran Pandiyan
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/drm_dp_helper.c | 2 ++
> drivers/gpu/drm/i915/intel_psr.c | 6 ++
> include/drm/drm_dp_helper.h | 1 +
> 3 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index 2d6c491a0542..b00fd5ced0a0 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1273,6 +1273,8 @@ static const struct dpcd_quirk
> dpcd_quirk_list[] = {
> { OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true,
> BIT(DP_DPCD_QUIRK_CONSTANT_N) },
> /* LG LP140WF6-SPM1 eDP panel */
> { OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r',
> 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
> + /* Apple panels needs some additional handling to support PSR
> */
> + { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false,
> BIT(DP_DPCD_QUIRK_NO_PSR) }
> };
>
> #undef OUI
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 2084784f320d..40ca6cc43cc4 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -278,6 +278,12 @@ void intel_psr_init_dpcd(struct intel_dp
> *intel_dp)
> DRM_DEBUG_KMS("Panel lacks power state control, PSR
> cannot be enabled\n");
> return;
> }
> +
> + if (drm_dp_has_quirk(_dp->desc, DP_DPCD_QUIRK_NO_PSR)) {
> + DRM_DEBUG_KMS("PSR support not currently available for
> this panel\n");
> + return;
> + }
> +
> dev_priv->psr.sink_support = true;
> dev_priv->psr.sink_sync_latency =
> intel_dp_get_sink_sync_latency(intel_dp);
> diff --git a/include/drm/drm_dp_helper.h
> b/include/drm/drm_dp_helper.h
> index 5736c942c85b..047314ce25d6 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1365,6 +1365,7 @@ enum drm_dp_quirk {
>* to 16 bits. So will give a constant value (0x8000) for
> compatability.
>*/
> DP_DPCD_QUIRK_CONSTANT_N,
nit: Documentation missing here. I guess we need something along the
lines of "PSR not supported" without referring to the specific DP
device. With that,
Reviewed-by: Dhinakaran Pandiyan
> + DP_DPCD_QUIRK_NO_PSR,
> };
>
> /**
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 02:40:19PM -0800, Jarkko Sakkinen wrote: > Got you... Well I now read the 2nd amendment now through, and yeah, kind > of way I work/function anyway. Ugh, looked up the word from dictionary for something that makes additions to some guidelines because did not know the english word. Not meant as a political reference of any kind. Just don't know any better English word. /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] of: Add a GitLab CI config file for unittests
On Fri, Nov 30, 2018 at 11:22 PM Rob Herring wrote: > > On Fri, Nov 30, 2018 at 2:53 PM Daniel Vetter wrote: > > > > On Fri, Nov 30, 2018 at 9:32 PM Rob Herring wrote: > > > > > > This adds a GitLab CI config file running the DT unittest in a usermode > > > Linux build. The corresponding CI job can be found here: > > > > > > https://gitlab.com/robherring/linux-dt-unittest/pipelines > > > > > > This CI job can be duplicated by others by creating a kernel repo on a > > > GitLab instance and configuring GitLab CI to use > > > drivers/of/.gitlab-ci.yml config file. > > > > > > Cc: Frank Rowand > > > Cc: Daniel Vetter > > > Signed-off-by: Rob Herring > > > > Adding dri-devel. > > > > > --- > > > drivers/of/.gitlab-ci.yml | 18 ++ > > > 1 file changed, 18 insertions(+) > > > create mode 100644 drivers/of/.gitlab-ci.yml > > > > > > diff --git a/drivers/of/.gitlab-ci.yml b/drivers/of/.gitlab-ci.yml > > > new file mode 100644 > > > index ..44a4824f5c33 > > > --- /dev/null > > > +++ b/drivers/of/.gitlab-ci.yml > > > @@ -0,0 +1,18 @@ > > > +# SPDX-License-Identifier: GPL-2.0+ > > > + > > > +image: registry.gitlab.com/robherring/docker-images/ubuntu-kernel-build > > > > I think it's better to include the docker recipe too. One because > > shipping pre-built dockers is a license nightmare, second because > > better hackability. For a full on example see: > > > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/.gitlab-ci.yml > > Humm, but in the end you are still publishing the docker images either > way. It's just a difference of publishing across projects and whether > it's 2 jobs or 1 job with 2 stages. Yeah, but it comes with sources for rebuilding the image :-) > I do tend to prefer the Travis-CI simplicity of just listing packages > to install. It seems like the same could be done with docker and some > magic on the server side. At least for the simple case of 'install > this list of packages'. Yeah, gitlab is a bunch more explicit, and you need to do your caching of the setup you want yourself. > > For the kernel I guess the question is where we should put all the > > docker files. There's going to be some need for subsystem specific > > tooling (e.g. we want something that has igt installed eventually for > > anything drm), so maybe just keeping them in subsystem directories is > > best. Was at least my plan with drivers/gpu. > > My plan (to the extent I have one) was to put the common parts into > the docker image and leave the small job specific things in the job > setup. Then I can use the same docker image across jobs. Maybe that's > a pointless goal in the docker world. We do that too. The different docker images we have is for different testing (new/old distros, and the cross build chain in userspace conflicts, so need separate docker images for that). Aside from those cases where we need different images because they're incompatible, we do reuse them. For the kernel there's much less of these userspace problems, so one image should be good enough for all jobs. -Daniel > My dockerfile is simple enough, I could probably just create it within > the .gitlab-ci.yml. > > Rob -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v8 3/7] mm, devm_memremap_pages: Fix shutdown handling
On Fri, Nov 30, 2018 at 2:34 PM Logan Gunthorpe wrote: > > > > On 2018-11-30 3:28 p.m., Dan Williams wrote: > > On Fri, Nov 30, 2018 at 2:19 PM Logan Gunthorpe wrote: > >> > >> Hey, > >> > >> On 2018-11-29 11:51 a.m., Dan Williams wrote: > >>> Got it, let me see how bad moving arch_remove_memory() turns out, > >>> sounds like a decent approach to coordinate multiple users of a single > >>> ref. > >> > >> I've put together a patch set[1] that fixes all the users of > >> devm_memremap_pages() without moving arch_remove_memory(). It's pretty > >> clean except for the p2pdma case which is fairly tricky but I don't > >> think there's an easy way around that. > > > > The solution I'm trying is to introduce a devm_memremap_pages_remove() > > that each user can call after they have called percpu_ref_exit(), it's > > just crashing for me currently... > > Ok, that's probably less of a clean up for other users, but sounds like > it would be less tricky for p2pdma. I'd have to create a list of all > pgmaps, but that's not so hard and doesn't create any nasty races to > consider like my current solution. > > >> If you come up with a better solution that's great, otherwise let me > >> know and I'll do some clean up and more testing and send this set to the > >> lists. Though, we might need to wait for your patch to land before we > >> can properly send the fix to it (the first patch in my series)... > > > > I'd say go ahead and send it. We can fix p2pdma as a follow-on. Send > > it to Andrew as a patch relative to the current -next tree. > > Ok, though, how do I reference the current patch in Andrew's tree? Or > does it matter? I would just let Andrew know that this applies incrementally to "mm-hmm-mark-hmm_devmem_add-add_resource-export_symbol_gpl.patch" in his tree. You can't specify Fixes: tags for pending patches in -mm. Andrew may choose to squash the change into the existing patch, which may be the best outcome for not exposing a bisect regression point for p2pdma. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 02:30:45PM -0800, James Bottomley wrote: > On Fri, 2018-11-30 at 14:26 -0800, Jarkko Sakkinen wrote: > > On Fri, Nov 30, 2018 at 03:14:59PM -0700, Jonathan Corbet wrote: > [...] > > > Have you read Documentation/process/code-of-conduct- > > > interpretation.rst? > > > As has been pointed out, it contains a clear answer to how things > > > should be interpreted here. > > > > Ugh, was not aware that there two documents. > > > > Yeah, definitely sheds light. Why the documents could not be merged > > to single common sense code of conduct? > > The fact that we've arrived at essentially an original CoC > reinterpreted to the point where it's effectively a new CoC has been > the source of much debate and recrimination over the last few months > ... you can read it in the ksummit-discuss archives, but I really think > we don't want to reopen that can of worms. Got you... Well I now read the 2nd amendment now through, and yeah, kind of way I work/function anyway. Thank you for the patience... /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/7] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT
On Tue, Nov 27, 2018 at 10:00:50PM -0800, Doug Anderson wrote:
> Hi,
>
> On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke wrote:
> > @@ -409,8 +410,9 @@ static void dsi_pll_28nm_destroy(struct msm_dsi_pll
> > *pll)
> > static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm)
> > {
> > char *clk_name, *parent_name, *vco_name;
> > + const char *ref_clk_name = __clk_get_name(pll_28nm->vco_ref_clk);
>
> IMO for the 28nm PHY driver you should probably make things work OK
> even if the "ref" clock wasn't supplied. In the spirit of the stable
> device tree it would be nice (even if nobody actually ships device
> trees separate from kernels). ...and also it makes the whole thing
> easier to land. If you add compatibility here then the code and
> device tree patch can go in separately.
Ok, I'll make it fall back to the 'default' values if the ref clock is
not specified.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 2018-11-30 at 14:26 -0800, Jarkko Sakkinen wrote: > On Fri, Nov 30, 2018 at 03:14:59PM -0700, Jonathan Corbet wrote: [...] > > Have you read Documentation/process/code-of-conduct- > > interpretation.rst? > > As has been pointed out, it contains a clear answer to how things > > should be interpreted here. > > Ugh, was not aware that there two documents. > > Yeah, definitely sheds light. Why the documents could not be merged > to single common sense code of conduct? The fact that we've arrived at essentially an original CoC reinterpreted to the point where it's effectively a new CoC has been the source of much debate and recrimination over the last few months ... you can read it in the ksummit-discuss archives, but I really think we don't want to reopen that can of worms. James ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 02:26:05PM -0800, Jarkko Sakkinen wrote: > On Fri, Nov 30, 2018 at 03:14:59PM -0700, Jonathan Corbet wrote: > > On Fri, 30 Nov 2018 14:12:19 -0800 > > Jarkko Sakkinen wrote: > > > > > As a maintainer myself (and based on somewhat disturbed feedback from > > > other maintainers) I can only make the conclusion that nobody knows what > > > the responsibility part here means. > > > > > > I would interpret, if I read it like at lawyer at least, that even for > > > existing code you would need to do the changes postmorterm. > > > > > > Is this wrong interpretation? Should I conclude that I made a mistake > > > by reading the CoC and trying to understand what it *actually* says? > > > After this discussion, I can say that I understand it less than before. > > > > Have you read Documentation/process/code-of-conduct-interpretation.rst? > > As has been pointed out, it contains a clear answer to how things should > > be interpreted here. > > Ugh, was not aware that there two documents. > > Yeah, definitely sheds light. Why the documents could not be merged to > single common sense code of conduct? I.e. if the latter that you pointed out tells you what you actually should do what value does the former bring? Just looked up archives and realized that there has been whole alot of CoC related discussions. No wonder this is seen as waste of time. /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v8 3/7] mm, devm_memremap_pages: Fix shutdown handling
On Fri, Nov 30, 2018 at 2:19 PM Logan Gunthorpe wrote: > > Hey, > > On 2018-11-29 11:51 a.m., Dan Williams wrote: > > Got it, let me see how bad moving arch_remove_memory() turns out, > > sounds like a decent approach to coordinate multiple users of a single > > ref. > > I've put together a patch set[1] that fixes all the users of > devm_memremap_pages() without moving arch_remove_memory(). It's pretty > clean except for the p2pdma case which is fairly tricky but I don't > think there's an easy way around that. The solution I'm trying is to introduce a devm_memremap_pages_remove() that each user can call after they have called percpu_ref_exit(), it's just crashing for me currently... > If you come up with a better solution that's great, otherwise let me > know and I'll do some clean up and more testing and send this set to the > lists. Though, we might need to wait for your patch to land before we > can properly send the fix to it (the first patch in my series)... I'd say go ahead and send it. We can fix p2pdma as a follow-on. Send it to Andrew as a patch relative to the current -next tree. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 2018-11-30 at 14:12 -0800, Jarkko Sakkinen wrote: [...] > I pasted this already to another response and this was probably the > part that ignited me to send the patch set (was a few days ago, so > had to revisit to find the exact paragraph): I replied in to the other thread. > "Maintainers have the right and responsibility to remove, edit, or > reject comments, commits, code, wiki edits, issues, and other > contributions that are not aligned to this Code of Conduct, or to ban > temporarily or permanently any contributor for other behaviors that > they deem inappropriate, threatening, offensive, or harmful." > > The whole patch set is neither a joke/troll nor something I would > necessarily want to be include myself. It does have the RFC tag. > > As a maintainer myself (and based on somewhat disturbed feedback from > other maintainers) I can only make the conclusion that nobody knows > what the responsibility part here means. > > I would interpret, if I read it like at lawyer at least, that even > for existing code you would need to do the changes postmorterm. That's wrong in the light of the interpretation document, yes. > Is this wrong interpretation? Should I conclude that I made a > mistake by reading the CoC and trying to understand what it > *actually* says? You can't read it in isolation, you need to read it along with the interpretation document. The latter was created precisely because there was a lot of push back on interpretation problems and ambiguities with the original CoC and it specifically covers this case (and a lot of others). James > After this discussion, I can say that I understand it less than > before. > > /Jarkko > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 03:14:59PM -0700, Jonathan Corbet wrote: > On Fri, 30 Nov 2018 14:12:19 -0800 > Jarkko Sakkinen wrote: > > > As a maintainer myself (and based on somewhat disturbed feedback from > > other maintainers) I can only make the conclusion that nobody knows what > > the responsibility part here means. > > > > I would interpret, if I read it like at lawyer at least, that even for > > existing code you would need to do the changes postmorterm. > > > > Is this wrong interpretation? Should I conclude that I made a mistake > > by reading the CoC and trying to understand what it *actually* says? > > After this discussion, I can say that I understand it less than before. > > Have you read Documentation/process/code-of-conduct-interpretation.rst? > As has been pointed out, it contains a clear answer to how things should > be interpreted here. Ugh, was not aware that there two documents. Yeah, definitely sheds light. Why the documents could not be merged to single common sense code of conduct? /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices
On Wed, Nov 28, 2018 at 07:46:06PM +, Ho, Kenny wrote: > > On Wed, Nov 28, 2018 at 4:14 AM Joonas Lahtinen > wrote: > > So we can only choose the lowest common denominator, right? > > > > Any core count out of total core count should translate nicely into a > > fraction, so what would be the problem with percentage amounts? > > I don't think having an abstracted resource necessarily equate > 'lowest'. The issue with percentage is the lack of precision. If you > look at cpuset cgroup, you can see the specification can be very > precise: > > # /bin/echo 1-4,6 > cpuset.cpus -> set cpus list to cpus 1,2,3,4,6 > (https://www.kernel.org/doc/Documentation/cgroup-v1/cpusets.txt) > > The driver can translate something like this to core count and then to > percentage and handle accordingly while the reverse is not possible. > (You can't tell which set of CUs/EUs a user want from a percentage > request.) It's also not clear to me, from > user/application/admin/resource management perspective, how the base > core counts of a GPU is relevant to the workload (since percentage is > a 'relative' quantity.) For example, let say a workload wants to use > 256 'cores', does it matter if that workload is put on a GPU with 1024 > cores or a GPU with 4096 cores total? > > I am not dismissing the possible need for percentage. I just think > there should be a way to accommodate more than just the 'lowest'. > As you noted, your proposal is similar to the cgroup-v1 "cpuset" controller, which is sort of a way of partitioning your underlying hardware resources; I think Joonas is describing something closer in design to the cgroup-v2 "cpu" controller, which partitions the general time/usage allocated to via cgroup; afaiu, "cpu" doesn't really care which specific core the tasks run on, just the relative weights that determine how much time they get to run on any of the cores. It sounds like with your hardware, your kernel driver is able to specify exactly which subset of GPU EU's a specific GPU context winds up running on. However I think there are a lot of platforms that don't allow that kind of low-level control. E.g., I don't think we can do that on Intel hardware; we have a handful of high-level GPU engines that we can submit different types of batchbuffers to (render, blitter, media, etc.). What we can do is use GPU preemption to limit how much time specific GPU contexts get to run on the render engine before the engine is reclaimed for use by a different context. Using a %gputime approach like Joonas is suggesting could be handled in a driver by reserving specific subsets of EU's on hardware like yours that's capable of doing that, whereas it could be mostly handled on other types of hardware via GPU engine preemption. I think either approach "gpu_euset" or "%gputime" should map well to a cgroup controller implementation. Granted, neither one solves the specific use case I was working on earlier this year where we need unfair (starvation-okay) scheduling that will run contexts strictly according to priority (i.e., lower priority contexts will never run at all unless all higher priority contexts have completed all of their submitted work), but that's a pretty specialized use case that we'll probably need to handle in a different manner anyway. Matt > Regards, > Kennny > > > > > > That combined with the "GPU memory usable" property should be a good > > > > starting point to start subdividing the GPU resources for multiple > > > > users. > > > > > > > > Regards, Joonas > > > > > > > > > > > > > > Your feedback is highly appreciated. > > > > > > > > > > Best Regards, > > > > > Harish > > > > > > > > > > > > > > > > > > > > From: amd-gfx on behalf of > > > > > Tejun Heo > > > > > Sent: Tuesday, November 20, 2018 5:30 PM > > > > > To: Ho, Kenny > > > > > Cc: cgro...@vger.kernel.org; intel-...@lists.freedesktop.org; > > > > > y2ke...@gmail.com; amd-...@lists.freedesktop.org; > > > > > dri-devel@lists.freedesktop.org > > > > > Subject: Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor > > > > > specific DRM devices > > > > > > > > > > > > > > > Hello, > > > > > > > > > > On Tue, Nov 20, 2018 at 10:21:14PM +, Ho, Kenny wrote: > > > > > > By this reply, are you suggesting that vendor specific resources > > > > > > will never be acceptable to be managed under cgroup? Let say a user > > > > > > > > > > I wouldn't say never but whatever which gets included as a cgroup > > > > > controller should have clearly defined resource abstractions and the > > > > > control schemes around them including support for delegation. AFAICS, > > > > > gpu side still seems to have a long way to go (and it's not clear > > > > > whether that's somewhere it will or needs to end up). > > > > > > > > > > > want to have similar functionality as what cgroup is offering but to > > > > > > manage vendor specific resources, what would you suggest as a > > > > > > solution? When you say keeping vendor specific
Re: [RFC PATCH] of: Add a GitLab CI config file for unittests
On Fri, Nov 30, 2018 at 2:53 PM Daniel Vetter wrote: > > On Fri, Nov 30, 2018 at 9:32 PM Rob Herring wrote: > > > > This adds a GitLab CI config file running the DT unittest in a usermode > > Linux build. The corresponding CI job can be found here: > > > > https://gitlab.com/robherring/linux-dt-unittest/pipelines > > > > This CI job can be duplicated by others by creating a kernel repo on a > > GitLab instance and configuring GitLab CI to use > > drivers/of/.gitlab-ci.yml config file. > > > > Cc: Frank Rowand > > Cc: Daniel Vetter > > Signed-off-by: Rob Herring > > Adding dri-devel. > > > --- > > drivers/of/.gitlab-ci.yml | 18 ++ > > 1 file changed, 18 insertions(+) > > create mode 100644 drivers/of/.gitlab-ci.yml > > > > diff --git a/drivers/of/.gitlab-ci.yml b/drivers/of/.gitlab-ci.yml > > new file mode 100644 > > index ..44a4824f5c33 > > --- /dev/null > > +++ b/drivers/of/.gitlab-ci.yml > > @@ -0,0 +1,18 @@ > > +# SPDX-License-Identifier: GPL-2.0+ > > + > > +image: registry.gitlab.com/robherring/docker-images/ubuntu-kernel-build > > I think it's better to include the docker recipe too. One because > shipping pre-built dockers is a license nightmare, second because > better hackability. For a full on example see: > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/.gitlab-ci.yml Humm, but in the end you are still publishing the docker images either way. It's just a difference of publishing across projects and whether it's 2 jobs or 1 job with 2 stages. I do tend to prefer the Travis-CI simplicity of just listing packages to install. It seems like the same could be done with docker and some magic on the server side. At least for the simple case of 'install this list of packages'. > For the kernel I guess the question is where we should put all the > docker files. There's going to be some need for subsystem specific > tooling (e.g. we want something that has igt installed eventually for > anything drm), so maybe just keeping them in subsystem directories is > best. Was at least my plan with drivers/gpu. My plan (to the extent I have one) was to put the common parts into the docker image and leave the small job specific things in the job setup. Then I can use the same docker image across jobs. Maybe that's a pointless goal in the docker world. My dockerfile is simple enough, I could probably just create it within the .gitlab-ci.yml. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] backlight: pwm_bl: Fix brightness levels for non-DT case.
Hi,
Missatge de Enric Balletbo i Serra del
dia dc., 7 de nov. 2018 a les 9:56:
>
> Commit '88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED
> linearly to human eye")' allows the possibility to compute a default
> brightness table when there isn't the brightness-levels property in the
> DT. Unfortunately the changes made broke the pwm backlight for the
> non-DT boards.
>
> Usually, the non-DT boards don't pass the brightness levels via platform
> data, instead, it sets the max_brightness in their platform data and the
> driver calculates the level without a table. The offending patch assumed
> that when there is no brightness levels table we should create one, but this
> is clearly wrong for the non-DT case.
>
> After this patch the code handles the DT and the non-DT case taking in
> consideration also if max_brightness is set or not.
>
> Fixes: 88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED linearly
> to human eye")
> Cc:
> Reported-by: Robert Jarzmik
> Signed-off-by: Enric Balletbo i Serra
> Tested-by: Robert Jarzmik
> Acked-by: Daniel Thompson
> ---
>
> Changes in v3:
> - Fixed some typos in commit message.
> - Removed ' in Fixes tag.
>
> Changes in v2:
> - Rebase on top of mainline
> - Add Tested-by and Acked-by tags.
>
> drivers/video/backlight/pwm_bl.c | 41 +++-
> 1 file changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/backlight/pwm_bl.c
> b/drivers/video/backlight/pwm_bl.c
> index 678b27063198..f9ef0673a083 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -562,7 +562,30 @@ static int pwm_backlight_probe(struct platform_device
> *pdev)
> goto err_alloc;
> }
>
> - if (!data->levels) {
> + if (data->levels) {
> + /*
> +* For the DT case, only when brightness levels is defined
> +* data->levels is filled. For the non-DT case, data->levels
> +* can come from platform data, however is not usual.
> +*/
> + for (i = 0; i <= data->max_brightness; i++) {
> + if (data->levels[i] > pb->scale)
> + pb->scale = data->levels[i];
> +
> + pb->levels = data->levels;
> + }
> + } else if (!data->max_brightness) {
> + /*
> +* If no brightness levels are provided and max_brightness is
> +* not set, use the default brightness table. For the DT case,
> +* max_brightness is set to 0 when brightness levels is not
> +* specified. For the non-DT case, max_brightness is usually
> +* set to some value.
> +*/
> +
> + /* Get the PWM period (in nanoseconds) */
> + pwm_get_state(pb->pwm, );
> +
> ret = pwm_backlight_brightness_default(>dev, data,
>state.period);
> if (ret < 0) {
> @@ -570,13 +593,19 @@ static int pwm_backlight_probe(struct platform_device
> *pdev)
> "failed to setup default brightness table\n");
> goto err_alloc;
> }
> - }
>
> - for (i = 0; i <= data->max_brightness; i++) {
> - if (data->levels[i] > pb->scale)
> - pb->scale = data->levels[i];
> + for (i = 0; i <= data->max_brightness; i++) {
> + if (data->levels[i] > pb->scale)
> + pb->scale = data->levels[i];
>
> - pb->levels = data->levels;
> + pb->levels = data->levels;
> + }
> + } else {
> + /*
> +* That only happens for the non-DT case, where platform data
> +* sets the max_brightness value.
> +*/
> + pb->scale = data->max_brightness;
> }
>
> pb->lth_brightness = data->lth_brightness * (state.period /
> pb->scale);
> --
> 2.19.1
>
A gentle ping on this patch.
Best regards,
Enric
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/7] drm/msm/dsi: 14nm PHY: Get ref clock from the DT
On Tue, Nov 27, 2018 at 09:56:46PM -0800, Doug Anderson wrote:
> Hi,
>
> On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke wrote:
> >
> > Get the ref clock of the PHY from the device tree instead of
> > hardcoding its name and rate.
>
> In the case of the 14nm PHY I think it's OK that you break
> compatibility with old device tree files (as this patch does) since
> the 14nm support was added sorta recently and "git grep" shows no
> users in linuxnext. You should note that you're breaking
> compatibility with old DTS files in the commit message here so that if
> someone crawls out of the woodwork it will be easy for them to
> understand what happened.
ok, I'll add the note
> > + pll_14nm->vco_ref_clk = devm_clk_get(>dev, "ref");
> > + if (IS_ERR(pll_14nm->vco_ref_clk)) {
> > + ret = PTR_ERR(pll_14nm->vco_ref_clk);
> > + if (ret != EPROBE_DEFER)
>
> Shouldn't this check against -EPROBE_DEFER, not against EPROBE_DEFER?
> It's negative. Presumably this same feedback needs to be applied to
> the whole patch series.
You are right, will fix it throughout the series, thanks!
> Other than that this looks good to me and you can feel free to add my
> Reviewed-by tag FWIW.
Great, thanks for the review!
Matthias
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/7] dt-bindings: msm/dsi: Add ref clock for PHYs
On Tue, Nov 27, 2018 at 09:41:39PM -0800, Doug Anderson wrote: > Hi, > > On Mon, Nov 26, 2018 at 3:12 PM Matthias Kaehlcke wrote: > > > > Allow the PHY drivers to get the ref clock from the DT. > > > > Signed-off-by: Matthias Kaehlcke > > --- > > Changes in v2: > > - add the ref clock for all PHYs, not only the 10nm one > > - updated commit message > > --- > > Documentation/devicetree/bindings/display/msm/dsi.txt | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt > > b/Documentation/devicetree/bindings/display/msm/dsi.txt > > index dfc743219bd88..b0485559a719c 100644 > > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt > > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt > > @@ -106,6 +106,7 @@ Required properties: > > - clocks: Phandles to device clocks. See [1] for details on clock bindings. > > - clock-names: the following clocks are required: > >* "iface" > > + * "ref" > > We can't quite make ref "required" because there are some old device > tree files dating back to 2016 that would be broken. It could be > listed as optional, but I _think_ Rob is OK with it being listed as > "required" for all new device tree files with the footnote that if > it's not present then the code will still work. Ok, will update the code to keep supporting old DT files and update the description. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 30 Nov 2018 14:12:19 -0800 Jarkko Sakkinen wrote: > As a maintainer myself (and based on somewhat disturbed feedback from > other maintainers) I can only make the conclusion that nobody knows what > the responsibility part here means. > > I would interpret, if I read it like at lawyer at least, that even for > existing code you would need to do the changes postmorterm. > > Is this wrong interpretation? Should I conclude that I made a mistake > by reading the CoC and trying to understand what it *actually* says? > After this discussion, I can say that I understand it less than before. Have you read Documentation/process/code-of-conduct-interpretation.rst? As has been pointed out, it contains a clear answer to how things should be interpreted here. Thanks, jon ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 01:57:49PM -0800, James Bottomley wrote: > On Fri, 2018-11-30 at 13:44 -0800, Jarkko Sakkinen wrote: > > On Fri, Nov 30, 2018 at 01:01:02PM -0800, James Bottomley wrote: > > > No because use of what some people consider to be bad language > > > isn't necessarily abusive, offensive or degrading. Our most > > > heavily censored medium is TV and "fuck" is now considered > > > acceptable in certain contexts on most channels in the UK and EU. > > > > This makes following the CoC extremely hard to a non-native speaker > > as it is not too explicit on what is OK and what is not. I did > > through the whole thing with an eye glass and this what I deduced > > from it. > > OK, so something that would simply be considered in some quarters as > bad language isn't explicitly banned. The thing which differentiates > simple bad language from "abusive, offensive or degrading language", > which is called out by the CoC, is the context and the target. > > So when it's a simple expletive or the code of the author or even the > hardware is the target, I'd say it's an easy determination it's not a > CoC violation. If someone else's code is the target or the inventor of > the hardware is targetted by name, I'd say it is. Even non-native > English speakers should be able to determine target and context, > because that's the essence of meaning. I pasted this already to another response and this was probably the part that ignited me to send the patch set (was a few days ago, so had to revisit to find the exact paragraph): "Maintainers have the right and responsibility to remove, edit, or reject comments, commits, code, wiki edits, issues, and other contributions that are not aligned to this Code of Conduct, or to ban temporarily or permanently any contributor for other behaviors that they deem inappropriate, threatening, offensive, or harmful." The whole patch set is neither a joke/troll nor something I would necessarily want to be include myself. It does have the RFC tag. As a maintainer myself (and based on somewhat disturbed feedback from other maintainers) I can only make the conclusion that nobody knows what the responsibility part here means. I would interpret, if I read it like at lawyer at least, that even for existing code you would need to do the changes postmorterm. Is this wrong interpretation? Should I conclude that I made a mistake by reading the CoC and trying to understand what it *actually* says? After this discussion, I can say that I understand it less than before. /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 2018-11-30 at 13:54 -0800, Jarkko Sakkinen wrote: > On Fri, Nov 30, 2018 at 01:48:08PM -0800, David Miller wrote: > > From: Jarkko Sakkinen > > Date: Fri, 30 Nov 2018 13:44:05 -0800 > > > > > On Fri, Nov 30, 2018 at 01:01:02PM -0800, James Bottomley wrote: > > > > No because use of what some people consider to be bad language > > > > isn't > > > > necessarily abusive, offensive or degrading. Our most heavily > > > > censored > > > > medium is TV and "fuck" is now considered acceptable in certain > > > > contexts on most channels in the UK and EU. > > > > > > This makes following the CoC extremely hard to a non-native > > > speaker as > > > it is not too explicit on what is OK and what is not. I did > > > through the > > > whole thing with an eye glass and this what I deduced from it. > > > > It would be helpful if you could explain what part of the language > > is unclear wrt. explaining how CoC does not apply to existing code. > > > > That part seems very explicit to me. > > Well, now that I re-read it, it was this part to be exact: > > "Maintainers have the right and responsibility to remove, edit, or > reject comments, commits, code, wiki edits, issues, and other > contributions that are not aligned to this Code of Conduct, or to ban > temporarily or permanently any contributor for other behaviors that > they deem inappropriate, threatening, offensive, or harmful." > > How this should be interpreted? Firstly, this is *only* about contributions going forward. The interpretation document says we don't have to look back into the repository and we definitely can't remove something from git that's already been committed. As a Maintainer, if you feel bad language is inappropriate for your subsystem then you say so and reject with that reason patches that come in containing it (suggesting alternative rewordings). However, your determination about what is or isn't acceptable in your subsystem isn't binding on other maintainers, who may have different standards ... this is identical to what we do with coding, like your insistence on one line per variable or other subsystem's insistence on reverse christmas tree for includes ... James > I have not really followed the previous CoC discussions as I try to > always use polite language so I'm sorry if this duplicate. > > /Jarkko > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 14/24] drm/msm: dpu: Grab the modeset locks in frame_event
From: Sean Paul
This patch wraps dpu_core_perf_crtc_release_bw() with modeset locks
since it digs into the state objects.
Changes in v2:
- None
Changes in v3:
- Use those nifty new DRM_MODESET_LOCK_ALL_* helpers (Daniel)
Cc: Daniel Vetter
Cc: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 74ef384d9cd6a..03ddd281a354f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -306,6 +306,19 @@ static void dpu_crtc_vblank_cb(void *data)
trace_dpu_crtc_vblank_cb(DRMID(crtc));
}
+static void dpu_crtc_release_bw_unlocked(struct drm_crtc *crtc)
+{
+ int ret = 0;
+ struct drm_modeset_acquire_ctx ctx;
+
+ DRM_MODESET_LOCK_ALL_BEGIN(crtc->dev, ctx, 0, ret);
+ dpu_core_perf_crtc_release_bw(crtc);
+ DRM_MODESET_LOCK_ALL_END(ctx, ret);
+ if (ret)
+ DRM_ERROR("Failed to acquire modeset locks to release bw, %d\n",
+ ret);
+}
+
static void dpu_crtc_frame_event_work(struct kthread_work *work)
{
struct dpu_crtc_frame_event *fevent = container_of(work,
@@ -335,7 +348,7 @@ static void dpu_crtc_frame_event_work(struct kthread_work
*work)
/* release bandwidth and other resources */
trace_dpu_crtc_frame_event_done(DRMID(crtc),
fevent->event);
- dpu_core_perf_crtc_release_bw(crtc);
+ dpu_crtc_release_bw_unlocked(crtc);
} else {
trace_dpu_crtc_frame_event_more_pending(DRMID(crtc),
fevent->event);
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 2018-11-30 at 13:44 -0800, Jarkko Sakkinen wrote: > On Fri, Nov 30, 2018 at 01:01:02PM -0800, James Bottomley wrote: > > No because use of what some people consider to be bad language > > isn't necessarily abusive, offensive or degrading. Our most > > heavily censored medium is TV and "fuck" is now considered > > acceptable in certain contexts on most channels in the UK and EU. > > This makes following the CoC extremely hard to a non-native speaker > as it is not too explicit on what is OK and what is not. I did > through the whole thing with an eye glass and this what I deduced > from it. OK, so something that would simply be considered in some quarters as bad language isn't explicitly banned. The thing which differentiates simple bad language from "abusive, offensive or degrading language", which is called out by the CoC, is the context and the target. So when it's a simple expletive or the code of the author or even the hardware is the target, I'd say it's an easy determination it's not a CoC violation. If someone else's code is the target or the inventor of the hardware is targetted by name, I'd say it is. Even non-native English speakers should be able to determine target and context, because that's the essence of meaning. James ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
From: Jarkko Sakkinen Date: Fri, 30 Nov 2018 13:42:33 -0800 > Can you tell how the CoC should be interpreted then? Regardless of what I think, as others have showen the CoC explicitly does not apply to existing code. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 01:48:08PM -0800, David Miller wrote: > From: Jarkko Sakkinen > Date: Fri, 30 Nov 2018 13:44:05 -0800 > > > On Fri, Nov 30, 2018 at 01:01:02PM -0800, James Bottomley wrote: > >> No because use of what some people consider to be bad language isn't > >> necessarily abusive, offensive or degrading. Our most heavily censored > >> medium is TV and "fuck" is now considered acceptable in certain > >> contexts on most channels in the UK and EU. > > > > This makes following the CoC extremely hard to a non-native speaker as > > it is not too explicit on what is OK and what is not. I did through the > > whole thing with an eye glass and this what I deduced from it. > > It would be helpful if you could explain what part of the language > is unclear wrt. explaining how CoC does not apply to existing code. > > That part seems very explicit to me. Well, now that I re-read it, it was this part to be exact: "Maintainers have the right and responsibility to remove, edit, or reject comments, commits, code, wiki edits, issues, and other contributions that are not aligned to this Code of Conduct, or to ban temporarily or permanently any contributor for other behaviors that they deem inappropriate, threatening, offensive, or harmful." How this should be interpreted? I have not really followed the previous CoC discussions as I try to always use polite language so I'm sorry if this duplicate. /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
From: Jarkko Sakkinen Date: Fri, 30 Nov 2018 13:44:05 -0800 > On Fri, Nov 30, 2018 at 01:01:02PM -0800, James Bottomley wrote: >> No because use of what some people consider to be bad language isn't >> necessarily abusive, offensive or degrading. Our most heavily censored >> medium is TV and "fuck" is now considered acceptable in certain >> contexts on most channels in the UK and EU. > > This makes following the CoC extremely hard to a non-native speaker as > it is not too explicit on what is OK and what is not. I did through the > whole thing with an eye glass and this what I deduced from it. It would be helpful if you could explain what part of the language is unclear wrt. explaining how CoC does not apply to existing code. That part seems very explicit to me. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 01:01:02PM -0800, James Bottomley wrote: > No because use of what some people consider to be bad language isn't > necessarily abusive, offensive or degrading. Our most heavily censored > medium is TV and "fuck" is now considered acceptable in certain > contexts on most channels in the UK and EU. This makes following the CoC extremely hard to a non-native speaker as it is not too explicit on what is OK and what is not. I did through the whole thing with an eye glass and this what I deduced from it. /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 12:35:07PM -0800, David Miller wrote: > From: Jens Axboe > Date: Fri, 30 Nov 2018 13:12:26 -0700 > > > On 11/30/18 12:56 PM, Davidlohr Bueso wrote: > >> On Fri, 30 Nov 2018, Kees Cook wrote: > >> > >>> On Fri, Nov 30, 2018 at 11:27 AM Jarkko Sakkinen > >>> wrote: > > In order to comply with the CoC, replace with a hug. > >> > >> I hope this is some kind of joke. How would anyone get offended by reading > >> technical comments? This is all beyond me... > > > > Agree, this is insanity. > > And even worse it is censorship. It is not close to a cencorship, especially when I used RFC tag, which essentially says that I'm not saying "please take this", right? Can you tell how the CoC should be interpreted then? I read it through on my plane trip with an eye glass. Is cursing OK? /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 08:42:23PM +, Chris Mason wrote: > I think the bar for changing the documentation/comments should be > improvement in the clarity or approachability of whatever is being > changed. > > This patch set is kind of like Linus sitting at kernel summit with a > sign that says "Free Hugs". Kind of confusing, and really unlikely to > make anyone involved happier about working on the kernel. > > Jarkko, making things clearer and more approachable is absolutely worth > the time if you're interested, but I think it'll mean larger and more > individualized changes to these files. Fully 110% agree. /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 09:31:13PM +0100, Matthias Brugger wrote: > Like John I don't think that the word "fuck" is something we have to ban from > the source code, but I don't care too much. Anyway, please don't change it to > something like heck as it might be difficult for non-english speaker to > understand. I make context sensitive better thought updates based on the feedback that Kees gave. I used RFC tag for a reason. /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 09:09:48PM +0100, John Paul Adrian Glaubitz wrote: > Or just leave it as is because we're all grown up and don't freak out > when a piece of text contains the word "fuck". > > I still don't understand why people think that the word "fuck" is what > would keep certain groups from contributing to the Linux kernel. In all > seriousness, it doesn't. Are you making a claim that your personal experience, and maybe your mates, is the objective truth, or am I misunderstanding something? /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 03/15] drm/nouveau: replace **** with a hug
On Fri, Nov 30, 2018 at 06:13:50PM -0200, Diego Viola wrote:
> On Fri, Nov 30, 2018 at 5:30 PM Jarkko Sakkinen
> wrote:
> >
> > In order to comply with the CoC, replace with a hug.
> >
> > Signed-off-by: Jarkko Sakkinen
> > ---
> > drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c| 2 +-
> > drivers/gpu/drm/nouveau/nvkm/subdev/pmu/fuc/macros.fuc | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c
> > b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c
> > index 9cc10e438b3d..55a0060881ea 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c
> > @@ -446,7 +446,7 @@ init_ram_restrict(struct nvbios_init *init)
> > {
> > /* This appears to be the behaviour of the VBIOS parser, and *is*
> > * important to cache the NV_PEXTDEV_BOOT0 on later chipsets to
> > -* avoid fucking up the memory controller (somehow) by reading it
> > +* avoid hugging up the memory controller (somehow) by reading it
> > * on every INIT_RAM_RESTRICT_ZM_GROUP opcode.
> > *
> > * Preserving the non-caching behaviour on earlier chipsets just
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/fuc/macros.fuc
> > b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/fuc/macros.fuc
> > index 3737bd27f74e..ee364c71cc2e 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/fuc/macros.fuc
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/fuc/macros.fuc
> > @@ -46,7 +46,7 @@
> > #define NV_PPWR_INTR_EN_SET_SUBINTR
> > 0x0800
> > #define NV_PPWR_INTR_EN_SET_WATCHDOG
> > 0x0002
> > #define NV_PPWR_INTR_EN_CLR
> > 0x0014
> > -#define NV_PPWR_INTR_EN_CLR_MASK/* fuck i hate envyas
> > */ -1
> > +#define NV_PPWR_INTR_EN_CLR_MASK/* hug i hate envyas
> > */ -1
>
> Can you also capitalize the "i" please?
Yeah, maybe I just change it as "/* I hate envyas */"?
/Jarkko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 4/9] drm/i915/icl: Do not change reserved registers related to PSR2
I'll check on it.
> -Original Message-
> From: Souza, Jose
> Sent: Thursday, 29 November, 2018 3:47 PM
> To: Vivi, Rodrigo
> Cc: dri-devel@lists.freedesktop.org; intel-...@lists.freedesktop.org;
> Runyan, Arthur J ; Pandiyan, Dhinakaran
>
> Subject: Re: [PATCH 4/9] drm/i915/icl: Do not change reserved registers
> related to PSR2
>
> On Thu, 2018-11-29 at 14:15 -0800, Rodrigo Vivi wrote:
> > On Mon, Nov 26, 2018 at 04:37:05PM -0800, José Roberto de Souza
> > wrote:
> > > For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not
> > > touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is
> already
> > > unset in gen10 + GLK we can just drop it and fix for both gens.
> > >
> > > Cc: Dhinakaran Pandiyan
> > > Cc: Rodrigo Vivi
> > > Signed-off-by: José Roberto de Souza
> >
> > Ok, this patch seems right according to spec.
> >
> > Reviewed-by: Rodrigo Vivi
> >
> > But I wonder now if we need intel_psr_setup_vsc() at all for
> > platforms different than gen9.
> >
> > Because description of this bit is:
> > This field enables the programmable header for the PSR2 VSC packet.
> >
> > Without the programmable version I would assume display
> > engine is now responsible for setting header entirely?
>
> As this was in a chicken register in gen 9 my guess is that it was
> fixed on newer gens and as it is required for PSR2 we don't need to
> manually enable it in driver but Art could confirm it.
>
> >
> > Art?
> >
> > > ---
> > > drivers/gpu/drm/i915/intel_psr.c | 11 ---
> > > 1 file changed, 4 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 607c3ec41679..7607a58a6ec0 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct
> > > intel_dp *intel_dp,
> > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > > hsw_psr_setup_aux(intel_dp);
> > >
> > > - if (dev_priv->psr.psr2_enabled) {
> > > + if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) &&
> > > +!IS_GEMINILAKE(dev_priv))) {
> > > i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> > > cpu_transcoder)
> > > ;
> > > u32 chicken = I915_READ(reg);
> > >
> > > - if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
> > > - chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
> > > -| PSR2_ADD_VERTICAL_LINE_COUNT);
> > > -
> > > - else
> > > - chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
> > > + chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
> > > +PSR2_ADD_VERTICAL_LINE_COUNT;
> > > I915_WRITE(reg, chicken);
> > > }
> > >
> > > --
> > > 2.19.2
> > >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 2018-11-30 at 12:55 -0800, Jarkko Sakkinen wrote: > On Fri, Nov 30, 2018 at 11:56:52AM -0800, Davidlohr Bueso wrote: > > On Fri, 30 Nov 2018, Kees Cook wrote: > > > > > On Fri, Nov 30, 2018 at 11:27 AM Jarkko Sakkinen > > > wrote: > > > > > > > > In order to comply with the CoC, replace with a hug. > > > > I hope this is some kind of joke. How would anyone get offended by > > reading technical comments? This is all beyond me... > > Well... Not a joke really but more like conversation starter :-) > > I had 10h flight from Amsterdam to Portland and one of the things > that I did was to read the new CoC properly. > > This a direct quote from the CoC: > > "Harassment includes the use of abusive, offensive or degrading > language, intimidation, stalking, harassing photography or recording, > inappropriate physical contact, sexual imagery and unwelcome sexual > advances or requests for sexual favors." > > Doesn't this fall into this category? No because use of what some people consider to be bad language isn't necessarily abusive, offensive or degrading. Our most heavily censored medium is TV and "fuck" is now considered acceptable in certain contexts on most channels in the UK and EU. > Your argument is not that great because you could say that from any > LKML discussion. If you don't like hugging, please propose something > else > :-) The interpretation document also says this: ontributions submitted for the kernel should use appropriate language. Content that already exists predating the Code of Conduct will not be addressed now as a violation. So that definitely means there should be no hunting down of existing comments in kernel code. James ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 11:40:17AM -0800, Kees Cook wrote: > On Fri, Nov 30, 2018 at 11:27 AM Jarkko Sakkinen > wrote: > > > > In order to comply with the CoC, replace with a hug. > > Heh. I support the replacement of the stronger language, but I find > "hug", "hugged", and "hugging" to be very weird replacements. Can we > bikeshed this to "heck", "hecked", and "hecking" (or "heckin" to > follow true Doggo meme style). > > "This API is hugged" doesn't make any sense to me. "This API is > hecked" is better, or at least funnier (to me). "Hug this interface" > similarly makes no sense, but "Heck this interface" seems better. > "Don't touch my hecking code", "What the heck were they thinking?" > etc... "hug" is odd. > > Better yet, since it's only 17 files, how about doing context-specific > changes? "This API is terrible", "Hateful interface", "Don't touch my > freakin' code", "What in the world were they thinking?" etc? I'm happy to refine this (thus the RFC tag)! And depending on the culture, hugging could fall in the harrasment category. Actually, when I think about it, in Finland this kind of poking of ones personal bubble would be such :-) I'll refine the patch set with more context sensitive replacements, perhaps removing the comment altogether in some places. Thank you for the feedback! /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 30 Nov 2018 12:55:21 -0800 Jarkko Sakkinen wrote: > This a direct quote from the CoC: > > "Harassment includes the use of abusive, offensive or degrading > language, intimidation, stalking, harassing photography or recording, > inappropriate physical contact, sexual imagery and unwelcome sexual > advances or requests for sexual favors." ...and this is from the interpretation document: > Contributions submitted for the kernel should use appropriate language. > Content that already exists predating the Code of Conduct will not be > addressed now as a violation. So existing code is explicitly not a CoC violation and need not be treated as such. That said, improvements to the comments are always welcome, as long as they are actually improvements. Thanks, jon ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 30 Nov 2018 12:55:21 -0800 Jarkko Sakkinen wrote: > On Fri, Nov 30, 2018 at 11:56:52AM -0800, Davidlohr Bueso wrote: > > On Fri, 30 Nov 2018, Kees Cook wrote: > > > > > On Fri, Nov 30, 2018 at 11:27 AM Jarkko Sakkinen > > > wrote: > > > > > > > > In order to comply with the CoC, replace with a hug. > > > > I hope this is some kind of joke. How would anyone get offended by reading > > technical comments? This is all beyond me... > > Well... Not a joke really but more like conversation starter :-) > > I had 10h flight from Amsterdam to Portland and one of the things that I > did was to read the new CoC properly. > > This a direct quote from the CoC: > > "Harassment includes the use of abusive, offensive or degrading > language, intimidation, stalking, harassing photography or recording, > inappropriate physical contact, sexual imagery and unwelcome sexual > advances or requests for sexual favors." > > Doesn't this fall into this category? > It has also been discussed that existing code (and past conduct) will not be covered under the CoC. It's about new code and conduct moving forward. -- Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 11:56:52AM -0800, Davidlohr Bueso wrote: > On Fri, 30 Nov 2018, Kees Cook wrote: > > > On Fri, Nov 30, 2018 at 11:27 AM Jarkko Sakkinen > > wrote: > > > > > > In order to comply with the CoC, replace with a hug. > > I hope this is some kind of joke. How would anyone get offended by reading > technical comments? This is all beyond me... Well... Not a joke really but more like conversation starter :-) I had 10h flight from Amsterdam to Portland and one of the things that I did was to read the new CoC properly. This a direct quote from the CoC: "Harassment includes the use of abusive, offensive or degrading language, intimidation, stalking, harassing photography or recording, inappropriate physical contact, sexual imagery and unwelcome sexual advances or requests for sexual favors." Doesn't this fall into this category? Your argument is not that great because you could say that from any LKML discussion. If you don't like hugging, please propose something else :-) /Jarkko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] of: Add a GitLab CI config file for unittests
On Fri, Nov 30, 2018 at 9:32 PM Rob Herring wrote: > > This adds a GitLab CI config file running the DT unittest in a usermode > Linux build. The corresponding CI job can be found here: > > https://gitlab.com/robherring/linux-dt-unittest/pipelines > > This CI job can be duplicated by others by creating a kernel repo on a > GitLab instance and configuring GitLab CI to use > drivers/of/.gitlab-ci.yml config file. > > Cc: Frank Rowand > Cc: Daniel Vetter > Signed-off-by: Rob Herring Adding dri-devel. > --- > drivers/of/.gitlab-ci.yml | 18 ++ > 1 file changed, 18 insertions(+) > create mode 100644 drivers/of/.gitlab-ci.yml > > diff --git a/drivers/of/.gitlab-ci.yml b/drivers/of/.gitlab-ci.yml > new file mode 100644 > index ..44a4824f5c33 > --- /dev/null > +++ b/drivers/of/.gitlab-ci.yml > @@ -0,0 +1,18 @@ > +# SPDX-License-Identifier: GPL-2.0+ > + > +image: registry.gitlab.com/robherring/docker-images/ubuntu-kernel-build I think it's better to include the docker recipe too. One because shipping pre-built dockers is a license nightmare, second because better hackability. For a full on example see: https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/.gitlab-ci.yml For the kernel I guess the question is where we should put all the docker files. There's going to be some need for subsystem specific tooling (e.g. we want something that has igt installed eventually for anything drm), so maybe just keeping them in subsystem directories is best. Was at least my plan with drivers/gpu. Needs latest gitlab to make the build-on-demand stuff work, but gitlab.com has that. -Daniel > + > +variables: > + ARCH: um > + > +job-unittest: > + artifacts: > +paths: > + - "*.log" > + > + script: > +- echo -e "CONFIG_OF=y\nCONFIG_OF_UNITTEST=y\nCONFIG_OF_OVERLAY=y" > > kernel/configs/extra.config > +- make defconfig extra.config > +- make -s -j $(nproc) vmlinux | tee build.log > +- TMP=/tmp ./vmlinux > boot.log || true > +- grep -E '\#\#\# dt-test \#\#\# end of unittest - [0-9]* passed, 0 > failed' boot.log > -- > 2.19.1 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
From: Jens Axboe Date: Fri, 30 Nov 2018 13:12:26 -0700 > On 11/30/18 12:56 PM, Davidlohr Bueso wrote: >> On Fri, 30 Nov 2018, Kees Cook wrote: >> >>> On Fri, Nov 30, 2018 at 11:27 AM Jarkko Sakkinen >>> wrote: In order to comply with the CoC, replace with a hug. >> >> I hope this is some kind of joke. How would anyone get offended by reading >> technical comments? This is all beyond me... > > Agree, this is insanity. And even worse it is censorship. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 30 Nov 2018 20:39:01 + Abuse wrote: > On Friday, 30 November 2018 20:35:07 GMT David Miller wrote: > > From: Jens Axboe > > Date: Fri, 30 Nov 2018 13:12:26 -0700 > > > > > On 11/30/18 12:56 PM, Davidlohr Bueso wrote: > > >> On Fri, 30 Nov 2018, Kees Cook wrote: > > >> > > >>> On Fri, Nov 30, 2018 at 11:27 AM Jarkko Sakkinen > > >>> wrote: > > > > In order to comply with the CoC, replace with a hug. > > >> > > >> I hope this is some kind of joke. How would anyone get offended by > > >> reading > > >> technical comments? This is all beyond me... > > > > > > Agree, this is insanity. > > > > And even worse it is censorship. > > > > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > Hug > > I assume I will now be barred. > -- Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
From: Abuse Date: Fri, 30 Nov 2018 20:39:01 + > I assume I will now be barred. Perhaps, but not because you said fuck. It would be because you're intentionally creating a disturbance on the list and making it more difficult for developers to get their work done and intentionally creating a distraction and a hostile environment for the discussion at hand. That would not be censorship. There is a big difference. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
From: Davidlohr Bueso Date: Fri, 30 Nov 2018 11:56:52 -0800 > I hope this is some kind of joke. Whether or not it is a joke, it is censorship. And because of that I have no intention to apply any patches like this to any code I am in charge of. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/vc4: Add a debugfs entry to disable/enable the load tracker
Paul Kocialkowski writes: > In order to test whether the load tracker is working as expected, we > need the ability to compare the commit result with the underrun > indication. With the load tracker always enabled, commits that are > expected to trigger an underrun are always rejected, so userspace > cannot get the actual underrun indication from the hardware. > > Add a debugfs entry to disable/enable the load tracker, so that a DRM > commit expected to trigger an underrun can go through with the load > tracker disabled. The underrun indication is then available to > userspace and can be checked against the commit result with the load > tracker enabled. > > Signed-off-by: Paul Kocialkowski Given that the load tracker is going to be conservative and say things will underrun even when they might not in practice, will this actually be useful for automated testing? Or is the intent to make it easier to tune the load tracker by disabling it so that you can experiment freely? signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH v2 5/5] drm/msm: subclass work object for vblank events
On Fri, Nov 30, 2018 at 11:45:55AM -0800, Jeykumar Sankaran wrote:
> On 2018-11-29 14:15, Sean Paul wrote:
> > On Tue, Nov 20, 2018 at 02:04:14PM -0800, Jeykumar Sankaran wrote:
> > > On 2018-11-07 07:55, Sean Paul wrote:
> > > > On Tue, Nov 06, 2018 at 02:36:30PM -0800, Jeykumar Sankaran wrote:
> > > > > msm maintains a separate structure to define vblank
> > > > > work definitions and a list to track events submitted
> > > > > to the workqueue. We can avoid this redundant list
> > > > > and its protection mechanism, if we subclass the
> > > > > work object to encapsulate vblank event parameters.
> > > > >
> > > > > changes in v2:
> > > > > - subclass optimization on system wq (Sean Paul)
> > > >
> > > > I wouldn't do it like this, tbh. One problem is that you've lost your
> > > > flush() on
> > > > unbind, so there's no way to know if you have workers in the wild
> > > > waiting
> > > > to
> > > > enable/disable vblank.
> > > >
> > > > Another issues is that AFAICT, we don't need a queue of
> > > > enables/disables,
> > > > but
> > > > rather just the last requested state (ie: should we be on or off). So
> > > > things
> > > > don't need to be this complicated (and we're possibly thrashing vblank
> > > > on/off
> > > > for no reason).
> > > >
> > > > I'm still of the mind that you should just make this synchronous and
> > be
> > > > done
> > > > with the threads (especially since we're still uncovering/introducing
> > > > races!).
> > > >
> > > While scoping out the effort to make vblank events synchronous, I
> > > found
> > > that the spinlock locking order of vblank request sequence and vblank
> > > callback
> > > sequences are the opposite.
> > >
> > > In DPU, drm_vblank_enable acquires vblank_time_lock before registering
> > > the crtc to encoder which happens after acquiring encoder_spinlock.
> > > But
> > > the vblank_callback acquires encoder_spinlock before accessing the
> > > registered
> > > crtc and calling into drm_vblank_handler which tries to acquire
> > > vblank_time_lock.
> > > Acquiring both vblank_time_lock and encoder_spinlock in the same
> > > thread
> > > is leading to deadlock.
> >
> > Hmm, I'm not sure I follow. Are you seeing issues where irq overlaps
> > with
> > enable/disable? I hacked in sync vblank enable/disable quickly to see if
> > I
> > could
> > reproduce what you're seeing, but things seemed well behaved.
> >
>
> The race is between drm_vblank_get/put and vblank_handler contexts.
>
> When made synchronous:
>
> while calling drm_vblank_get, the callstack looks like below:
> drm_vblank_get -> drm_vblank_enable (acquires vblank_time_lock) ->
> __enable_vblank -> dpu_crtc_vblank -> dpu_encoder_toggle_vblank_for_crtc
> (tries to acquire enc_spinlock)
>
> In vblank handler, the call stack will be:
> dpu_encoder_phys_vid_vblank_irq -> dpu_encoder_vblank_callback (acquires
> enc_spinlock) -> dpu_crtc_vblank_callback -> drm_handle_vblank (tries to
> acquire vblank_time_lock)
Hmm, I'm not sure how this can happen. We acquire and release the enc_spinlock
before enabling the irq, yes we will hold on to the vbl_time_lock, but we
shouldn't be trying to reacquire an encoder's spinlock after we've enabled it.
I don't know how that can deadlock, since we should never be running enable and
the handler concurrently.
The only thing I can think of is that the vblank interrupts are firing after
vblank has been disabled? In that case, it seems like we should properly flush
them.
Sean
>
>
> > I do see that there is a chance to call drm_handle_vblank() while
> > holding
> > enc_spinlock, but couldn't find any obvious lock recursion there.
> >
> > Maybe a callstack or lockdep splat would help?
> >
> > Sean
> >
> >
> > Here's my hack to bypass the display thread:
> >
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c
> > b/drivers/gpu/drm/msm/msm_drv.c
> > index 9c9f7ff6960b38..5a3cac5825319e 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@ -242,24 +242,19 @@ static void vblank_ctrl_worker(struct kthread_work
> > *work)
> > static int vblank_ctrl_queue_work(struct msm_drm_private *priv,
> > int crtc_id, bool enable)
> > {
> > + struct msm_kms *kms = priv->kms;
> > struct msm_vblank_ctrl *vbl_ctrl = >vblank_ctrl;
> > - struct vblank_event *vbl_ev;
> > unsigned long flags;
> >
> > - vbl_ev = kzalloc(sizeof(*vbl_ev), GFP_ATOMIC);
> > - if (!vbl_ev)
> > - return -ENOMEM;
> > + spin_lock_irqsave(_ctrl->lock, flags);
> >
> > - vbl_ev->crtc_id = crtc_id;
> > - vbl_ev->enable = enable;
> > + if (enable)
> > + kms->funcs->enable_vblank(kms, priv->crtcs[crtc_id]);
> > + else
> > + kms->funcs->disable_vblank(kms, priv->crtcs[crtc_id]);
> >
> > - spin_lock_irqsave(_ctrl->lock, flags);
> > - list_add_tail(_ev->node, _ctrl->event_list);
> > spin_unlock_irqrestore(_ctrl->lock, flags);
> >
> > -
Re: [Freedreno] [PATCH v2 5/5] drm/msm: subclass work object for vblank events
On 2018-11-29 14:15, Sean Paul wrote:
On Tue, Nov 20, 2018 at 02:04:14PM -0800, Jeykumar Sankaran wrote:
On 2018-11-07 07:55, Sean Paul wrote:
> On Tue, Nov 06, 2018 at 02:36:30PM -0800, Jeykumar Sankaran wrote:
> > msm maintains a separate structure to define vblank
> > work definitions and a list to track events submitted
> > to the workqueue. We can avoid this redundant list
> > and its protection mechanism, if we subclass the
> > work object to encapsulate vblank event parameters.
> >
> > changes in v2:
> > - subclass optimization on system wq (Sean Paul)
>
> I wouldn't do it like this, tbh. One problem is that you've lost your
> flush() on
> unbind, so there's no way to know if you have workers in the wild
> waiting
> to
> enable/disable vblank.
>
> Another issues is that AFAICT, we don't need a queue of
> enables/disables,
> but
> rather just the last requested state (ie: should we be on or off). So
> things
> don't need to be this complicated (and we're possibly thrashing vblank
> on/off
> for no reason).
>
> I'm still of the mind that you should just make this synchronous and
be
> done
> with the threads (especially since we're still uncovering/introducing
> races!).
>
While scoping out the effort to make vblank events synchronous, I
found
that the spinlock locking order of vblank request sequence and vblank
callback
sequences are the opposite.
In DPU, drm_vblank_enable acquires vblank_time_lock before registering
the crtc to encoder which happens after acquiring encoder_spinlock.
But
the vblank_callback acquires encoder_spinlock before accessing the
registered
crtc and calling into drm_vblank_handler which tries to acquire
vblank_time_lock.
Acquiring both vblank_time_lock and encoder_spinlock in the same
thread
is leading to deadlock.
Hmm, I'm not sure I follow. Are you seeing issues where irq overlaps
with
enable/disable? I hacked in sync vblank enable/disable quickly to see
if I
could
reproduce what you're seeing, but things seemed well behaved.
The race is between drm_vblank_get/put and vblank_handler contexts.
When made synchronous:
while calling drm_vblank_get, the callstack looks like below:
drm_vblank_get -> drm_vblank_enable (acquires vblank_time_lock) ->
__enable_vblank -> dpu_crtc_vblank -> dpu_encoder_toggle_vblank_for_crtc
(tries to acquire enc_spinlock)
In vblank handler, the call stack will be:
dpu_encoder_phys_vid_vblank_irq -> dpu_encoder_vblank_callback (acquires
enc_spinlock) -> dpu_crtc_vblank_callback -> drm_handle_vblank (tries to
acquire vblank_time_lock)
I do see that there is a chance to call drm_handle_vblank() while
holding
enc_spinlock, but couldn't find any obvious lock recursion there.
Maybe a callstack or lockdep splat would help?
Sean
Here's my hack to bypass the display thread:
diff --git a/drivers/gpu/drm/msm/msm_drv.c
b/drivers/gpu/drm/msm/msm_drv.c
index 9c9f7ff6960b38..5a3cac5825319e 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -242,24 +242,19 @@ static void vblank_ctrl_worker(struct
kthread_work
*work)
static int vblank_ctrl_queue_work(struct msm_drm_private *priv,
int crtc_id, bool enable)
{
+ struct msm_kms *kms = priv->kms;
struct msm_vblank_ctrl *vbl_ctrl = >vblank_ctrl;
- struct vblank_event *vbl_ev;
unsigned long flags;
- vbl_ev = kzalloc(sizeof(*vbl_ev), GFP_ATOMIC);
- if (!vbl_ev)
- return -ENOMEM;
+ spin_lock_irqsave(_ctrl->lock, flags);
- vbl_ev->crtc_id = crtc_id;
- vbl_ev->enable = enable;
+ if (enable)
+ kms->funcs->enable_vblank(kms, priv->crtcs[crtc_id]);
+ else
+ kms->funcs->disable_vblank(kms, priv->crtcs[crtc_id]);
- spin_lock_irqsave(_ctrl->lock, flags);
- list_add_tail(_ev->node, _ctrl->event_list);
spin_unlock_irqrestore(_ctrl->lock, flags);
- kthread_queue_work(>disp_thread[crtc_id].worker,
- _ctrl->work);
-
return 0;
}
Even with your patch above, I see frame is getting stuck but it recovers
in a while.
The patch I tried was assigning
crtc->funcs->enable_vblank/disable_vblank so that
__enable_vblank can call crtc directly. But the above callstack is still
valid for your patch.
Thanks,
Jeykumar S.
In MDP5, I see the same pattern between vblank_time_lock and list_lock
which
is used to track the irq handlers.
I believe that explains why msm_drv is queuing the vblank
enable/disable
works to WQ after acquiring vblank_time_lock.
Thanks,
Jeykumar S.
> Sean
>
> >
> > Signed-off-by: Jeykumar Sankaran
> > ---
> > drivers/gpu/drm/msm/msm_drv.c | 67
> +--
> > drivers/gpu/drm/msm/msm_drv.h | 7 -
> > 2 files changed, 20 insertions(+), 54 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c
> b/drivers/gpu/drm/msm/msm_drv.c
> > index 6d6c73b..8da5be2 100644
> > ---
[radeon-alex:drm-next-4.21 68/92] drivers/gpu//drm/amd/amdgpu/sdma_v4_0.c:1524:6: error: 'AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0' undeclared; did you mean 'AMDGPU_DOORBELL64_sDMA_ENGINE0'?
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next-4.21
head: 2c486cc4c2774df684d8a43ca7a20670c67ccd76
commit: 062f380725376efab279956b5441071684c2a7ff [68/92] drm/amdgpu: Vega10
doorbell index initialization
config: x86_64-randconfig-x002-201847 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout 062f380725376efab279956b5441071684c2a7ff
# save the attached .config to linux build tree
make ARCH=x86_64
Note: the radeon-alex/drm-next-4.21 HEAD
2c486cc4c2774df684d8a43ca7a20670c67ccd76 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/gpu//drm/amd/amdgpu/sdma_v4_0.c: In function 'sdma_v4_0_sw_init':
>> drivers/gpu//drm/amd/amdgpu/sdma_v4_0.c:1524:6: error:
>> 'AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0' undeclared (first use in this
>> function); did you mean 'AMDGPU_DOORBELL64_sDMA_ENGINE0'?
(AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1)
^
AMDGPU_DOORBELL64_sDMA_ENGINE0
drivers/gpu//drm/amd/amdgpu/sdma_v4_0.c:1524:6: note: each undeclared
identifier is reported only once for each function it appears in
>> drivers/gpu//drm/amd/amdgpu/sdma_v4_0.c:1525:8: error:
>> 'AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1' undeclared (first use in this
>> function); did you mean 'AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0'?
: (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 << 1);
^
AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0
--
drivers/gpu//drm/amd/amdgpu/amdgpu_amdkfd.c: In function
'amdgpu_amdkfd_device_init':
>> drivers/gpu//drm/amd/amdgpu/amdgpu_amdkfd.c:186:6: error:
>> 'AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0' undeclared (first use in this
>> function); did you mean 'AMDGPU_DOORBELL64_sDMA_ENGINE0'?
AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 + (i >> 1);
^
AMDGPU_DOORBELL64_sDMA_ENGINE0
drivers/gpu//drm/amd/amdgpu/amdgpu_amdkfd.c:186:6: note: each undeclared
identifier is reported only once for each function it appears in
>> drivers/gpu//drm/amd/amdgpu/amdgpu_amdkfd.c:190:6: error:
>> 'AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1' undeclared (first use in this
>> function); did you mean 'AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0'?
AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 + (i >> 1);
^
AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0
vim +1524 drivers/gpu//drm/amd/amdgpu/sdma_v4_0.c
2130f89ce Ken Wang 2017-03-03 1497
2130f89ce Ken Wang 2017-03-03 1498 static int sdma_v4_0_sw_init(void
*handle)
2130f89ce Ken Wang 2017-03-03 1499 {
2130f89ce Ken Wang 2017-03-03 1500struct amdgpu_ring *ring;
2130f89ce Ken Wang 2017-03-03 1501int r, i;
2130f89ce Ken Wang 2017-03-03 1502struct amdgpu_device *adev =
(struct amdgpu_device *)handle;
2130f89ce Ken Wang 2017-03-03 1503
2130f89ce Ken Wang 2017-03-03 1504/* SDMA trap event */
44a99b65f Andrey Grodzovsky 2018-05-25 1505r = amdgpu_irq_add_id(adev,
SOC15_IH_CLIENTID_SDMA0, SDMA0_4_0__SRCID__SDMA_TRAP,
2130f89ce Ken Wang 2017-03-03 1506
>sdma.trap_irq);
2130f89ce Ken Wang 2017-03-03 1507if (r)
2130f89ce Ken Wang 2017-03-03 1508return r;
2130f89ce Ken Wang 2017-03-03 1509
2130f89ce Ken Wang 2017-03-03 1510/* SDMA trap event */
44a99b65f Andrey Grodzovsky 2018-05-25 1511r = amdgpu_irq_add_id(adev,
SOC15_IH_CLIENTID_SDMA1, SDMA1_4_0__SRCID__SDMA_TRAP,
2130f89ce Ken Wang 2017-03-03 1512
>sdma.trap_irq);
2130f89ce Ken Wang 2017-03-03 1513if (r)
2130f89ce Ken Wang 2017-03-03 1514return r;
2130f89ce Ken Wang 2017-03-03 1515
2130f89ce Ken Wang 2017-03-03 1516for (i = 0; i <
adev->sdma.num_instances; i++) {
2130f89ce Ken Wang 2017-03-03 1517ring =
>sdma.instance[i].ring;
2130f89ce Ken Wang 2017-03-03 1518ring->ring_obj = NULL;
2130f89ce Ken Wang 2017-03-03 1519ring->use_doorbell =
true;
2130f89ce Ken Wang 2017-03-03 1520
ec3db8a63 Philip Yang 2018-11-19 1521/* doorbell size is 2
dwords, get DWORD offset */
a2a8fb512 Emily Deng2018-08-09 1522if (adev->asic_type ==
CHIP_VEGA10)
a2a8fb512 Emily Deng2018-08-09 1523
ring->doorbell_index = (i == 0) ?
ec3db8a63 Philip Yang 2018-11-19 @1524
(AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1)
ec3db8a63 Philip Yang 2018-11-19 @1525:
(AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 << 1);
a2a8fb512 Emily Deng2018-08-09 1526else
2130f89ce Ken Wang 2017-03-03 1527
[PATCH RFC 00/15] Zero ****s, hugload of hugs <3
In order to comply with the CoC, replace with a hug. Jarkko Sakkinen (15): MIPS: replace with a hug Documentation: replace with a hug drm/nouveau: replace with a hug m68k: replace with a hug parisc: replace with a hug cpufreq: replace with a hug ide: replace with a hug media: replace with a hug mtd: replace with a hug net/sunhme: replace with a hug scsi: replace with a hug inotify: replace with a hug irq: replace with a hug lib: replace with a hug net: replace with a hug Documentation/kernel-hacking/locking.rst | 2 +- arch/m68k/include/asm/sun3ints.h | 2 +- arch/mips/pci/ops-bridge.c| 24 +-- arch/mips/sgi-ip22/ip22-setup.c | 2 +- arch/parisc/kernel/sys_parisc.c | 2 +- drivers/cpufreq/powernow-k7.c | 2 +- .../gpu/drm/nouveau/nvkm/subdev/bios/init.c | 2 +- .../nouveau/nvkm/subdev/pmu/fuc/macros.fuc| 2 +- drivers/ide/cmd640.c | 2 +- drivers/media/i2c/bt819.c | 8 --- drivers/mtd/mtd_blkdevs.c | 2 +- drivers/net/ethernet/sun/sunhme.c | 4 ++-- drivers/scsi/qlogicpti.h | 2 +- fs/notify/inotify/inotify_user.c | 2 +- kernel/irq/timings.c | 2 +- lib/vsprintf.c| 2 +- net/core/skbuff.c | 2 +- 17 files changed, 33 insertions(+), 31 deletions(-) -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RFC 03/15] drm/nouveau: replace **** with a hug
In order to comply with the CoC, replace with a hug.
Signed-off-by: Jarkko Sakkinen
---
drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c| 2 +-
drivers/gpu/drm/nouveau/nvkm/subdev/pmu/fuc/macros.fuc | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c
index 9cc10e438b3d..55a0060881ea 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c
@@ -446,7 +446,7 @@ init_ram_restrict(struct nvbios_init *init)
{
/* This appears to be the behaviour of the VBIOS parser, and *is*
* important to cache the NV_PEXTDEV_BOOT0 on later chipsets to
-* avoid fucking up the memory controller (somehow) by reading it
+* avoid hugging up the memory controller (somehow) by reading it
* on every INIT_RAM_RESTRICT_ZM_GROUP opcode.
*
* Preserving the non-caching behaviour on earlier chipsets just
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/fuc/macros.fuc
b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/fuc/macros.fuc
index 3737bd27f74e..ee364c71cc2e 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/fuc/macros.fuc
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/fuc/macros.fuc
@@ -46,7 +46,7 @@
#define NV_PPWR_INTR_EN_SET_SUBINTR 0x0800
#define NV_PPWR_INTR_EN_SET_WATCHDOG 0x0002
#define NV_PPWR_INTR_EN_CLR 0x0014
-#define NV_PPWR_INTR_EN_CLR_MASK/* fuck i hate envyas */ -1
+#define NV_PPWR_INTR_EN_CLR_MASK/* hug i hate envyas */ -1
#define NV_PPWR_INTR_ROUTE 0x001c
#define NV_PPWR_TIMER_LOW0x002c
#define NV_PPWR_WATCHDOG_TIME0x0034
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[pull] amdgpu, amdkfd, ttm, scheduler, radeon drm-next-4.21
Hi Dave, More new features for 4.21: amdgpu and amdkfd: - Freesync support - ABM support in DC - KFD support for vega12 and polaris12 - Add sdma paging queue support for vega - Use ACPI to query backlight range on supported platforms - Clean up doorbell handling - KFD fix for pasid handling under non-HWS - Misc cleanups and fixes scheduler: - Revert "fix timeout handling v2" radeon: - Fix possible overflow on 32 bit ttm: - Fix for LRU handling for ghost objects The following changes since commit 9235dd441af43599b9cdcce599a3da4083fcad3c: Merge branch 'drm-next-4.21' of git://people.freedesktop.org/~agd5f/linux into drm-next (2018-11-19 11:07:52 +1000) are available in the git repository at: git://people.freedesktop.org/~agd5f/linux drm-next-4.21 for you to fetch changes up to 2c486cc4c2774df684d8a43ca7a20670c67ccd76: drm/amdgpu: wait for IB test on first device open (2018-11-30 12:01:35 -0500) Alex Deucher (4): drm/amdgpu/gfx: use proper offset define for MEC doorbells drm/amdgpu/psp: use define rather than magic number for mode1 reset drm/amdgpu: don't expose fan attributes on APUs drm/amdgpu: add VCN JPEG support amdgpu_ctx_num_entities Andrey Grodzovsky (3): drm/amdgpu: Refactor amdgpu_xgmi_add_device drm/amdgpu: Expose hive adev list and xgmi_mutex drm/amdgpu: Refactor GPU reset for XGMI hive case Bhawanpreet Lakha (2): drm/amd/display: Set RMX_ASPECT as default drm/amd/display: Fix Scaling (RMX_*) for DC driver Brajeswar Ghosh (7): drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c: Remove duplicate header drm/amd/amdgpu/vce_v3_0.c: Remove duplicate header drm/amd/amdgpu: Remove duplicate header drm/amd/display/amdgpu_dm/amdgpu_dm.c: Remove duplicate header drm/amd/amdgpu: Remove duplicate header drm/amd/amdkfd: Remove duplicate header drm/amd/display: Remove duplicate header Charlene Liu (1): drm/amd/display: expose surface confirm color function Chengming Gui (1): Revert "drm/amdgpu: use GMC v9 KIQ workaround only for the GFXHUB" (v2) Chris Wilson (1): drm/amdgpu: Reorder uvd ring init before uvd resume Christian König (3): drm/sched: revert "fix timeout handling v2" v2 drm/ttm: fix LRU handling in ttm_buffer_object_transfer drm/amdgpu: wait for IB test on first device open Colin Ian King (3): drm/amd/display: fix dereference of pointer fs_params before it is null checked drm/amdgpu: fix spelling mistake "Pramater" -> "Parameter" drm/amd/pp: fix spelling mistake "dependancy" -> "dependency" David Francis (10): drm/amd/display: Remove dc_stream_state->status drm/amd/display: Check for dmcu initialization before calling dmcu drm/amd/display: Clean up dp_blank functions drm/amd/display: Get backlight controller id from link drm/amd/display: Clean up DCN1 clock requests drm/amd/display: Load DMCU IRAM drm/amd: Add abm level drm property drm/amd: update ATIF functions in AMD ACPI header drm/amd: Query and use ACPI backlight caps drm/amd/display: Fix compile error with ACPI disabled Dmytro Laktyushkin (2): drm/amd/display: redesign scaling rotation math drm/amd/display: fix pipe interdependent hubp programming Emily Deng (1): drm/amd/amdgpu/sriov: Aligned the definition with libgv Eric Bernstein (1): drm/amd/display: get tail pipe before aquire free pipe Eric Huang (1): drm/amdkfd: change system memory overcommit limit Felix Kuehling (2): drm/amdkfd: Fix and simplify sync object handling for KFD drm/amdgpu: Fix KFD doorbell SG BO mapping Gang Ba (1): drm/amdkfd: Added Vega12 and Polaris12 for KFD. Guttula, Suresh (2): drm/amd/powerplay:add hwmgr callback to update nbpstate on Carrizo drm/amd:Enable/Disable NBPSTATE on On/OFF of UVD Harish Kasiviswanathan (2): drm/amdgpu: Remove explicit wait after VM validate drm/amdgpu: KFD Restore process: Optimize waiting Jerry (Fangzhi) Zuo (1): drm/amd/display: Fix NULL ptr when calculating refresh rate Joerg Roedel (1): drm/amd/powerplay: Ratelimit all "was not implemented" messages Joshua Aberback (1): drm/amd/display: Adjust stream enable sequence Jun Lei (2): drm/amd/display: make underflow status clear explicit drm/amd/display: clear underflow on optc unblank Murton Liu (1): drm/amd/display: fix gamma not being applied correctly Nevenko Stupar (1): drm/amd/display: expose dentist_get_divider_from_did Nicholas Kazlauskas (9): drm/amdgpu: Add amdgpu "max bpc" connector property (v2) drm/amd/display: Support amdgpu "max bpc" connector property (v2) drm/amd/display: Use private obj helpers for dm_atomic_state drm: Add vrr_capable property to the drm connector drm: Add vrr_enabled property to drm CRTC drm: Document
Re: [PATCH v2] drm/vc4: Add a debugfs entry to disable/enable the load tracker
On Fri, 30 Nov 2018 17:11:04 +0100
Paul Kocialkowski wrote:
> In order to test whether the load tracker is working as expected, we
> need the ability to compare the commit result with the underrun
> indication. With the load tracker always enabled, commits that are
> expected to trigger an underrun are always rejected, so userspace
> cannot get the actual underrun indication from the hardware.
>
> Add a debugfs entry to disable/enable the load tracker, so that a DRM
> commit expected to trigger an underrun can go through with the load
> tracker disabled. The underrun indication is then available to
> userspace and can be checked against the commit result with the load
> tracker enabled.
>
> Signed-off-by: Paul Kocialkowski
> ---
>
> Changes since v1:
> * Moved all the debugfs-related functions and structure to vc4_debugfs.c.
>
> drivers/gpu/drm/vc4/vc4_debugfs.c | 58 +++
> drivers/gpu/drm/vc4/vc4_drv.c | 2 ++
> drivers/gpu/drm/vc4/vc4_drv.h | 2 ++
> drivers/gpu/drm/vc4/vc4_kms.c | 6 +++-
> 4 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c
> b/drivers/gpu/drm/vc4/vc4_debugfs.c
> index 7a0003de71ab..8f4d7fadb226 100644
> --- a/drivers/gpu/drm/vc4/vc4_debugfs.c
> +++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
> @@ -32,9 +32,67 @@ static const struct drm_info_list vc4_debugfs_list[] = {
>
> #define VC4_DEBUGFS_ENTRIES ARRAY_SIZE(vc4_debugfs_list)
>
> +static int vc4_debugfs_load_tracker_get(struct seq_file *m, void *data)
> +{
> + struct drm_device *dev = m->private;
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> +
> + if (vc4->load_tracker_enabled)
> + seq_printf(m, "enabled\n");
> + else
> + seq_printf(m, "disabled\n");
> +
> + return 0;
> +}
> +
> +static ssize_t vc4_debugfs_load_tracker_set(struct file *file,
> + const char __user *ubuf,
> + size_t len, loff_t *offp)
> +{
> + struct seq_file *m = file->private_data;
> + struct drm_device *dev = m->private;
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + char buf[32] = {};
> +
> + if (len >= sizeof(buf))
> + return -EINVAL;
> +
> + if (copy_from_user(buf, ubuf, len))
> + return -EFAULT;
> +
> + if (!strncasecmp(buf, "enable", 6))
> + vc4->load_tracker_enabled = true;
> + else if (!strncasecmp(buf, "disable", 7))
> + vc4->load_tracker_enabled = false;
> + else
> + return -EINVAL;
> +
> + return len;
> +}
> +
> +static int vc4_debugfs_load_tracker_open(struct inode *inode, struct file
> *file)
> +{
> + return single_open(file, vc4_debugfs_load_tracker_get,
> inode->i_private);
> +}
> +
> +static const struct file_operations vc4_debugfs_load_tracker_fops = {
> + .owner = THIS_MODULE,
> + .open = vc4_debugfs_load_tracker_open,
> + .read = seq_read,
> + .write = vc4_debugfs_load_tracker_set,
> +};
> +
> int
> vc4_debugfs_init(struct drm_minor *minor)
> {
> + struct dentry *dentry;
> +
> + dentry = debugfs_create_file("load_tracker", S_IRUGO | S_IWUSR,
> + minor->debugfs_root, minor->dev,
> + _debugfs_load_tracker_fops);
> + if (!dentry)
> + return -ENOMEM;
Hm, maybe you could use debugfs_create_bool() instead of
re-implementing it. The values won't be enabled/disabled though
(IIRC, it's Y or N).
> +
> return drm_debugfs_create_files(vc4_debugfs_list, VC4_DEBUGFS_ENTRIES,
> minor->debugfs_root, minor);
> }
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index 7195a0bcceb3..12ac971d24d6 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -290,6 +290,8 @@ static int vc4_drm_bind(struct device *dev)
>
> drm_fbdev_generic_setup(drm, 32);
>
> + vc4->load_tracker_enabled = true;
> +
> return 0;
>
> unbind_all:
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 11369da944b6..8d0f2f16a9e8 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -188,6 +188,8 @@ struct vc4_dev {
>
> int power_refcount;
>
> + bool load_tracker_enabled;
> +
> /* Mutex controlling the power refcount. */
> struct mutex power_lock;
>
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index b905a19c1470..4b2509b1b8ed 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -473,6 +473,7 @@ static const struct drm_private_state_funcs
> vc4_load_tracker_state_funcs = {
> static int
> vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
> {
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> int ret;
>
> ret = vc4_ctm_atomic_check(dev, state);
>
Re: [git pull] drm fixes for 4.20-rc5
The pull request you sent on Fri, 30 Nov 2018 13:11:42 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2018-11-30 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/570a37437cf24790d77fed6a59fdc9ac749e6b19 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v3 06/19] arch: um: enable running kunit from User Mode Linux
On Fri, Nov 30, 2018 at 08:05:34AM -0600, Rob Herring wrote: > On Thu, Nov 29, 2018 at 9:37 PM Luis Chamberlain wrote: > > > > On Wed, Nov 28, 2018 at 03:26:03PM -0600, Rob Herring wrote: > > > On Wed, Nov 28, 2018 at 1:37 PM Brendan Higgins > > > wrote: > > > > > > > > Make minimum number of changes outside of the KUnit directories for > > > > KUnit to build and run using UML. > > > > > > There's nothing in this patch limiting this to UML. > > > > Not that one, but the abort thing segv thing is, eventually. > > To support other architectures we'd need to make a wrapper to that > > hack which Brendan added, and then allow each os to implement > > its own call, and add an asm-generic helper. > > I've not looked into why this is needed, but can't you make the abort > support optional and arches can select it when they support it. Its why I have asked for it to be properly documented. The patches in no way illustrate *why* such thing is done. And if we are going to potentially have other archs do something similar best to make it explicit. > At > least before, the DT unittests didn't need this to run and shouldn't > depend on it after converting to kunit. Luis ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201497] [amdgpu]: '*ERROR* No EDID read' is back in 4.19
https://bugzilla.kernel.org/show_bug.cgi?id=201497 --- Comment #3 from Daniel Andersson (engyw...@gmail.com) --- Any progress on this issue? -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 92936] Tonga powerplay isssues
https://bugs.freedesktop.org/show_bug.cgi?id=92936 Andy Furniss changed: What|Removed |Added Resolution|--- |WORKSFORME Status|REOPENED|RESOLVED --- Comment #24 from Andy Furniss --- Doesn't lock with a brief test, though doesn't always decode properly with mpv vdpau. Solid with vaapi and seen as vdpau is dead I care less about testing with it. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 104391] DC R9 285 HDMI audio regression since drm/amd/display: try to find matching audio inst for enc inst first
https://bugs.freedesktop.org/show_bug.cgi?id=104391 --- Comment #9 from Andy Furniss --- I have a new TV now and can't get hdmi audio working at all. Maybe my use/setup of alsa doesn't help, but IIRC I did try and fail with a fedora live USB as well. It works perfectly if I boot into Windows 7, so I know this TV/card combo is OK. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
no handling of fbdev patches till 20.12
Hi, I will be traveling / busy with other things / without access to my development setup till 20.12. When I'm back I will handle the rest of pending v4.21 changes, sorry for the inconvenience. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [v2, 1/8] drm: Add optional COLOR_ENCODING and COLOR_RANGE properties to drm_plane
Hi, On Fri, Nov 30, 2018 at 04:34:54PM +0100, Hans Verkuil wrote: > On 11/30/18 16:16, Ville Syrjälä wrote: > > On Fri, Nov 30, 2018 at 03:48:00PM +0100, Hans Verkuil wrote: > >> On 11/30/18 15:29, Ville Syrjälä wrote: > >>> On Fri, Nov 30, 2018 at 03:20:59PM +0100, Andrzej Hajda wrote: > Hi Ville, > > As Christoph cannot respond till middle next week I can try to respond > in his absence, as I am familiar with the subject. > > On 30.11.2018 14:25, Ville Syrjälä wrote: > > On Fri, Nov 30, 2018 at 02:08:11PM +0100, Christoph Manszewski wrote: > >> Hi, > >> > >> I am looking for a way to export the color encoding and range selection > >> to user space. I came across those properties and am wondering, why > >> they are meant only for non RGB color encodings. Would it be okay, to > >> modify them and use with RGB formats as well? > > What you trying to do? Input limited range RGB data and expand to full > > range? > > > For example. But there are two more general questions, which > surprisingly we have not found answer for. > > 1. What color encoding and range drm should expect on its input RGB > buffers by default? This is where I personally think we've got an unfortunate disconnect in the KMS UAPI. For YCbCr buffers, these properties specify the encoding and range of the data in the buffer. But everything else in the pipe is described in terms of the processing to apply - i.e. the KMS driver doesn't know what transfer function the data uses, it only knows the degamma LUT it's told to apply to it. It would have been more uniform if the COLOR_ENCODING/COLOR_RANGE properties were a single "ENCODING_CONVERSION" property stating what conversion should be applied. > >>> > >>> RGB is just RGB. There is no encoding. It's assumed to be full range > >>> because no one really uses anything else. > >> > >> For simple desktop usage that's true. When dealing with video inputs, > >> this becomes much more complicated. > >> When the plane degamma/ctm/gamma properties land, those could be used to convert limited range to whatever the pipe-internal format is, I think. That pipe-internal format would be whatever userspace decides it is, via converting input buffers using the various color conversion properties. > >>> > > 2. How userspace should inform drm that given buffer has specified > non-default color encoding and range? > My understanding is that DRM would never be informed of this - only what to do with the data (which does of-course imply an encoding, but it's not told to DRM explicitly). > > Hopefully this patch introduces such properties but only for YCbCr > formats, the question is what should be the best way to expand it to RGB > formats: > > A. Add another enums: DRM_COLOR_RGB_BT601 and friends. > >>> > >>> BT.601 specifies how to encoder RGB data as YCbCr. So without > >>> YCbCr BT.601 does not mean anything. Well, the standard does > >>> contain other things as well I suppose, but for the purposes > >>> of the color encoding prop only that one part is relevant. > >> > >> Ah, I misunderstood the meaning of DRM_COLOR_RGB_BT601. > >> This is the equivalent of V4L2_YCBCR_ENC_601, and that's indeed > >> only defined for Y'CbCr. But it is often (ab)used as an alias for > >> the SMPTE170M colorspace (used by SDTV). > >> > >> V4L2 has the following defines for colorspaces, transfer functions, > >> Y'CbCr (and HSV) encodings and quantization ranges: > >> > >> https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces-defs.html > > > > Yeah, we're going to be introducing other properties to control > > colorspace and transfer function in kms as well. Actually some > > patches towards that have been floated a few times already. > > > > Great. Let's try to keep drm and V4L2 in sync for this. It should be > possible to convert from one to the other without having to do weird > things. > > I'll try to pay attention to these patches, but just ping me if you > want me to take a look at something. > > I put a lot of effort into the V4L2 colorspace documentation, trying to > put all the information in one place, esp. all the formulas. There's always going to be a bit of a disconnect here - in KMS, it's userspace which needs to handle all this stuff. It would be up to userspace to set e.g. DEGAMMA_LUT to a LUT which corresponds to SMPTE 2084, rather than the kernel driver being told directly that the buffer is encoded using the SMPTE 2084 transfer function. Actually I want to put an RFC together to allow DEGAMMA_LUT/GAMMA_LUT to be set to some pre-defined values (e.g. sRGB, PQ, HLG) to suit hardware which has built-in hard-coded transfer functions (and potentially also save userspace some effort of coming up with LUTs). Cheers, -Brian > > Regards, > > Hans > ___ > Intel-gfx mailing list >
[PATCH v2] drm/vc4: Add a debugfs entry to disable/enable the load tracker
In order to test whether the load tracker is working as expected, we
need the ability to compare the commit result with the underrun
indication. With the load tracker always enabled, commits that are
expected to trigger an underrun are always rejected, so userspace
cannot get the actual underrun indication from the hardware.
Add a debugfs entry to disable/enable the load tracker, so that a DRM
commit expected to trigger an underrun can go through with the load
tracker disabled. The underrun indication is then available to
userspace and can be checked against the commit result with the load
tracker enabled.
Signed-off-by: Paul Kocialkowski
---
Changes since v1:
* Moved all the debugfs-related functions and structure to vc4_debugfs.c.
drivers/gpu/drm/vc4/vc4_debugfs.c | 58 +++
drivers/gpu/drm/vc4/vc4_drv.c | 2 ++
drivers/gpu/drm/vc4/vc4_drv.h | 2 ++
drivers/gpu/drm/vc4/vc4_kms.c | 6 +++-
4 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c
b/drivers/gpu/drm/vc4/vc4_debugfs.c
index 7a0003de71ab..8f4d7fadb226 100644
--- a/drivers/gpu/drm/vc4/vc4_debugfs.c
+++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
@@ -32,9 +32,67 @@ static const struct drm_info_list vc4_debugfs_list[] = {
#define VC4_DEBUGFS_ENTRIES ARRAY_SIZE(vc4_debugfs_list)
+static int vc4_debugfs_load_tracker_get(struct seq_file *m, void *data)
+{
+ struct drm_device *dev = m->private;
+ struct vc4_dev *vc4 = to_vc4_dev(dev);
+
+ if (vc4->load_tracker_enabled)
+ seq_printf(m, "enabled\n");
+ else
+ seq_printf(m, "disabled\n");
+
+ return 0;
+}
+
+static ssize_t vc4_debugfs_load_tracker_set(struct file *file,
+ const char __user *ubuf,
+ size_t len, loff_t *offp)
+{
+ struct seq_file *m = file->private_data;
+ struct drm_device *dev = m->private;
+ struct vc4_dev *vc4 = to_vc4_dev(dev);
+ char buf[32] = {};
+
+ if (len >= sizeof(buf))
+ return -EINVAL;
+
+ if (copy_from_user(buf, ubuf, len))
+ return -EFAULT;
+
+ if (!strncasecmp(buf, "enable", 6))
+ vc4->load_tracker_enabled = true;
+ else if (!strncasecmp(buf, "disable", 7))
+ vc4->load_tracker_enabled = false;
+ else
+ return -EINVAL;
+
+ return len;
+}
+
+static int vc4_debugfs_load_tracker_open(struct inode *inode, struct file
*file)
+{
+ return single_open(file, vc4_debugfs_load_tracker_get,
inode->i_private);
+}
+
+static const struct file_operations vc4_debugfs_load_tracker_fops = {
+ .owner = THIS_MODULE,
+ .open = vc4_debugfs_load_tracker_open,
+ .read = seq_read,
+ .write = vc4_debugfs_load_tracker_set,
+};
+
int
vc4_debugfs_init(struct drm_minor *minor)
{
+ struct dentry *dentry;
+
+ dentry = debugfs_create_file("load_tracker", S_IRUGO | S_IWUSR,
+minor->debugfs_root, minor->dev,
+_debugfs_load_tracker_fops);
+ if (!dentry)
+ return -ENOMEM;
+
return drm_debugfs_create_files(vc4_debugfs_list, VC4_DEBUGFS_ENTRIES,
minor->debugfs_root, minor);
}
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 7195a0bcceb3..12ac971d24d6 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -290,6 +290,8 @@ static int vc4_drm_bind(struct device *dev)
drm_fbdev_generic_setup(drm, 32);
+ vc4->load_tracker_enabled = true;
+
return 0;
unbind_all:
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 11369da944b6..8d0f2f16a9e8 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -188,6 +188,8 @@ struct vc4_dev {
int power_refcount;
+ bool load_tracker_enabled;
+
/* Mutex controlling the power refcount. */
struct mutex power_lock;
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index b905a19c1470..4b2509b1b8ed 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -473,6 +473,7 @@ static const struct drm_private_state_funcs
vc4_load_tracker_state_funcs = {
static int
vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
{
+ struct vc4_dev *vc4 = to_vc4_dev(dev);
int ret;
ret = vc4_ctm_atomic_check(dev, state);
@@ -483,7 +484,10 @@ vc4_atomic_check(struct drm_device *dev, struct
drm_atomic_state *state)
if (ret)
return ret;
- return vc4_load_tracker_atomic_check(state);
+ if (vc4->load_tracker_enabled)
+ return vc4_load_tracker_atomic_check(state);
+
+ return 0;
}
static const struct drm_mode_config_funcs vc4_mode_funcs = {
--
[Bug 108689] new version of intel_gpu_top lost option of output log to stdio or file
https://bugs.freedesktop.org/show_bug.cgi?id=108689 --- Comment #3 from Alex <3.1...@ukr.net> --- Any news? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH v1] drm/bridge: fix AUX_CMD_SEND bit value for ti, sn65dsi86 bridge
On Fri, Nov 30, 2018 at 02:57:45PM +0530, Sandeep Panda wrote: > Fix the AUX_CMD_SEND bit for ti,sn65dsi86 bridge chip. With wrong > value the dpcd aux transactions with eDP panel are failing. > > Signed-off-by: Sandeep Panda Pushed to -misc-fixes, thanks for the patch Sean > --- > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > index 680566d97adcf..10243965ee7c0 100644 > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > @@ -54,7 +54,7 @@ > #define SN_AUX_ADDR_7_0_REG 0x76 > #define SN_AUX_LENGTH_REG0x77 > #define SN_AUX_CMD_REG 0x78 > -#define AUX_CMD_SENDBIT(1) > +#define AUX_CMD_SENDBIT(0) > #define AUX_CMD_REQ(x) ((x) << 4) > #define SN_AUX_RDATA_REG(x) (0x79 + (x)) > #define SN_SSC_CONFIG_REG0x93 > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > > ___ > Freedreno mailing list > freedr...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/msm: bump UAPI version
On Fri, Nov 30, 2018 at 10:36 AM Arnd Bergmann wrote:
>
> On Fri, Nov 30, 2018 at 4:31 PM Rob Clark wrote:
> >
> > On Fri, Nov 30, 2018 at 10:12 AM Arnd Bergmann wrote:
> > >
> > > On Fri, Nov 30, 2018 at 4:02 PM Rob Clark wrote:
> > > >
> > > > Signed-off-by: Rob Clark
> > > > ---
> > > > drivers/gpu/drm/msm/msm_drv.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/msm/msm_drv.c
> > > > b/drivers/gpu/drm/msm/msm_drv.c
> > > > index 6ebbd5010722..782cc33916d6 100644
> > > > --- a/drivers/gpu/drm/msm/msm_drv.c
> > > > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > > > @@ -36,9 +36,11 @@
> > > > * - 1.3.0 - adds GMEM_BASE + NR_RINGS params, SUBMITQUEUE_NEW +
> > > > * SUBMITQUEUE_CLOSE ioctls, and MSM_INFO_IOVA flag for
> > > > * MSM_GEM_INFO ioctl.
> > > > + * - 1.4.0 - softpin, MSM_RELOC_BO_DUMP, and GEM_INFO support to
> > > > set/get
> > > > + * GEM object's debug name
> > > > */
> > > > #define MSM_VERSION_MAJOR 1
> > > > -#define MSM_VERSION_MINOR 3
> > > > +#define MSM_VERSION_MINOR 4
> > > > #define MSM_VERSION_PATCHLEVEL 0
> > > >
> > >
> > > I don't know the background here, but generally speaking we don't have
> > > version numbers for ioctls in kernel drivers. Instead, the old ioctls
> > > need to remain functional, but you can add new ioctl commands
> > > in addition.
> > >
> > > Is there something that makes this driver special?
> > >
> >
> > The version # indicates to userspace that some new features are
> > supported, so that new userspace on kernel can work. For example, the
> > userspace side of setting a GEM obj debug name is:
>
> Ok, got it.
>
> > static void msm_bo_set_name(struct fd_bo *bo, const char *fmt, va_list ap)
> > {
> > struct drm_msm_gem_info req = {
> > .handle = bo->handle,
> > .info = MSM_INFO_SET_NAME,
> > };
> > char buf[32];
> > int sz;
> >
> > /* bail if kernel doesn't support this: */
> > if (bo->dev->version < FD_VERSION_SOFTPIN)
> > return;
> >
> > sz = vsnprintf(buf, sizeof(buf), fmt, ap);
> >
> > req.value = VOID2U64(buf);
> > req.len = MIN2(sz, sizeof(buf));
> >
> > drmCommandWrite(bo->dev->fd, DRM_MSM_GEM_INFO, , sizeof(req));
> > }
>
> So that version check seems harmless, but also not necessary,
> at least in this case, right? I would assume that calling into
> drmCommandWrite() with an invalid command will only return
> an error, which then gets ignored, where with the check, we
> would skip the call, knowing that it wont't work.
In this case, yes, probably a better example would be v1.1.0 when
support for >4 cmd buffers was added. For older kernels userspace has
to structure the cmdstream in a less efficient way to work around that
limitation.
I guess these are things where a dummy ioctl call to probe whether
kernel returns an error could be used.. but that gets awkward.
BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm: Fix up drm_atomic_state_helper.[hc] extraction
On Thu, Nov 29, 2018 at 10:36:13AM -0500, Sean Paul wrote:
> On Wed, Nov 28, 2018 at 5:07 AM Daniel Vetter wrote:
> >
> > I've misplaced two functions by accident:
> > - drm_atomic_helper_duplicate_state is really part of the
> > resume/suspend/shutdown device-wide helpers.
> > - drm_atomic_helper_legacy_gamma_set is part of the legacy ioctl
> > compat helpers.
> >
> > Move them both back.
> >
> > Fixes: 9ef8a9dc4b21 ("drm: Extract drm_atomic_state_helper.[hc]")
> > Cc: Ville Syrjälä
> > Signed-off-by: Daniel Vetter
>
> Reviewed-by: Sean Paul
Applied, thanks for reviewing.
-Daniel
>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 157 ++
> > drivers/gpu/drm/drm_atomic_state_helper.c | 157 --
> > include/drm/drm_atomic_helper.h | 7 +
> > include/drm/drm_atomic_state_helper.h | 7 -
> > 4 files changed, 164 insertions(+), 164 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 9b22774a9867..de7d872f9f1a 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3135,6 +3135,93 @@ void drm_atomic_helper_shutdown(struct drm_device
> > *dev)
> > }
> > EXPORT_SYMBOL(drm_atomic_helper_shutdown);
> >
> > +/**
> > + * drm_atomic_helper_duplicate_state - duplicate an atomic state object
> > + * @dev: DRM device
> > + * @ctx: lock acquisition context
> > + *
> > + * Makes a copy of the current atomic state by looping over all objects and
> > + * duplicating their respective states. This is used for example by
> > suspend/
> > + * resume support code to save the state prior to suspend such that it can
> > + * be restored upon resume.
> > + *
> > + * Note that this treats atomic state as persistent between save and
> > restore.
> > + * Drivers must make sure that this is possible and won't result in
> > confusion
> > + * or erroneous behaviour.
> > + *
> > + * Note that if callers haven't already acquired all modeset locks this
> > might
> > + * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
> > + *
> > + * Returns:
> > + * A pointer to the copy of the atomic state object on success or an
> > + * ERR_PTR()-encoded error code on failure.
> > + *
> > + * See also:
> > + * drm_atomic_helper_suspend(), drm_atomic_helper_resume()
> > + */
> > +struct drm_atomic_state *
> > +drm_atomic_helper_duplicate_state(struct drm_device *dev,
> > + struct drm_modeset_acquire_ctx *ctx)
> > +{
> > + struct drm_atomic_state *state;
> > + struct drm_connector *conn;
> > + struct drm_connector_list_iter conn_iter;
> > + struct drm_plane *plane;
> > + struct drm_crtc *crtc;
> > + int err = 0;
> > +
> > + state = drm_atomic_state_alloc(dev);
> > + if (!state)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + state->acquire_ctx = ctx;
> > +
> > + drm_for_each_crtc(crtc, dev) {
> > + struct drm_crtc_state *crtc_state;
> > +
> > + crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > + if (IS_ERR(crtc_state)) {
> > + err = PTR_ERR(crtc_state);
> > + goto free;
> > + }
> > + }
> > +
> > + drm_for_each_plane(plane, dev) {
> > + struct drm_plane_state *plane_state;
> > +
> > + plane_state = drm_atomic_get_plane_state(state, plane);
> > + if (IS_ERR(plane_state)) {
> > + err = PTR_ERR(plane_state);
> > + goto free;
> > + }
> > + }
> > +
> > + drm_connector_list_iter_begin(dev, _iter);
> > + drm_for_each_connector_iter(conn, _iter) {
> > + struct drm_connector_state *conn_state;
> > +
> > + conn_state = drm_atomic_get_connector_state(state, conn);
> > + if (IS_ERR(conn_state)) {
> > + err = PTR_ERR(conn_state);
> > + drm_connector_list_iter_end(_iter);
> > + goto free;
> > + }
> > + }
> > + drm_connector_list_iter_end(_iter);
> > +
> > + /* clear the acquire context so that it isn't accidentally reused */
> > + state->acquire_ctx = NULL;
> > +
> > +free:
> > + if (err < 0) {
> > + drm_atomic_state_put(state);
> > + state = ERR_PTR(err);
> > + }
> > +
> > + return state;
> > +}
> > +EXPORT_SYMBOL(drm_atomic_helper_duplicate_state);
> > +
> > /**
> > * drm_atomic_helper_suspend - subsystem-level suspend helper
> > * @dev: DRM device
> > @@ -3418,3 +3505,73 @@ int drm_atomic_helper_page_flip_target(struct
> > drm_crtc *crtc,
> > return ret;
> > }
> > EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
> > +
> > +/**
> > + * drm_atomic_helper_legacy_gamma_set - set the legacy gamma correction
>
Re: [PATCH 4/4] drm/msm: bump UAPI version
On Fri, Nov 30, 2018 at 4:31 PM Rob Clark wrote:
>
> On Fri, Nov 30, 2018 at 10:12 AM Arnd Bergmann wrote:
> >
> > On Fri, Nov 30, 2018 at 4:02 PM Rob Clark wrote:
> > >
> > > Signed-off-by: Rob Clark
> > > ---
> > > drivers/gpu/drm/msm/msm_drv.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > > index 6ebbd5010722..782cc33916d6 100644
> > > --- a/drivers/gpu/drm/msm/msm_drv.c
> > > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > > @@ -36,9 +36,11 @@
> > > * - 1.3.0 - adds GMEM_BASE + NR_RINGS params, SUBMITQUEUE_NEW +
> > > * SUBMITQUEUE_CLOSE ioctls, and MSM_INFO_IOVA flag for
> > > * MSM_GEM_INFO ioctl.
> > > + * - 1.4.0 - softpin, MSM_RELOC_BO_DUMP, and GEM_INFO support to set/get
> > > + * GEM object's debug name
> > > */
> > > #define MSM_VERSION_MAJOR 1
> > > -#define MSM_VERSION_MINOR 3
> > > +#define MSM_VERSION_MINOR 4
> > > #define MSM_VERSION_PATCHLEVEL 0
> > >
> >
> > I don't know the background here, but generally speaking we don't have
> > version numbers for ioctls in kernel drivers. Instead, the old ioctls
> > need to remain functional, but you can add new ioctl commands
> > in addition.
> >
> > Is there something that makes this driver special?
> >
>
> The version # indicates to userspace that some new features are
> supported, so that new userspace on kernel can work. For example, the
> userspace side of setting a GEM obj debug name is:
Ok, got it.
> static void msm_bo_set_name(struct fd_bo *bo, const char *fmt, va_list ap)
> {
> struct drm_msm_gem_info req = {
> .handle = bo->handle,
> .info = MSM_INFO_SET_NAME,
> };
> char buf[32];
> int sz;
>
> /* bail if kernel doesn't support this: */
> if (bo->dev->version < FD_VERSION_SOFTPIN)
> return;
>
> sz = vsnprintf(buf, sizeof(buf), fmt, ap);
>
> req.value = VOID2U64(buf);
> req.len = MIN2(sz, sizeof(buf));
>
> drmCommandWrite(bo->dev->fd, DRM_MSM_GEM_INFO, , sizeof(req));
> }
So that version check seems harmless, but also not necessary,
at least in this case, right? I would assume that calling into
drmCommandWrite() with an invalid command will only return
an error, which then gets ignored, where with the check, we
would skip the call, knowing that it wont't work.
Arnd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v2, 1/8] drm: Add optional COLOR_ENCODING and COLOR_RANGE properties to drm_plane
On 11/30/18 16:16, Ville Syrjälä wrote: > On Fri, Nov 30, 2018 at 03:48:00PM +0100, Hans Verkuil wrote: >> On 11/30/18 15:29, Ville Syrjälä wrote: >>> On Fri, Nov 30, 2018 at 03:20:59PM +0100, Andrzej Hajda wrote: Hi Ville, As Christoph cannot respond till middle next week I can try to respond in his absence, as I am familiar with the subject. On 30.11.2018 14:25, Ville Syrjälä wrote: > On Fri, Nov 30, 2018 at 02:08:11PM +0100, Christoph Manszewski wrote: >> Hi, >> >> I am looking for a way to export the color encoding and range selection >> to user space. I came across those properties and am wondering, why >> they are meant only for non RGB color encodings. Would it be okay, to >> modify them and use with RGB formats as well? > What you trying to do? Input limited range RGB data and expand to full > range? For example. But there are two more general questions, which surprisingly we have not found answer for. 1. What color encoding and range drm should expect on its input RGB buffers by default? >>> >>> RGB is just RGB. There is no encoding. It's assumed to be full range >>> because no one really uses anything else. >> >> For simple desktop usage that's true. When dealing with video inputs, >> this becomes much more complicated. >> >>> 2. How userspace should inform drm that given buffer has specified non-default color encoding and range? Hopefully this patch introduces such properties but only for YCbCr formats, the question is what should be the best way to expand it to RGB formats: A. Add another enums: DRM_COLOR_RGB_BT601 and friends. >>> >>> BT.601 specifies how to encoder RGB data as YCbCr. So without >>> YCbCr BT.601 does not mean anything. Well, the standard does >>> contain other things as well I suppose, but for the purposes >>> of the color encoding prop only that one part is relevant. >> >> Ah, I misunderstood the meaning of DRM_COLOR_RGB_BT601. >> This is the equivalent of V4L2_YCBCR_ENC_601, and that's indeed >> only defined for Y'CbCr. But it is often (ab)used as an alias for >> the SMPTE170M colorspace (used by SDTV). >> >> V4L2 has the following defines for colorspaces, transfer functions, >> Y'CbCr (and HSV) encodings and quantization ranges: >> >> https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces-defs.html > > Yeah, we're going to be introducing other properties to control > colorspace and transfer function in kms as well. Actually some > patches towards that have been floated a few times already. > Great. Let's try to keep drm and V4L2 in sync for this. It should be possible to convert from one to the other without having to do weird things. I'll try to pay attention to these patches, but just ping me if you want me to take a look at something. I put a lot of effort into the V4L2 colorspace documentation, trying to put all the information in one place, esp. all the formulas. Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/msm: bump UAPI version
On Fri, Nov 30, 2018 at 10:12 AM Arnd Bergmann wrote:
>
> On Fri, Nov 30, 2018 at 4:02 PM Rob Clark wrote:
> >
> > Signed-off-by: Rob Clark
> > ---
> > drivers/gpu/drm/msm/msm_drv.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > index 6ebbd5010722..782cc33916d6 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@ -36,9 +36,11 @@
> > * - 1.3.0 - adds GMEM_BASE + NR_RINGS params, SUBMITQUEUE_NEW +
> > * SUBMITQUEUE_CLOSE ioctls, and MSM_INFO_IOVA flag for
> > * MSM_GEM_INFO ioctl.
> > + * - 1.4.0 - softpin, MSM_RELOC_BO_DUMP, and GEM_INFO support to set/get
> > + * GEM object's debug name
> > */
> > #define MSM_VERSION_MAJOR 1
> > -#define MSM_VERSION_MINOR 3
> > +#define MSM_VERSION_MINOR 4
> > #define MSM_VERSION_PATCHLEVEL 0
> >
>
> I don't know the background here, but generally speaking we don't have
> version numbers for ioctls in kernel drivers. Instead, the old ioctls
> need to remain functional, but you can add new ioctl commands
> in addition.
>
> Is there something that makes this driver special?
>
The version # indicates to userspace that some new features are
supported, so that new userspace on kernel can work. For example, the
userspace side of setting a GEM obj debug name is:
static void msm_bo_set_name(struct fd_bo *bo, const char *fmt, va_list ap)
{
struct drm_msm_gem_info req = {
.handle = bo->handle,
.info = MSM_INFO_SET_NAME,
};
char buf[32];
int sz;
/* bail if kernel doesn't support this: */
if (bo->dev->version < FD_VERSION_SOFTPIN)
return;
sz = vsnprintf(buf, sizeof(buf), fmt, ap);
req.value = VOID2U64(buf);
req.len = MIN2(sz, sizeof(buf));
drmCommandWrite(bo->dev->fd, DRM_MSM_GEM_INFO, , sizeof(req));
}
(The old userspace on new kernel case is handled by zero padding in drm_ioctl())
BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/msm: rework GEM_INFO ioctl
On Fri, Nov 30, 2018 at 10:14 AM Arnd Bergmann wrote:
>
> On Fri, Nov 30, 2018 at 4:02 PM Rob Clark wrote:
> >
>
> > -
> > -#define MSM_INFO_FLAGS (MSM_INFO_IOVA)
> > +/* Get or set GEM buffer info. The requested value can be passed
> > + * directly in 'value', or for data larger than 64b 'value' is a
> > + * pointer to userspace buffer, with 'len' specifying the number of
> > + * bytes copied into that buffer. For info returned by pointer,
> > + * calling the GEM_INFO ioctl with null 'value' will return the
> > + * required buffer size in 'len'
> > + */
> > +#define MSM_INFO_GET_OFFSET0x00 /* get mmap() offset, returned by
> > value */
> > +#define MSM_INFO_GET_IOVA 0x01 /* get iova, returned by value */
> >
> > struct drm_msm_gem_info {
> > __u32 handle; /* in */
> > - __u32 flags; /* in - combination of MSM_INFO_* flags */
> > - __u64 offset; /* out, mmap() offset or iova */
> > + __u32 info; /* in - one of MSM_INFO_* */
> > + __u64 value; /* in or out */
> > + __u32 len;/* in or out */
> > };
>
> As structure with implicit padding has the problem of possibly leaking
> kernel stack data. It's better to make the padding explicit here so you
> can zero it from the kernel. Also, as I mentioned in the other patch,
> you probably need a new data structure and ioctl command number
> to keep compatiblity with the old interface.
hmm, right, pad field is a good idea. As far as compat, drm_ioctl()
handles zero-padding so adding new ioctl struct members at the end is
safe (as long as a zero value somehow results in previous behavior)
BR,
-R
>
> Arnd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108912] Null PTR deref on amd-staging-drm-next since rebase for rc3
https://bugs.freedesktop.org/show_bug.cgi?id=108912 --- Comment #2 from Samantha McVey --- Nicholas, That patch seems to resolve the issue. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107690] Blackscreen after installing AMD driver 18.30 for Vega 64 on Ubuntu 18.04.1 (HDMI EDID 2.0 issue)
https://bugs.freedesktop.org/show_bug.cgi?id=107690 --- Comment #17 from Gleb Zasyadko --- Created attachment 142669 --> https://bugs.freedesktop.org/attachment.cgi?id=142669=edit dmesg_log_kernel_4.20-rc4_with_HDMI_EDID2.0_OK Good news! On version 4.20-rc4, my display is finally working in 4k @ 60Hz mode! I hope in the next versions of the kernel the driver will also work properly. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v2, 1/8] drm: Add optional COLOR_ENCODING and COLOR_RANGE properties to drm_plane
On Fri, Nov 30, 2018 at 03:48:00PM +0100, Hans Verkuil wrote: > On 11/30/18 15:29, Ville Syrjälä wrote: > > On Fri, Nov 30, 2018 at 03:20:59PM +0100, Andrzej Hajda wrote: > >> Hi Ville, > >> > >> As Christoph cannot respond till middle next week I can try to respond > >> in his absence, as I am familiar with the subject. > >> > >> On 30.11.2018 14:25, Ville Syrjälä wrote: > >>> On Fri, Nov 30, 2018 at 02:08:11PM +0100, Christoph Manszewski wrote: > Hi, > > I am looking for a way to export the color encoding and range selection > to user space. I came across those properties and am wondering, why > they are meant only for non RGB color encodings. Would it be okay, to > modify them and use with RGB formats as well? > >>> What you trying to do? Input limited range RGB data and expand to full > >>> range? > >> > >> > >> For example. But there are two more general questions, which > >> surprisingly we have not found answer for. > >> > >> 1. What color encoding and range drm should expect on its input RGB > >> buffers by default? > > > > RGB is just RGB. There is no encoding. It's assumed to be full range > > because no one really uses anything else. > > For simple desktop usage that's true. When dealing with video inputs, > this becomes much more complicated. > > > > >> > >> 2. How userspace should inform drm that given buffer has specified > >> non-default color encoding and range? > >> > >> > >> Hopefully this patch introduces such properties but only for YCbCr > >> formats, the question is what should be the best way to expand it to RGB > >> formats: > >> > >> A. Add another enums: DRM_COLOR_RGB_BT601 and friends. > > > > BT.601 specifies how to encoder RGB data as YCbCr. So without > > YCbCr BT.601 does not mean anything. Well, the standard does > > contain other things as well I suppose, but for the purposes > > of the color encoding prop only that one part is relevant. > > Ah, I misunderstood the meaning of DRM_COLOR_RGB_BT601. > This is the equivalent of V4L2_YCBCR_ENC_601, and that's indeed > only defined for Y'CbCr. But it is often (ab)used as an alias for > the SMPTE170M colorspace (used by SDTV). > > V4L2 has the following defines for colorspaces, transfer functions, > Y'CbCr (and HSV) encodings and quantization ranges: > > https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces-defs.html Yeah, we're going to be introducing other properties to control colorspace and transfer function in kms as well. Actually some patches towards that have been floated a few times already. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/msm: rework GEM_INFO ioctl
On Fri, Nov 30, 2018 at 4:02 PM Rob Clark wrote:
>
> -
> -#define MSM_INFO_FLAGS (MSM_INFO_IOVA)
> +/* Get or set GEM buffer info. The requested value can be passed
> + * directly in 'value', or for data larger than 64b 'value' is a
> + * pointer to userspace buffer, with 'len' specifying the number of
> + * bytes copied into that buffer. For info returned by pointer,
> + * calling the GEM_INFO ioctl with null 'value' will return the
> + * required buffer size in 'len'
> + */
> +#define MSM_INFO_GET_OFFSET0x00 /* get mmap() offset, returned by
> value */
> +#define MSM_INFO_GET_IOVA 0x01 /* get iova, returned by value */
>
> struct drm_msm_gem_info {
> __u32 handle; /* in */
> - __u32 flags; /* in - combination of MSM_INFO_* flags */
> - __u64 offset; /* out, mmap() offset or iova */
> + __u32 info; /* in - one of MSM_INFO_* */
> + __u64 value; /* in or out */
> + __u32 len;/* in or out */
> };
As structure with implicit padding has the problem of possibly leaking
kernel stack data. It's better to make the padding explicit here so you
can zero it from the kernel. Also, as I mentioned in the other patch,
you probably need a new data structure and ioctl command number
to keep compatiblity with the old interface.
Arnd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/msm: bump UAPI version
On Fri, Nov 30, 2018 at 4:02 PM Rob Clark wrote: > > Signed-off-by: Rob Clark > --- > drivers/gpu/drm/msm/msm_drv.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index 6ebbd5010722..782cc33916d6 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -36,9 +36,11 @@ > * - 1.3.0 - adds GMEM_BASE + NR_RINGS params, SUBMITQUEUE_NEW + > * SUBMITQUEUE_CLOSE ioctls, and MSM_INFO_IOVA flag for > * MSM_GEM_INFO ioctl. > + * - 1.4.0 - softpin, MSM_RELOC_BO_DUMP, and GEM_INFO support to set/get > + * GEM object's debug name > */ > #define MSM_VERSION_MAJOR 1 > -#define MSM_VERSION_MINOR 3 > +#define MSM_VERSION_MINOR 4 > #define MSM_VERSION_PATCHLEVEL 0 > I don't know the background here, but generally speaking we don't have version numbers for ioctls in kernel drivers. Instead, the old ioctls need to remain functional, but you can add new ioctl commands in addition. Is there something that makes this driver special? Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108912] Null PTR deref on amd-staging-drm-next since rebase for rc3
https://bugs.freedesktop.org/show_bug.cgi?id=108912 --- Comment #1 from Nicholas Kazlauskas --- Created attachment 142666 --> https://bugs.freedesktop.org/attachment.cgi?id=142666=edit 0001-drm-amd-display-Fix-NULL-ptr-deref-for-commit_planes.patch Does this resolve the issue? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
