Re: [PATCH 1/4] drm: qxl: Drop misleading comment

[Daniel Vetter]

On Mon, Jan 30, 2017 at 06:14:49PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > drm-misc runs with the committer model, i.e. a few maintainers to do pull
> > requests and backmerges, a big pile of people directly pushing patches.
> 
> [ looked at docs too meanwhile ]
> 
> Sounds good.  I guess switching over simplifies things for all of us.
> We'll avoid issues like the one at hand.  Patch flow would be faster
> too.  Right now I'm only doing 1-2 drm-qemu pull requests per kernel
> release due to low patch volume even for all four qemu drivers combined.

Yeah, with dim I do send out pull requests every 1-2 weeks, depending upon
how much pending stuff there is.

> > Someone wreaked the entire 01.org domain, but you can get at all the
> > tooling and documentation with
> > 
> > git://anongit.freedesktop.org/drm-intel maintainer-tools
> 
> Hmm.  On a quick glance most of dim (except apply-patch) seems to be
> more useful for maintainers which do merges etc, not so much for
> committers.
> 
> I'm used to use https://github.com/stefanha/patches for qemu, and
> started using it for drm-qemu too.  It makes applying patches easier.
> It manages a patch database, using notmuch mail storage, and can apply
> patches and patch series from the patch database.  That way I don't have
> to save the patches as mbox somewhere.  The tool also picks up
> [Reviewed,Tested,Acked}-by lines from replies, and it stores the message
> id (but unlike dim it doesn't build a patchwork link out of it).
> 
> See bfac9f4fb4d87881375ccdc5c85d5ad59f2f115d for example.

That sha1 isn't in linux-next, so no idea. But in principle, yes dim isn't
all that much about handling patches, and much more about handling
branches. One part that imo really should stick around is the drm-tip
integration tree rebuilding. That allows us to distribute conflict
handling (e.g. between drm-misc-fixes and drm-misc-next), and with more
people and more drivers in drm-misc I expect more conflicts.

> Would that format be acceptable for drm-misc?

The other bit is that dim is a nice place for catching obvious screw-ups.
E.g. for drm-intel it checks that the patch only touches i915, and if not
if there's an ack from Dave on it. So I think standardizing on that tool
has benefits (as long as we can't just enforce such simple checks with CI
on the server side).

But dim apply-branch is a supposed to be a drop-in replacement for git
apply-mbox, so should be simple to integrate into another set of scripts
to manage the mails. And if there's changes to dim needed to make that
happen, we can do that (we push patches to it at a regular pace).

> > And then run make in there. We're not yet clear how exactly drivers within
> > drm-misc would look like (wrt which drivers and how much review and stuff
> > like that), hence the RFC.
> 
> Ok.  How quickly could I start using drm-misc?  I have some pending
> patches for the 4.11 merge window.  Any chance I can push them through
> drm-misc-next?  Or should I better send a pull req to Dave?

If you want you can get started right away. I plan to type some small docs
for this experiment, but the only thing you need is an fdo account with
drm-misc commit rights. Simplest to ping me (and fdo admins) on
#dri-devel.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Daniel Vetter]

On Mon, Jan 30, 2017 at 11:15:34AM +0100, Boris Brezillon wrote:
> Hi Daniel,
> 
> On Thu, 26 Jan 2017 18:08:42 +0100
> Daniel Vetter  wrote:
> 
> > Hi all,
> > 
> > We've discussed this a bit at LCA (with Dave and Eric), and it's
> > probably best if I just summarize all the questions and opens and
> > throw them out here for discussions:
> > 
> > - When's a driver small enough for a shared tree, and when is a
> > separate tree a good idea? i915 and amdgpu are definitely big, and
> > there's definitely drivers who are really small and in-between it's
> > unclear. Personally I think this is easy to do with a sliding scale,
> > with using topic branches (we can do them in drm-misc easily) for
> > bigger stuff, and if that's a common thing, split out the driver
> > (thanks to the drm-tip integration tree there's not much of a
> > difference in handling conflicts due to that anyway).
> > 
> > - Should it be an entire separate tree for soc drivers? Problem here
> > is that we lack a volunteer group (and imo it really should be a group
> > to avoid the single-maintainer troubles) to run that. I think it's
> > easier to proof the process first, and if we want a separate tree,
> > split that out later on. This is the same thing we've done with
> > drm-misc, first with a topic branch in drm-intel.git, then separate. I
> > think it worked really well.
> > 
> > - Should we require review or at least acks for patches committed by
> > the author? We have a bunch of drivers with effectively just 1 person
> > working on it, where getting real review is hard. But otoh a few of
> > those 1-person drivers will become popular, and then it's good to
> > start with establishing peer-review early on. I also think that
> > requiring peer-review is good to share best practices and knowledge
> > between different people in our community, not just to make sure the
> > code is correct. For all these reasons I'm leaning towards not making
> > an exception for drivers, and requiring the same amount of review for
> > them if they go in through drm-misc as for any other patch.
> > 
> > - Who's elligible? I think we could start small with a few volunteers
> > and their drivers, and then anyone who's willing.
> 
> I'd be happy to have the atmel-hlcdc driver maintained in this drm-misc
> tree. I just had to send a PR containing a single patch for 4.11, and it
> really feels like these simple fixes/improvements patches do not deserve
> a dedicated PR (not to mention that sometime I forget to send the
> PR and miss a release :-)).
> 
> Now, regarding the peer-review thing, I'm not against reviewing a few
> simple patches from time to time, but I don't think I'll have time to
> review entire new drivers, and I guess that's the kind of thing your
> looking for :-/.

It' should be equal for equal really imo, so if you have a few small
patches, then you'd need to review a few small patches to not drain the
review pool. I haven't figured out a good way to offload the
new-driver-review yet :-/

Anyway it sounds like we have enough interested folks for an attempt, I'll
try and type up some rough guidelines for the drm-misc docs and then we'll
see what happens.
-Daniel

> 
> > 
> > - Should we force new submissions to be managed in that shared treee?
> > I think for initial submission a separate pull request for
> > approval-by-Dave is good (but we could do that with topic branches
> > too). And it's also way too early to tell, probably better to first
> > figure out how well this goes.
> > 
> > - CI, needed? It would be great, but we're not there yet :( Atm
> > drm-misc just has a bunch of defconfigs that need to always compile,
> > and that's it. Long term I definitely want more, but we're just not
> > there yet. And it's a problem in general for drm-misc.
> > 
> > - dim scripts. Since we don't have a github flow where we can
> > reasonably automate stuff on the server side we need something to
> > automate on the client side. Thus far almost everyone seemed ok with
> > the scripting that's used to drive drm-misc/intel/tip, but we can
> > always improve things. And long term we can rework the approach
> > however we want to really.
> > 
> > - Other stuff I've missed?
> > 
> > Cheers, Daniel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 0/3] drm/i915: Handle hanging during nonblocking modeset correctly.

[Daniel Vetter]

On Mon, Jan 30, 2017 at 03:42:17PM +0100, Maarten Lankhorst wrote:
> Op 30-01-17 om 09:17 schreef Daniel Vetter:
> > On Fri, Jan 27, 2017 at 03:08:45PM +, Chris Wilson wrote:
> >> On Fri, Jan 27, 2017 at 03:58:08PM +0100, Daniel Vetter wrote:
> >>> On Fri, Jan 27, 2017 at 02:31:55PM +, Chris Wilson wrote:
>  On Fri, Jan 27, 2017 at 03:21:29PM +0100, Daniel Vetter wrote:
> > On Fri, Jan 27, 2017 at 09:30:50AM +, Chris Wilson wrote:
> >> On Thu, Jan 26, 2017 at 04:59:21PM +0100, Maarten Lankhorst wrote:
> >>> When writing some testcases for nonblocking modesets. I found out 
> >>> that the
> >>> infinite wait on the old fb was causing issues.
> >> The crux of the issue here is the locked wait for old dependencies and
> >> the inability to inject the intel_prepare_reset disabling of all 
> >> planes.
> >> There are a couple of locked waits on struct_mutex within the modeset
> >> locks for intel_overlay and if we happen to be using the display plane
> >> for the first time.
> >>
> >> The first I suggested solving using fences to track dependencies and
> >> keep the order between atomic states. Cancelling the outstanding
> >> modesets, replacing with a disable and then on restore jumping to the
> >> final state look doable. It also requires avoiding the struct_mutex for
> >> disabling, which is quite easy. To avoid the wait under struct_mutex,
> >> we've talked about switching to mmio, but for starters we could move 
> >> the
> >> wait from inside intel_overlay into the fence for the atomic operation.
> >> (But's that a little more surgery than we would like for intel_overlay 
> >> I
> >> guess - dig out Ville's patches for overlay planes?) And to prevent the
> >> wait under struct_mutex for pin_to_display_plane, my plane is to move
> >> that to an async fenced operation that is then naturally waited upon by
> >> the atomic modeset.
> > A bit more a hack, but a different idea, and I think hack for gen234.0 
> > is
> > ok:
> >
> > We complete all the requests before we start the hw reset with 
> > fence.error
> > = -EIO. But we do this only when we need to get at the display locks. A
> > slightly more elegant solution would be to trylock modeset locks, and if
> > one of them fails (and only then) complete all requests with -EIO to get
> > the concurrent modeset to proceed before we reset the hardware. That's
> > essentially the logic we had before all the reworks, and it worked. But 
> > I
> > didn't look at how scary that all would be to make it work again ...
>  The modeset lock may not just be waiting on our requests (even on pnv we
>  can expect that there are already users celebrating that pnv+nouveau
>  finally works ;) and that the display is not the only user/observer of
>  those requests. Using the requests to break the modeset lock just feels
>  like the wrong approach.
> >>> It's a cycle, and we need to break it somewhere. Another option might be
> >>> to break the cycle the same way we do it for gem locks: Wake up everyone
> >>> and restart the modeset ioctl. Since the trouble only happens for
> >>> synchronous modesets where we hold the locks while waiting for fences, we
> >>> can also break out of that and restart. And I also don't think that would
> >>> leak to other drivers, after all our gem locking restart dances also don't
> >>> leak to other drivers - it's just our own driver's lock which are affected
> >>> by these special wakupe semantics.
> >> It's a queue of nonblocking modesets that we need to worry about, afaik.
> >> Moving the wait for blocking modeset outside of modeset lock is easily
> >> achievable (and avoiding the other waits under both the modeset + 
> >> struct_mutex I have at least an idea for). So the challenge is how to
> >> inject all-planes-off for gen3 and then allow the queue to continue again
> >> afterwards.
> > Hm right, I missed the nonblocking updates which don't take locks. But
> > assuming we do the display reset for gpu resets as a full modeset (i.e.
> > going through ->atomic_commit) it should still work out correctly:
> >
> > Starting state: gpu is hung, nonblocking modeset waiting for some requests
> > to complete.
> Missing one evil detail here, else things would have moved forward..
> 
> A unrelated thread performs a blocking commit, and holds all locks until the 
> nonblocking modeset completes.

And where is the problem in that? If we first set all fences to -EIO, and
then try to grab locks, that other thread will be able to complete. After
all this scheme worked before we reworked the reset logic completely.
-Daniel

> > 1. hangcheck kicks in, fires off reset work.
> >
> > 2. We complete all requests with fence.error = -EIO and wake up any
> > waiters. That means no re-queueing for older platforms, but oh well.
> >
> > 3. We grab all the display locks. Nothing 

Re: [PATCH] drm: Don't race connector registration

[Daniel Vetter]

On Mon, Jan 30, 2017 at 08:43:17AM -0800, Dave Hansen wrote:
> On 01/30/2017 01:12 AM, Daniel Vetter wrote:
> > On Thu, Jan 26, 2017 at 12:34:29PM -0800, Dave Hansen wrote:
> ...
> >> And, yeah, I think it just gets the connected status wrong.  The
> >> connector is still there.
> > 
> > Hm, I thought I replied here but I didn't:
> > - Is this just after boot (and then the connector is stuck forever), or
> >   starts to happen after suspend/resume, or some other system change like
> >   that? Or do they just crop up eventually?
> 
> The most consistent thing I do to screw it up is switch systems on my
> DVI KVM switch.  When I switch back to the system in question, it
> doesn't seem to notice the condition.  The connectors eventually show up
> with random combinations of switching to the console (ctrl-alt-f1) and
> back, running xrandr, or running gnome-control-panel and opening the
> Displays applet.

Hm, so is this a dp mst kvm switch (i.e. do the connectors get
hot-added/removed when you plug/unplug that thing)? Or just some other
non-mst switch? I was working under the assumption that this is mst still,
but I've never seen an mst kvm switch.

> I haven't been able to discern any pattern to it.  Sometimes just
> running xrandr fixes it.  Sometimes just opening the control panel.
> Others, I have to do it several times.
> 
> I don't think it shows up if I just leave it for a while.
> 
> > - Does this only happen once the connector is destroyed? Please trace
> >   intel_dp_destroy_mst_connector with something like:
> 
> I'll see if I can gather that.

If it's not mst, then don't bother with this for obvious reasons :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/10] drm/etnaviv: add uapi for register read feature

[Daniel Vetter]

On Mon, Jan 30, 2017 at 11:45:59PM +, Russell King - ARM Linux wrote:
> On Mon, Jan 30, 2017 at 09:18:25PM +0100, Thierry Reding wrote:
> > On Fri, Dec 09, 2016 at 12:21:22PM +0100, Christian Gmeiner wrote:
> > > @@ -167,6 +174,9 @@ struct drm_etnaviv_gem_submit {
> > >   __u64 bos;/* in, ptr to array of submit_bo's */
> > >   __u64 relocs; /* in, ptr to array of submit_reloc's */
> > >   __u64 stream; /* in, ptr to cmdstream */
> > > + __u64 readbacks;  /* in, ptr to array of submit_readback's */
> > > + __u32 nr_readbacks;   /* in, number of submit_readback's */
> > > + __u32 padding;
> > >  };
> > 
> > What about ABI stability? How's this going to work when userspace uses
> > old headers but runs against a new kernel? What about userspace using
> > newer headers than the kernel?
> 
> +1, this is unacceptable.  We went through a period of making sure
> that the ABI was going to be stable once we merged the driver into
> the kernel, and the rule is that we don't ever break userspace.  This
> does exactly that, so it's not permissible.
> 
> If this change is necessary, it needs to be a new ioctl number for the
> call with the new structure, and the old ioctl has to keep working.
> 
> There are other users of etnaviv other than mesa that will break.

As long as you don't do it wrong (i.e. struct not used in arrays, and some
flags to indicate when the new fields should be looked at) then it's
perfectly safe to extend drm ioctl structs at the end. The core drm ioctl
functions zero-pad length mismatches in both directions if needed.

The only additional recommendation is to still do a new #define (the ioctl
number has changed anyway since it encodes the size), to make sure that
old userspace still uses the old structs even with upgraded headers.
Otherwise they might not properly clear the new fields, which is the one
case that breaks backwards/forwards compat (been there, done that, not
fun).

We should probably have a chapter in the drm-uapi.rst docs about this,
with a link to the botching-up-ioctls.txt file with the more general
recommendatations.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=99275

--- Comment #15 from Michel Dänzer  ---
There are two basic explanations for not getting consistent bisection results:

* In each case, at least one bad commit was accidentally marked as good. Test
longer / more times before declaring a commit as good to avoid this.

* The problem (or at least the trigger) isn't in the kernel but somewhere else.

-- 
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 99444] [radeonsi] The Witcher 3 (GOG/1.31) [Wine] starting menu is distorted

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=99444

--- Comment #11 from Shmerl  ---
I just replayed my trace (recorder on radeonsi / RX480), and I see this in the
log:

120612: message: major api error 2: GL_INVALID_ENUM in
glActiveTexture(texture=GL_COMBINER0_NV)
120612 @1 glActiveTexture(texture = GL_COMBINER0_NV)
120619: message: major api error 2: GL_INVALID_ENUM in
glActiveTexture(texture=GL_COMBINER1_NV)
120619 @1 glActiveTexture(texture = GL_COMBINER1_NV)
120626: message: major api error 2: GL_INVALID_ENUM in
glActiveTexture(texture=GL_COMBINER2_NV)
120626 @1 glActiveTexture(texture = GL_COMBINER2_NV)
120633: message: major api error 2: GL_INVALID_ENUM in
glActiveTexture(texture=GL_COMBINER3_NV)
120633 @1 glActiveTexture(texture = GL_COMBINER3_NV)
120640: message: major api error 2: GL_INVALID_ENUM in
glActiveTexture(texture=GL_COMBINER4_NV)
120640 @1 glActiveTexture(texture = GL_COMBINER4_NV)
120647: message: major api error 2: GL_INVALID_ENUM in
glActiveTexture(texture=GL_COMBINER5_NV)
120647 @1 glActiveTexture(texture = GL_COMBINER5_NV)
120654: message: major api error 2: GL_INVALID_ENUM in
glActiveTexture(texture=GL_COMBINER6_NV)
120654 @1 glActiveTexture(texture = GL_COMBINER6_NV)
120661: message: major api error 2: GL_INVALID_ENUM in
glActiveTexture(texture=GL_COMBINER7_NV)
120661 @1 glActiveTexture(texture = GL_COMBINER7_NV)

I can open a bug in Wine about 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

[PATCH] drm: remove unnecessary fault wrappers

[Ross Zwisler]

The fault wrappers drm_vm_fault(), drm_vm_shm_fault(), drm_vm_dma_fault()
and drm_vm_sg_fault() used to provide extra logic beyond what was in the
"drm_do_*" versions of these functions, but as of this commit:

commit ca0b07d9a969 ("drm: convert drm from nopage to fault.")

They are just unnecessary wrappers that do nothing.  Remove them.

Signed-off-by: Ross Zwisler 
---
 drivers/gpu/drm/drm_vm.c | 28 
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index bae6e26..e677b11 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -349,50 +349,30 @@ static int drm_do_vm_sg_fault(struct vm_fault *vmf)
return 0;
 }
 
-static int drm_vm_fault(struct vm_fault *vmf)
-{
-   return drm_do_vm_fault(vmf);
-}
-
-static int drm_vm_shm_fault(struct vm_fault *vmf)
-{
-   return drm_do_vm_shm_fault(vmf);
-}
-
-static int drm_vm_dma_fault(struct vm_fault *vmf)
-{
-   return drm_do_vm_dma_fault(vmf);
-}
-
-static int drm_vm_sg_fault(struct vm_fault *vmf)
-{
-   return drm_do_vm_sg_fault(vmf);
-}
-
 /** AGP virtual memory operations */
 static const struct vm_operations_struct drm_vm_ops = {
-   .fault = drm_vm_fault,
+   .fault = drm_do_vm_fault,
.open = drm_vm_open,
.close = drm_vm_close,
 };
 
 /** Shared virtual memory operations */
 static const struct vm_operations_struct drm_vm_shm_ops = {
-   .fault = drm_vm_shm_fault,
+   .fault = drm_do_vm_shm_fault,
.open = drm_vm_open,
.close = drm_vm_shm_close,
 };
 
 /** DMA virtual memory operations */
 static const struct vm_operations_struct drm_vm_dma_ops = {
-   .fault = drm_vm_dma_fault,
+   .fault = drm_do_vm_dma_fault,
.open = drm_vm_open,
.close = drm_vm_close,
 };
 
 /** Scatter-gather virtual memory operations */
 static const struct vm_operations_struct drm_vm_sg_ops = {
-   .fault = drm_vm_sg_fault,
+   .fault = drm_do_vm_sg_fault,
.open = drm_vm_open,
.close = drm_vm_close,
 };
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] drm: remove unnecessary fault wrappers

[Ross Zwisler]

The fault wrappers drm_vm_fault(), drm_vm_shm_fault(), drm_vm_dma_fault()
and drm_vm_sg_fault() used to provide extra logic beyond what was in the
"drm_do_*" versions of these functions, but as of this commit:

commit ca0b07d9a969 ("drm: convert drm from nopage to fault.")

They are just unnecessary wrappers that do nothing.  Remove them, and
rename the the drm_do_* fault handlers to remove the "do_" since they no
longer have corresponding wrappers.

Signed-off-by: Ross Zwisler 
---

This patch applies cleanly to mmots/master, which is currently at
v4.10-rc5-mmots-2017-01-26-15-49.

---
 drivers/gpu/drm/drm_vm.c | 30 +-
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 0cd993a..039a1e0 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -95,7 +95,7 @@ static pgprot_t drm_dma_prot(uint32_t map_type, struct 
vm_area_struct *vma)
  * map, get the page, increment the use count and return it.
  */
 #if IS_ENABLED(CONFIG_AGP)
-static int drm_do_vm_fault(struct vm_fault *vmf)
+static int drm_vm_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
struct drm_file *priv = vma->vm_file->private_data;
@@ -168,7 +168,7 @@ static int drm_do_vm_fault(struct vm_fault *vmf)
return VM_FAULT_SIGBUS; /* Disallow mremap */
 }
 #else
-static int drm_do_vm_fault(struct vm_fault *vmf)
+static int drm_vm_fault(struct vm_fault *vmf)
 {
return VM_FAULT_SIGBUS;
 }
@@ -183,7 +183,7 @@ static int drm_do_vm_fault(struct vm_fault *vmf)
  * Get the mapping, find the real physical page to map, get the page, and
  * return it.
  */
-static int drm_do_vm_shm_fault(struct vm_fault *vmf)
+static int drm_vm_shm_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
struct drm_local_map *map = vma->vm_private_data;
@@ -285,7 +285,7 @@ static void drm_vm_shm_close(struct vm_area_struct *vma)
  *
  * Determine the page number from the page offset and get it from 
drm_device_dma::pagelist.
  */
-static int drm_do_vm_dma_fault(struct vm_fault *vmf)
+static int drm_vm_dma_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
struct drm_file *priv = vma->vm_file->private_data;
@@ -320,7 +320,7 @@ static int drm_do_vm_dma_fault(struct vm_fault *vmf)
  *
  * Determine the map offset from the page offset and get it from 
drm_sg_mem::pagelist.
  */
-static int drm_do_vm_sg_fault(struct vm_fault *vmf)
+static int drm_vm_sg_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
struct drm_local_map *map = vma->vm_private_data;
@@ -347,26 +347,6 @@ static int drm_do_vm_sg_fault(struct vm_fault *vmf)
return 0;
 }
 
-static int drm_vm_fault(struct vm_fault *vmf)
-{
-   return drm_do_vm_fault(vmf);
-}
-
-static int drm_vm_shm_fault(struct vm_fault *vmf)
-{
-   return drm_do_vm_shm_fault(vmf);
-}
-
-static int drm_vm_dma_fault(struct vm_fault *vmf)
-{
-   return drm_do_vm_dma_fault(vmf);
-}
-
-static int drm_vm_sg_fault(struct vm_fault *vmf)
-{
-   return drm_do_vm_sg_fault(vmf);
-}
-
 /** AGP virtual memory operations */
 static const struct vm_operations_struct drm_vm_ops = {
.fault = drm_vm_fault,
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/10] drm/etnaviv: add uapi for register read feature

[Russell King - ARM Linux]

On Mon, Jan 30, 2017 at 09:18:25PM +0100, Thierry Reding wrote:
> On Fri, Dec 09, 2016 at 12:21:22PM +0100, Christian Gmeiner wrote:
> > @@ -167,6 +174,9 @@ struct drm_etnaviv_gem_submit {
> > __u64 bos;/* in, ptr to array of submit_bo's */
> > __u64 relocs; /* in, ptr to array of submit_reloc's */
> > __u64 stream; /* in, ptr to cmdstream */
> > +   __u64 readbacks;  /* in, ptr to array of submit_readback's */
> > +   __u32 nr_readbacks;   /* in, number of submit_readback's */
> > +   __u32 padding;
> >  };
> 
> What about ABI stability? How's this going to work when userspace uses
> old headers but runs against a new kernel? What about userspace using
> newer headers than the kernel?

+1, this is unacceptable.  We went through a period of making sure
that the ABI was going to be stable once we merged the driver into
the kernel, and the rule is that we don't ever break userspace.  This
does exactly that, so it's not permissible.

If this change is necessary, it needs to be a new ioctl number for the
call with the new structure, and the old ioctl has to keep working.

There are other users of etnaviv other than mesa that will break.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] Gpu: drm: exynos - Fix possible NULL derefrence.

[Shailendra Verma]

of_device_get_match_data could return NULL, and so can cause
a NULL pointer dereference later.

Signed-off-by: Shailendra Verma 
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c  |4 
 drivers/gpu/drm/exynos/exynos_drm_fimd.c |4 
 drivers/gpu/drm/exynos/exynos_hdmi.c |4 
 drivers/gpu/drm/exynos/exynos_mixer.c|4 
 4 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index e07cb1f..fba0ffc 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1765,6 +1765,10 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 
dsi->dev = dev;
dsi->driver_data = of_device_get_match_data(dev);
+   if (!dsi->driver_data) {
+   dev_err(dev, "no device match found\n");
+   return -ENODEV;
+   }
 
ret = exynos_dsi_parse_dt(dsi);
if (ret)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c 
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index e2e4051..f568234 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -1076,6 +1076,10 @@ static int fimd_probe(struct platform_device *pdev)
ctx->dev = dev;
ctx->suspended = true;
ctx->driver_data = of_device_get_match_data(dev);
+   if (!ctx->driver_data) {
+   dev_err(dev, "no device match found\n");
+   return -ENODEV;
+   }
 
if (of_property_read_bool(dev->of_node, "samsung,invert-vden"))
ctx->vidcon1 |= VIDCON1_INV_VDEN;
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 38eaa63..27aeb74 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1837,6 +1837,10 @@ static int hdmi_probe(struct platform_device *pdev)
return -ENOMEM;
 
hdata->drv_data = of_device_get_match_data(dev);
+   if (!hdata->drv_data) {
+   dev_err(dev, "no device match found\n");
+   return -ENODEV;
+   }
 
platform_set_drvdata(pdev, hdata);
 
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
b/drivers/gpu/drm/exynos/exynos_mixer.c
index edb20a3..b3c6bbb 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1205,6 +1205,10 @@ static int mixer_probe(struct platform_device *pdev)
}
 
drv = of_device_get_match_data(dev);
+   if (!drv) {
+   dev_err(dev, "no device match found\n");
+   return -ENODEV;
+   }
 
ctx->pdev = pdev;
ctx->dev = dev;
-- 
1.7.9.5

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Tomi Sarvela]

On Monday, 30 January 2017 10:49:10 EET Daniel Vetter wrote:
> On Mon, Jan 30, 2017 at 10:30:43AM +0100, Gerd Hoffmann wrote:
> >   Hi,
> >   
> > > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.
> > > html
> > > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/dim.html> 
> > apache throws 403.
> 
> We're looking into it. Adding Tomi, who's herding the autobuilder
> for these docs.

https://01.org/ is 403. The problem is not limited to documentation 
only. The issue has been pushed forward, but I don't have better 
information yet.

Tomi
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm: remove unnecessary fault wrappers

[Ross Zwisler]

On Mon, Jan 30, 2017 at 03:09:39PM -0700, Ross Zwisler wrote:

> This patch applies cleanly to mmots/master, which is currently at
> v4.10-rc5-mmots-2017-01-26-15-49.

Which may not be what you want...  The reason I was looking at this code was
because it was recently changed by Dave Jiang to remove the 'vma' argument
from all the fault handlers.

So, would you rather:

1) Take it through the -mm tree to avoid merge conflicts, in which case I'll
add akpm to the thread.

2) Take it through your tree & deal with merge conflicts later, in which case
I can rebase on your tree or on v4.10-rc6.

3) Just drop it and keep the extra 20 lines or whatever that the complier will
just optimize out.

I'm fine with any of the above.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/14] drm/omap: use atomic_dec_not_zero()

[Fabian Frederick]

instead of atomic_add_unless(value, -1, 0)

Signed-off-by: Fabian Frederick 
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 3cab066..abd0b15 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -184,7 +184,7 @@ static struct dmm_txn *dmm_txn_init(struct dmm *dmm, struct 
tcm *tcm)
 
/* wait until an engine is available */
ret = wait_event_interruptible(omap_dmm->engine_queue,
-   atomic_add_unless(_dmm->engine_counter, -1, 0));
+   atomic_dec_not_zero(_dmm->engine_counter));
if (ret)
return ERR_PTR(ret);
 
-- 
2.9.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/10] drm/etnaviv: validate readback register address

[Russell King - ARM Linux]

On Mon, Jan 30, 2017 at 11:58:32AM +0100, Lucas Stach wrote:
> Am Freitag, den 09.12.2016, 12:21 +0100 schrieb Christian Gmeiner:
> > Reading some registers end in a system crash ala:
> > 
> >   Unhandled fault: external abort on non-linefetch (0x1028) at 0xfe641000
> >   Internal error: : 1028 [#1] PREEMPT ARM
> > 
> > Avoid those by register validation.
> 
> Avoiding crashes is one thing, but I believe this needs to go further
> and avoid reads from any register that isn't a performance counter. This
> probably isn't a big hole, but we want to avoid leaking the GPU state to
> userspace.

We certainly don't want to let people use this to read stuff like the
ID registers, bypassing the kernel.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/10] drm/etnaviv: make it possible to reconfigure perf counter

[Russell King - ARM Linux]

On Mon, Jan 30, 2017 at 12:01:18PM +0100, Lucas Stach wrote:
> Am Freitag, den 09.12.2016, 12:21 +0100 schrieb Christian Gmeiner:
> > -   if (r->flags) {
> > -   DRM_ERROR("readback flags not 0");
> > +   if (r->flags > ETNA_READBACK_PERF) {
> > +   DRM_ERROR("invalid readback flags");

This test should be more robust - while ETNA_READBACK_PERF may have a
value of '1', this is not how we should be checking for invalid _flags_.
It may work, but it's not the best solution.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 00/14] use atomic_dec_not_zero()

[Fabian Frederick]

complementary definition to atomic_inc_not_zero() featured in
lib/fault-inject.c. This small patchset moves it to
include/linux/atomic.h using it instead of
atomic_add_unless(value, -1, 0)

s390 patches were not compile-tested.

Fabian Frederick (14):
  locking/atomic: import atomic_dec_not_zero()
  drm/exynos: use atomic_dec_not_zero()
  drm/omap: use atomic_dec_not_zero()
  m5mols: use atomic_dec_not_zero()
  omap3isp: use atomic_dec_not_zero()
  s390/qeth: use atomic_dec_not_zero()
  PM / RUNTIME: use atomic_dec_not_zero()
  ipmi: use atomic_dec_not_zero()
  kdb: use atomic_dec_not_zero()
  PM / Hibernate: use atomic_dec_not_zero()
  PM: use atomic_dec_not_zero()
  s390/topology: use atomic_dec_not_zero()
  ext4: use atomic_dec_not_zero()
  xfs: use atomic_dec_not_zero()

 arch/s390/kernel/topology.c   | 2 +-
 drivers/base/power/runtime.c  | 4 ++--
 drivers/char/ipmi/ipmi_msghandler.c   | 2 +-
 drivers/gpu/drm/exynos/exynos_drm_fimd.c  | 2 +-
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c  | 2 +-
 drivers/media/i2c/m5mols/m5mols_core.c| 2 +-
 drivers/media/platform/omap3isp/ispstat.c | 2 +-
 drivers/s390/net/qeth_core_main.c | 2 +-
 fs/ext4/ext4.h| 2 +-
 fs/xfs/xfs_buf.c  | 2 +-
 include/linux/atomic.h| 2 ++
 kernel/debug/kdb/kdb_main.c   | 2 +-
 kernel/power/hibernate.c  | 4 ++--
 kernel/power/user.c   | 2 +-
 lib/fault-inject.c| 2 --
 15 files changed, 17 insertions(+), 17 deletions(-)

-- 
2.9.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Don't race connector registration

[Dave Hansen]

On 01/30/2017 01:12 AM, Daniel Vetter wrote:
> On Thu, Jan 26, 2017 at 12:34:29PM -0800, Dave Hansen wrote:
...
>> And, yeah, I think it just gets the connected status wrong.  The
>> connector is still there.
> 
> Hm, I thought I replied here but I didn't:
> - Is this just after boot (and then the connector is stuck forever), or
>   starts to happen after suspend/resume, or some other system change like
>   that? Or do they just crop up eventually?

The most consistent thing I do to screw it up is switch systems on my
DVI KVM switch.  When I switch back to the system in question, it
doesn't seem to notice the condition.  The connectors eventually show up
with random combinations of switching to the console (ctrl-alt-f1) and
back, running xrandr, or running gnome-control-panel and opening the
Displays applet.

I haven't been able to discern any pattern to it.  Sometimes just
running xrandr fixes it.  Sometimes just opening the control panel.
Others, I have to do it several times.

I don't think it shows up if I just leave it for a while.

> - Does this only happen once the connector is destroyed? Please trace
>   intel_dp_destroy_mst_connector with something like:

I'll see if I can gather that.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V7 3/4] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge

[Jani Nikula]

On Sat, 28 Jan 2017, Peter Senna Tschudin  wrote:
> On Thu, Jan 05, 2017 at 01:18:47PM +0530, Archit Taneja wrote:
> Hi Archit,
>
> Thank you for the comments!
>
> [...]
>> > +  total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
>> > +  if (total_size > EDID_LENGTH) {
>> > +  kfree(block);
>> > +  block = kmalloc(total_size, GFP_KERNEL);
>> > +  if (!block)
>> > +  return NULL;
>> > +
>> > +  /* Yes, read the entire buffer, and do not skip the first
>> > +   * EDID_LENGTH bytes.
>> > +   */
>> 
>> Is this the reason why you aren't using drm_do_get_edid()?
>
> Yes, for some hw specific reason, it is necessary to read the entire
> EDID buffer starting from 0, not block by block.

Hrmh, I'm planning on moving the edid override and firmware edid
mechanisms at the drm_do_get_edid() level to be able to truly and
transparently use a different edid. Currently, they're only used for
modes, really, and lead to some info retrieved from overrides, some from
the real edid. This kind of hacks will bypass the override/firmware edid
mechanisms then too. :(

BR,
Jani.


>
> [...]
>
> I fixed all your other suggestions. Thank you!
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH v2 7/9] drm: Connector helper function to release atomic state

[Pandiyan, Dhinakaran]

On Wed, 2017-01-25 at 07:18 +0100, Daniel Vetter wrote:
> On Tue, Jan 24, 2017 at 03:49:35PM -0800, Dhinakaran Pandiyan wrote:
> > Having a ->atomic_release callback is useful to release shared resources
> > that get allocated in compute_config().
> > 
> > Suggested-by: Daniel Vetter 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  include/drm/drm_modeset_helper_vtables.h | 15 +++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/include/drm/drm_modeset_helper_vtables.h 
> > b/include/drm/drm_modeset_helper_vtables.h
> > index 46f5b34..e41b18a 100644
> > --- a/include/drm/drm_modeset_helper_vtables.h
> > +++ b/include/drm/drm_modeset_helper_vtables.h
> > @@ -831,6 +831,21 @@ struct drm_connector_helper_funcs {
> >  */
> > struct drm_encoder *(*atomic_best_encoder)(struct drm_connector 
> > *connector,
> >struct drm_connector_state 
> > *connector_state);
> > +
> > +   /**
> > +* @atomic_release:
> > +*
> > +* This function is used to release shared resources that were
> > +* previously acquired. For example, resources acquired in
> > +* encoder->compute_config() can be released by calling this function
> 
> @compute_config is the right way to do references within the same struct.

compute_config is not in the same structure, which made me realize I
should not be referring to compute_config() at all, as it is a helper
for struct intel_encoder. 


-DK
> 
> > +* from mode_fixup()
> 
> Same here.
> 
> Patch split up is a bit strange, hence why my review of the design is in
> later patches.
> 
> Thanks, Daniel
> 
> > +*
> > +* NOTE:
> > +*
> > +* This function is called in the check phase of an atomic update.
> > +*/
> > +   void (*atomic_release)(struct drm_connector *connector,
> > +  struct drm_connector_state *connector_state);
> >  };
> >  
> >  /**
> > -- 
> > 2.7.4
> > 
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] Gpu: drm: tegra - Fix possible NULL derefrence.

[Shailendra Verma]

of_match_device could return NULL, and so can cause a NULL
pointer dereference later.

Signed-off-by: Shailendra Verma 
---
 drivers/gpu/drm/tegra/sor.c |4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 74d0540..34f032f 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -2540,6 +2540,10 @@ static int tegra_sor_probe(struct platform_device *pdev)
int err;
 
match = of_match_device(tegra_sor_of_match, >dev);
+   if (!match) {
+   dev_err(>dev, "Error: No device match found\n");
+   return -ENODEV;
+   }
 
sor = devm_kzalloc(>dev, sizeof(*sor), GFP_KERNEL);
if (!sor)
-- 
1.7.9.5

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: BUG: 4.10 i915 drm display noise regression - bisected to a6a7cc4b7

[lkml]

On Mon, Jan 09, 2017 at 12:32:40AM -0600, l...@pengaru.com wrote:
> Hello all,
> 
> I'm experiencing display noise in the form of 8x1 pixel bars spuriously
> appearing in random locations.  This doesn't happen on 4.9, the machine
> is an X61s, a Core2Duo 1.8Ghz w/XGA via LVDS.
> 
> I was able to bisect the issue to a6a7cc4b7:
> 
> commit a6a7cc4b7db6deaeca11cdd38844ea147a354c7a
> Author: Chris Wilson 
> Date:   Fri Nov 18 21:17:46 2016 +
> 
> drm/i915: Always flush the dirty CPU cache when pinning the scanout
> 
> Currently we only clflush the scanout if it is in the CPU domain. Also
> flush if we have a pending CPU clflush. We also want to treat the
> dirtyfb path similar, and flush any pending writes there as well.
> 
> v2: Only send the fb flush message if flushing the dirt on flip
> v3: Make flush-for-flip and dirtyfb look more alike since they serve
> similar roles as end-of-frame marker.
> 
> Reproduction is simple, just run this native drm eye candy program:
> https://github.com/vcaputo/rototiller
> 

This regression still remains as of 4.10.0-rc6.

Chris Wilson had posted a fix:
https://www.spinics.net/lists/kernel/msg2420777.html

But it seems to have been ignored so far.  How do we get this fixed in
4.10 before it ships?

Regards,
Vito Caputo
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[EVoC] Beginner question about X.org projects

[Vinay Neekhra]

Hello,
I am Vinay. I want to get involved in X.org projects.
As of now, to have a big picture view, I am going through the documentation.
If possible, could you please suggest me where should I start from?

About me: I am pursuing M.S. in Machine Translation from IIIT-Hyderabad, India. 
I am comfortable with C++, C, Git, IRC. My motivations are to learn how 
open-source orgs
work and to make some contributions in open source technologies.

On the FAQ page (https://www.x.org/wiki/FAQ/), I was not sure if this phrase 
was intentional or not
"hay guys why do u still hav network transparency dont you know its making 
everyfing slow".
I was wondering if it should be corrected?  

Thanks,
Vinay Neekhra 
B.Tech + M.S. by Research   
Language Technology and Research Center 
IIIT Hyderabad
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[GIT PULL] exynos-drm-next

[Inki Dae]

Hi Dave,

   adding runtime PM support to MIC driver, and including some
   cleanups - especially using atomic helper functions
   instead of specific ones - and fixups.

   In addition, it includes S6E3HA2 panel driver and its support.
   The panel driver should go to mainline by Thierry.
   However, there is no comment from him. Seems he is busy maybe.
   I think we had review enough so I picked it up.

   And we have one patch series for UHD support which is under the review.
   So I plan to have a pull request again as soon as finising the review.

   Please kindly let me know if there is any problem.

Thanks,
Inki Dae

The following changes since commit a5b2b6ebf34b20e70a2bdb5214c371744e7fa260:

  drm/sti: Fix compilation failure for drm_framebuffer.pixel_format (2017-01-27 
12:54:15 +1000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git 
exynos-drm-next

for you to fetch changes up to 2473244a04edf5480770bd840c38546b800f839d:

  drm/panel: Add support for S6E3HA2 panel driver on TM2 board (2017-01-31 
09:02:21 +0900)


Dan Carpenter (1):
  drm/exynos: fix a timeout loop

Daniel Vetter (1):
  drm/exynos: Stop using drm_framebuffer_unregister_private

Hoegeun Kwon (3):
  drm/exynos: mic: Add mode_set callback function
  drm/exynos: mic: Fix parse_dt function
  drm/panel: Add support for S6E3HA2 panel driver on TM2 board

Inki Dae (2):
  drm/exynos: remove unnecessary codes
  drm/exynos: use atomic helper commit

Joonyoung Shim (1):
  drm/exynos: g2d: prevent integer overflow in

Marek Szyprowski (1):
  drm/exynos: mic: Add runtime PM support

 drivers/gpu/drm/exynos/exynos_drm_crtc.c  |  28 +-
 drivers/gpu/drm/exynos/exynos_drm_crtc.h  |   4 -
 drivers/gpu/drm/exynos/exynos_drm_drv.c   | 114 
 drivers/gpu/drm/exynos/exynos_drm_fb.c|  32 +-
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   4 +-
 drivers/gpu/drm/exynos/exynos_drm_g2d.c   |  17 +-
 drivers/gpu/drm/exynos/exynos_drm_mic.c   | 126 +++--
 drivers/gpu/drm/exynos/exynos_mixer.c |   2 +-
 drivers/gpu/drm/panel/Kconfig |   6 +
 drivers/gpu/drm/panel/Makefile|   1 +
 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 754 ++
 11 files changed, 892 insertions(+), 196 deletions(-)
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v8 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board

[Inki Dae]

2017년 01월 24일 10:50에 Hoegeun Kwon 이(가) 쓴 글:
> Dear Thierry,
> 
> Could you please review this patch?

Thierry, I think this patch has been reviewed enough but no comment from you. 
Seems you are busy. I will pick up this.

Thanks.

> 
> Best Regards,
> Hoegeun Kwon
> 
> On 01/11/2017 03:33 PM, Hoegeun Kwon wrote:
>> This patch add support for MIPI-DSI based S6E3HA2 AMOLED panel
>> driver. This panel has 1440x2560 resolution in 5.7-inch physical
>> panel in the TM2 device.
>>
>> Signed-off-by: Donghwa Lee 
>> Signed-off-by: Hyungwon Hwang 
>> Signed-off-by: Hoegeun Kwon 
>> Tested-by: Chanwoo Choi 
>> Reviewed-by: Andrzej Hajda 
>> ---
>>   drivers/gpu/drm/panel/Kconfig |   6 +
>>   drivers/gpu/drm/panel/Makefile|   1 +
>>   drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 754 
>> ++
>>   3 files changed, 761 insertions(+)
>>   create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
>>
>> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
>> index 62aba97..d913c83 100644
>> --- a/drivers/gpu/drm/panel/Kconfig
>> +++ b/drivers/gpu/drm/panel/Kconfig
>> @@ -52,6 +52,12 @@ config DRM_PANEL_PANASONIC_VVX10F034N00
>> WUXGA (1920x1200) Novatek NT1397-based DSI panel as found in some
>> Xperia Z2 tablets
>>   +config DRM_PANEL_SAMSUNG_S6E3HA2
>> +tristate "Samsung S6E3HA2 DSI video mode panel"
>> +depends on OF
>> +depends on DRM_MIPI_DSI
>> +select VIDEOMODE_HELPERS
>> +
>>   config DRM_PANEL_SAMSUNG_S6E8AA0
>>   tristate "Samsung S6E8AA0 DSI video mode panel"
>>   depends on OF
>> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
>> index a5c7ec0..1d483b0 100644
>> --- a/drivers/gpu/drm/panel/Makefile
>> +++ b/drivers/gpu/drm/panel/Makefile
>> @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += 
>> panel-jdi-lt070me05000.o
>>   obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
>>   obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += 
>> panel-panasonic-vvx10f034n00.o
>>   obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
>> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
>>   obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
>>   obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>>   obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
>> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c 
>> b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
>> new file mode 100644
>> index 000..0b9c6f4
>> --- /dev/null
>> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
>> @@ -0,0 +1,754 @@
>> +/*
>> + * MIPI-DSI based s6e3ha2 AMOLED 5.7 inch panel driver.
>> + *
>> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
>> + * Donghwa Lee 
>> + * Hyungwon Hwang 
>> + * Hoegeun Kwon 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define S6E3HA2_MIN_BRIGHTNESS0
>> +#define S6E3HA2_MAX_BRIGHTNESS100
>> +#define S6E3HA2_DEFAULT_BRIGHTNESS80
>> +
>> +#define S6E3HA2_NUM_GAMMA_STEPS46
>> +#define S6E3HA2_GAMMA_CMD_CNT35
>> +#define S6E3HA2_VINT_STATUS_MAX10
>> +
>> +static const u8 gamma_tbl[S6E3HA2_NUM_GAMMA_STEPS][S6E3HA2_GAMMA_CMD_CNT] = 
>> {
>> +{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x82, 0x83,
>> +  0x85, 0x88, 0x8b, 0x8b, 0x84, 0x88, 0x82, 0x82, 0x89, 0x86, 0x8c,
>> +  0x94, 0x84, 0xb1, 0xaf, 0x8e, 0xcf, 0xad, 0xc9, 0x00, 0x00, 0x00,
>> +  0x00, 0x00 },
>> +{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x84, 0x84,
>> +  0x85, 0x87, 0x8b, 0x8a, 0x84, 0x88, 0x82, 0x82, 0x89, 0x86, 0x8a,
>> +  0x93, 0x84, 0xb0, 0xae, 0x8e, 0xc9, 0xa8, 0xc5, 0x00, 0x00, 0x00,
>> +  0x00, 0x00 },
>> +{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83,
>> +  0x85, 0x86, 0x8a, 0x8a, 0x84, 0x88, 0x81, 0x84, 0x8a, 0x88, 0x8a,
>> +  0x91, 0x84, 0xb1, 0xae, 0x8b, 0xd5, 0xb2, 0xcc, 0x00, 0x00, 0x00,
>> +  0x00, 0x00 },
>> +{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x89, 0x87, 0x87, 0x83, 0x83,
>> +  0x85, 0x86, 0x8a, 0x8a, 0x84, 0x87, 0x81, 0x84, 0x8a, 0x87, 0x8a,
>> +  0x91, 0x85, 0xae, 0xac, 0x8a, 0xc3, 0xa3, 0xc0, 0x00, 0x00, 0x00,
>> +  0x00, 0x00 },
>> +{ 0x00, 0xb8, 0x00, 0xc3, 0x00, 0xb1, 0x88, 0x86, 0x87, 0x85, 0x85,
>> +  0x86, 0x85, 0x88, 0x89, 0x84, 0x89, 0x82, 0x84, 0x87, 0x85, 0x8b,
>> +  0x91, 0x88, 0xad, 0xab, 0x8a, 0xb7, 0x9b, 0xb6, 0x00, 0x00, 0x00,
>> +  0x00, 0x00 

[Bug 74973] [radeonsi] Gimp OpenCL does not work

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=74973

--- Comment #6 from Jan Vesely  ---
(In reply to darkbasic from comment #5)
> Not sure, but I don't think so. AFAIK it' only GEGL-related stuff.

a lot of gegl tests pass OK for me on Turks.
notably clone test that includes box-blur and gaussian-blur operations passes
OK.

Can you retest with the latest llvm/mesa?

-- 
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 v3 16/24] drm/rockchip: dw-mipi-dsi: properly configure PHY timing

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:36PM +, John Keeping wrote:
> These values are specified as constant time periods but the PHY
> configuration is in terms of the current lane byte clock so using
> constant values guarantees that the timings will be outside the
> specification with some display configurations.
> 
> Derive the necessary configuration from the byte clock in order to
> ensure that the PHY configuration is correct.
> 
> Signed-off-by: John Keeping 
> ---
> v3:
> - Wrap some long lines
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 39 
> ++
>  1 file changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index cfe7e4ba305c..85edf6dd2bac 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -383,6 +383,26 @@ static void dw_mipi_dsi_phy_write(struct dw_mipi_dsi 
> *dsi, u8 test_code,
>   dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
>  }
>  
> +/**
> + * ns2bc - Nanoseconds to byte clock cycles
> + */
> +static inline unsigned int ns2bc(struct dw_mipi_dsi *dsi, int ns)
> +{
> + unsigned long byte_clk_khz = dsi->lane_mbps * MSEC_PER_SEC / 8;

Why multiply by 1000 (MSEC_PER_SEC) only to immediately divide by 1000?

> +
> + return (ns * (byte_clk_khz / 1000) + 999) / 1000;

Can you replace this whole function with:

return DIV_ROUND_UP(ns * dsi->lane_mbps / 8, 1000);

> +}
> +
> +/**
> + * ns2ui - Nanoseconds to UI time periods
> + */
> +static inline unsigned int ns2ui(struct dw_mipi_dsi *dsi, int ns)
> +{
> + unsigned long byte_clk_khz = dsi->lane_mbps * MSEC_PER_SEC;
> +
> + return (ns * (byte_clk_khz / 1000) + 999) / 1000;

Same remarks here.

Sean


> +}
> +
>  static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>  {
>   int ret, testdin, vco, val;
> @@ -434,10 +454,21 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>SETRD_MAX | POWER_MANAGE |
>TER_RESISTORS_ON);
>  
> -
> - dw_mipi_dsi_phy_write(dsi, 0x70, TLP_PROGRAM_EN | 0xf);
> - dw_mipi_dsi_phy_write(dsi, 0x71, THS_PRE_PROGRAM_EN | 0x55);
> - dw_mipi_dsi_phy_write(dsi, 0x72, THS_ZERO_PROGRAM_EN | 0xa);
> + dw_mipi_dsi_phy_write(dsi, 0x60, TLP_PROGRAM_EN | ns2bc(dsi, 500));
> + dw_mipi_dsi_phy_write(dsi, 0x61, THS_PRE_PROGRAM_EN | ns2ui(dsi, 40));
> + dw_mipi_dsi_phy_write(dsi, 0x62, THS_ZERO_PROGRAM_EN | ns2bc(dsi, 300));
> + dw_mipi_dsi_phy_write(dsi, 0x63, THS_PRE_PROGRAM_EN | ns2ui(dsi, 100));
> + dw_mipi_dsi_phy_write(dsi, 0x64, BIT(5) | ns2bc(dsi, 100));
> + dw_mipi_dsi_phy_write(dsi, 0x65, BIT(5) | (ns2bc(dsi, 60) + 7));
> +
> + dw_mipi_dsi_phy_write(dsi, 0x70, TLP_PROGRAM_EN | ns2bc(dsi, 500));
> + dw_mipi_dsi_phy_write(dsi, 0x71,
> +   THS_PRE_PROGRAM_EN | (ns2ui(dsi, 50) + 5));
> + dw_mipi_dsi_phy_write(dsi, 0x72,
> +   THS_ZERO_PROGRAM_EN | (ns2bc(dsi, 140) + 2));
> + dw_mipi_dsi_phy_write(dsi, 0x73,
> +   THS_PRE_PROGRAM_EN | (ns2ui(dsi, 60) + 8));
> + dw_mipi_dsi_phy_write(dsi, 0x74, BIT(5) | ns2bc(dsi, 100));
>  
>   dsi_write(dsi, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
>PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 99515] SIGSEGV MAPERR on Android nougat-x86 with mesa 17.0.0rc

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=99515

--- Comment #3 from Mauro Rossi  ---
Created attachment 129239
  --> https://bugs.freedesktop.org/attachment.cgi?id=129239=edit
various EGL issues with addr2line logs

-- 
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 v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Bjorn Helgaas
> Sent: Monday, January 30, 2017 3:41 PM
> To: Deucher, Alexander; Koenig, Christian
> Cc: linux...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; amd-...@lists.freedesktop.org; dri-
> de...@lists.freedesktop.org; Maarten Lankhorst
> Subject: [PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk
> more generic
> 
> The PCI Power Management Spec, r1.2, sec 5.6.1, requires a 10 millisecond
> delay when powering on a device, i.e., transitioning from state D3hot to
> D0.
> 
> Apparently some devices require more time, and d1f9809ed131
> ("drm/radeon:
> add quirk for d3 delay during switcheroo poweron for apple macbooks")
> added
> an additional delay for the Radeon device in a MacBook Pro.  4807c5a8a0c8
> ("drm/radeon: add a PX quirk list") made the affected device more explicit.
> 
> Add a generic PCI quirk to increase the d3_delay.  This means we will use
> the additional delay for *all* wakeups from D3, not just those initiated by
> radeon_switcheroo_set_state().
> 
> Signed-off-by: Bjorn Helgaas 
> CC: Maarten Lankhorst 

For the series:
Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/radeon/radeon_device.c |   12 
>  drivers/pci/quirks.c   |   13 +
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c
> b/drivers/gpu/drm/radeon/radeon_device.c
> index 8a1df2a1afbd..8b8fd981cae5 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -113,7 +113,6 @@ static inline bool radeon_is_atpx_hybrid(void) {
> return false; }
>  #endif
> 
>  #define RADEON_PX_QUIRK_DISABLE_PX  (1 << 0)
> -#define RADEON_PX_QUIRK_LONG_WAKEUP (1 << 1)
> 
>  struct radeon_px_quirk {
>   u32 chip_vendor;
> @@ -136,9 +135,6 @@ static struct radeon_px_quirk radeon_px_quirk_list[]
> = {
>* https://bugzilla.kernel.org/show_bug.cgi?id=51381
>*/
>   { PCI_VENDOR_ID_ATI, 0x6840, 0x1043, 0x2122,
> RADEON_PX_QUIRK_DISABLE_PX },
> - /* macbook pro 8.2 */
> - { PCI_VENDOR_ID_ATI, 0x6741, PCI_VENDOR_ID_APPLE, 0x00e2,
> RADEON_PX_QUIRK_LONG_WAKEUP },
> - { 0, 0, 0, 0, 0 },
>  };
> 
>  bool radeon_is_px(struct drm_device *dev)
> @@ -1241,25 +1237,17 @@ static void radeon_check_arguments(struct
> radeon_device *rdev)
>  static void radeon_switcheroo_set_state(struct pci_dev *pdev, enum
> vga_switcheroo_state state)
>  {
>   struct drm_device *dev = pci_get_drvdata(pdev);
> - struct radeon_device *rdev = dev->dev_private;
> 
>   if (radeon_is_px(dev) && state == VGA_SWITCHEROO_OFF)
>   return;
> 
>   if (state == VGA_SWITCHEROO_ON) {
> - unsigned d3_delay = dev->pdev->d3_delay;
> -
>   printk(KERN_INFO "radeon: switched on\n");
>   /* don't suspend or resume card normally */
>   dev->switch_power_state =
> DRM_SWITCH_POWER_CHANGING;
> 
> - if (d3_delay < 20 && (rdev->px_quirk_flags &
> RADEON_PX_QUIRK_LONG_WAKEUP))
> - dev->pdev->d3_delay = 20;
> -
>   radeon_resume_kms(dev, true, true);
> 
> - dev->pdev->d3_delay = d3_delay;
> -
>   dev->switch_power_state = DRM_SWITCH_POWER_ON;
>   drm_kms_helper_poll_enable(dev);
>   } else {
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 1800befa8b8b..512d7a875d62 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1683,6 +1683,19 @@
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,  0x2609,
> quirk_intel_pcie_pm);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x260a,
> quirk_intel_pcie_pm);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x260b,
> quirk_intel_pcie_pm);
> 
> +static void quirk_radeon_pm(struct pci_dev *dev)
> +{
> + if (dev->subsystem_vendor == PCI_VENDOR_ID_APPLE &&
> + dev->subsystem_device == 0x00e2) {
> + if (dev->d3_delay < 20) {
> + dev->d3_delay = 20;
> + dev_info(>dev, "extending delay after power-
> on from D3 to %d msec\n",
> +  dev->d3_delay);
> + }
> + }
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x6741,
> quirk_radeon_pm);
> +
>  #ifdef CONFIG_X86_IO_APIC
>  /*
>   * Boot interrupts on some chipsets cannot be turned off. For these
> chipsets,
> 
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3] drm/edid: Complete CEA modedb(VIC 1-107)

[Harry Wentland]

We'd like these in amdgpu as well. I've only skimmed the patch but it looks 
pretty good to me.

Acked-by: Harry Wentland 

Harry


On 2017-01-29 12:41 AM, Shashank Sharma wrote:
> CEA-861-F specs defines new 4k video modes to be used with
> HDMI 2.0 EDIDs. These modes start at VIC=93 and go all the
> way till VIC=107.
>
> Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now
> to be able to parse 4k modes using the existing techniques, we have
> to complete the modedb (VIC=65 onwards).
>
> This patch adds:
> - Timings for existing CEA video modes (from VIC=65 till VIC=92)
> - Newly added 4k modes (from VIC=93 to VIC=107).
>
> Cc: Jose Abreu 
> Cc: Alex Deucher 
> Cc: Andrzej Hajda 
>
> V2: Addressed review comments from Jose:
> - fix the timings for VIC 83, 90 and 91
> - fix formatting for VIC 93-107
>
> V3: Rebase on drm-tip
>
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Sonika Jindal 
> Reviewed-by: Jose Abreu 
> Reviewed-by: Alex Deucher 
> ---
>  drivers/gpu/drm/drm_edid.c | 215 
> +
>  1 file changed, 215 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index baa6ccb..b4eae1f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -995,6 +995,221 @@ struct minimode {
>  2492, 2640, 0, 1080, 1084, 1089, 1125, 0,
>  DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
>.vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
> + /* 65 - 1280x720@24Hz */
> + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 59400, 1280, 3040,
> +3080, 3300, 0, 720, 725, 730, 750, 0,
> +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
> +   .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
> + /* 66 - 1280x720@25Hz */
> + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 3700,
> +3740, 3960, 0, 720, 725, 730, 750, 0,
> +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
> +   .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
> + /* 67 - 1280x720@30Hz */
> + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 3040,
> +3080, 3300, 0, 720, 725, 730, 750, 0,
> +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
> +   .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
> + /* 68 - 1280x720@50Hz */
> + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1720,
> +1760, 1980, 0, 720, 725, 730, 750, 0,
> +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
> +   .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
> + /* 69 - 1280x720@60Hz */
> + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1390,
> +1430, 1650, 0, 720, 725, 730, 750, 0,
> +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
> +   .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
> + /* 70 - 1280x720@100Hz */
> + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 148500, 1280, 1720,
> +1760, 1980, 0, 720, 725, 730, 750, 0,
> +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
> +   .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
> + /* 71 - 1280x720@120Hz */
> + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 148500, 1280, 1390,
> +1430, 1650, 0, 720, 725, 730, 750, 0,
> +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
> +   .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
> + /* 72 - 1920x1080@24Hz */
> + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2558,
> +2602, 2750, 0, 1080, 1084, 1089, 1125, 0,
> +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
> +   .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
> + /* 73 - 1920x1080@25Hz */
> + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2448,
> +2492, 2640, 0, 1080, 1084, 1089, 1125, 0,
> +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
> +   .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
> + /* 74 - 1920x1080@30Hz */
> + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2008,
> +2052, 2200, 0, 1080, 1084, 1089, 1125, 0,
> +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
> +   .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
> + /* 75 - 1920x1080@50Hz */
> + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 148500, 1920, 2448,
> +2492, 2640, 0, 1080, 1084, 1089, 1125, 0,
> +   

[PATCH v1 1/2] drm/amdgpu: remove unnecessary save/restore of pdev->d3_delay

[Bjorn Helgaas]

Remove unnecessary save/restore of pdev->d3_delay.

The only assignments to pdev->d3_delay are in radeon_switcheroo_set_state()
and some quirks, none of which should be relevant in the
amdgpu_switcheroo_set_state() path.

Signed-off-by: Bjorn Helgaas 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 60bd4afe45c8..3a403a87ec62 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1042,16 +1042,12 @@ static void amdgpu_switcheroo_set_state(struct pci_dev 
*pdev, enum vga_switchero
return;
 
if (state == VGA_SWITCHEROO_ON) {
-   unsigned d3_delay = dev->pdev->d3_delay;
-
printk(KERN_INFO "amdgpu: switched on\n");
/* don't suspend or resume card normally */
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
amdgpu_device_resume(dev, true, true);
 
-   dev->pdev->d3_delay = d3_delay;
-
dev->switch_power_state = DRM_SWITCH_POWER_ON;
drm_kms_helper_poll_enable(dev);
} else {

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic

[Bjorn Helgaas]

The PCI Power Management Spec, r1.2, sec 5.6.1, requires a 10 millisecond
delay when powering on a device, i.e., transitioning from state D3hot to
D0.

Apparently some devices require more time, and d1f9809ed131 ("drm/radeon:
add quirk for d3 delay during switcheroo poweron for apple macbooks") added
an additional delay for the Radeon device in a MacBook Pro.  4807c5a8a0c8
("drm/radeon: add a PX quirk list") made the affected device more explicit.

Add a generic PCI quirk to increase the d3_delay.  This means we will use
the additional delay for *all* wakeups from D3, not just those initiated by
radeon_switcheroo_set_state().

Signed-off-by: Bjorn Helgaas 
CC: Maarten Lankhorst 
---
 drivers/gpu/drm/radeon/radeon_device.c |   12 
 drivers/pci/quirks.c   |   13 +
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index 8a1df2a1afbd..8b8fd981cae5 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -113,7 +113,6 @@ static inline bool radeon_is_atpx_hybrid(void) { return 
false; }
 #endif
 
 #define RADEON_PX_QUIRK_DISABLE_PX  (1 << 0)
-#define RADEON_PX_QUIRK_LONG_WAKEUP (1 << 1)
 
 struct radeon_px_quirk {
u32 chip_vendor;
@@ -136,9 +135,6 @@ static struct radeon_px_quirk radeon_px_quirk_list[] = {
 * https://bugzilla.kernel.org/show_bug.cgi?id=51381
 */
{ PCI_VENDOR_ID_ATI, 0x6840, 0x1043, 0x2122, RADEON_PX_QUIRK_DISABLE_PX 
},
-   /* macbook pro 8.2 */
-   { PCI_VENDOR_ID_ATI, 0x6741, PCI_VENDOR_ID_APPLE, 0x00e2, 
RADEON_PX_QUIRK_LONG_WAKEUP },
-   { 0, 0, 0, 0, 0 },
 };
 
 bool radeon_is_px(struct drm_device *dev)
@@ -1241,25 +1237,17 @@ static void radeon_check_arguments(struct radeon_device 
*rdev)
 static void radeon_switcheroo_set_state(struct pci_dev *pdev, enum 
vga_switcheroo_state state)
 {
struct drm_device *dev = pci_get_drvdata(pdev);
-   struct radeon_device *rdev = dev->dev_private;
 
if (radeon_is_px(dev) && state == VGA_SWITCHEROO_OFF)
return;
 
if (state == VGA_SWITCHEROO_ON) {
-   unsigned d3_delay = dev->pdev->d3_delay;
-
printk(KERN_INFO "radeon: switched on\n");
/* don't suspend or resume card normally */
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
-   if (d3_delay < 20 && (rdev->px_quirk_flags & 
RADEON_PX_QUIRK_LONG_WAKEUP))
-   dev->pdev->d3_delay = 20;
-
radeon_resume_kms(dev, true, true);
 
-   dev->pdev->d3_delay = d3_delay;
-
dev->switch_power_state = DRM_SWITCH_POWER_ON;
drm_kms_helper_poll_enable(dev);
} else {
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 1800befa8b8b..512d7a875d62 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1683,6 +1683,19 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x2609, 
quirk_intel_pcie_pm);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,   0x260a, quirk_intel_pcie_pm);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,   0x260b, quirk_intel_pcie_pm);
 
+static void quirk_radeon_pm(struct pci_dev *dev)
+{
+   if (dev->subsystem_vendor == PCI_VENDOR_ID_APPLE &&
+   dev->subsystem_device == 0x00e2) {
+   if (dev->d3_delay < 20) {
+   dev->d3_delay = 20;
+   dev_info(>dev, "extending delay after power-on 
from D3 to %d msec\n",
+dev->d3_delay);
+   }
+   }
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x6741, quirk_radeon_pm);
+
 #ifdef CONFIG_X86_IO_APIC
 /*
  * Boot interrupts on some chipsets cannot be turned off. For these chipsets,

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 0/2] drm amdgpu/radeon: clean up d3_delay usage

[Bjorn Helgaas]

amdgpu doesn't need to touch pdev->d3_delay at all.

radeon has a d3_delay quirk for MacBook Pro, but it only affects
radeon_switcheroo_set_state().  I think it should affect wakeups done by
the PCI core as well.

---

Bjorn Helgaas (2):
  drm/amdgpu: remove unnecessary save/restore of pdev->d3_delay
  drm/radeon: make MacBook Pro d3_delay quirk more generic


 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |4 
 drivers/gpu/drm/radeon/radeon_device.c |   12 
 drivers/pci/quirks.c   |   13 +
 3 files changed, 13 insertions(+), 16 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: remove unnecessary fault wrappers

[Sean Paul]

On Mon, Jan 30, 2017 at 12:20:02PM -0700, Ross Zwisler wrote:
> The fault wrappers drm_vm_fault(), drm_vm_shm_fault(), drm_vm_dma_fault()
> and drm_vm_sg_fault() used to provide extra logic beyond what was in the
> "drm_do_*" versions of these functions, but as of this commit:
> 
> commit ca0b07d9a969 ("drm: convert drm from nopage to fault.")
> 
> They are just unnecessary wrappers that do nothing.  Remove them.

Can we rename the drm_do_* functions to remove "do_" instead? I don't think it
adds anything and this seems like a good time to fix that.

Sean

> 
> Signed-off-by: Ross Zwisler 
> ---
>  drivers/gpu/drm/drm_vm.c | 28 
>  1 file changed, 4 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> index bae6e26..e677b11 100644
> --- a/drivers/gpu/drm/drm_vm.c
> +++ b/drivers/gpu/drm/drm_vm.c
> @@ -349,50 +349,30 @@ static int drm_do_vm_sg_fault(struct vm_fault *vmf)
>   return 0;
>  }
>  
> -static int drm_vm_fault(struct vm_fault *vmf)
> -{
> - return drm_do_vm_fault(vmf);
> -}
> -
> -static int drm_vm_shm_fault(struct vm_fault *vmf)
> -{
> - return drm_do_vm_shm_fault(vmf);
> -}
> -
> -static int drm_vm_dma_fault(struct vm_fault *vmf)
> -{
> - return drm_do_vm_dma_fault(vmf);
> -}
> -
> -static int drm_vm_sg_fault(struct vm_fault *vmf)
> -{
> - return drm_do_vm_sg_fault(vmf);
> -}
> -
>  /** AGP virtual memory operations */
>  static const struct vm_operations_struct drm_vm_ops = {
> - .fault = drm_vm_fault,
> + .fault = drm_do_vm_fault,
>   .open = drm_vm_open,
>   .close = drm_vm_close,
>  };
>  
>  /** Shared virtual memory operations */
>  static const struct vm_operations_struct drm_vm_shm_ops = {
> - .fault = drm_vm_shm_fault,
> + .fault = drm_do_vm_shm_fault,
>   .open = drm_vm_open,
>   .close = drm_vm_shm_close,
>  };
>  
>  /** DMA virtual memory operations */
>  static const struct vm_operations_struct drm_vm_dma_ops = {
> - .fault = drm_vm_dma_fault,
> + .fault = drm_do_vm_dma_fault,
>   .open = drm_vm_open,
>   .close = drm_vm_close,
>  };
>  
>  /** Scatter-gather virtual memory operations */
>  static const struct vm_operations_struct drm_vm_sg_ops = {
> - .fault = drm_vm_sg_fault,
> + .fault = drm_do_vm_sg_fault,
>   .open = drm_vm_open,
>   .close = drm_vm_close,
>  };
> -- 
> 2.7.4

-- 
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 v3 15/24] drm/rockchip: dw-mipi-dsi: configure PHY before enabling

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:35PM +, John Keeping wrote:
> The bias, bandgap and PLL should all be configured before we enable
> them.
> 

Do you know why the test codes are hard-coded magic? It'd be nice to make some
sense of them in a future patch.

Reviewed-by: Sean Paul 

> Signed-off-by: John Keeping 
> ---
> v3:
> - Squash together two patches that both affect initialization order of
>   the PHY
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 5b3068e9e8db..cfe7e4ba305c 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -413,12 +413,17 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>  
>   dw_mipi_dsi_phy_write(dsi, 0x44, HSFREQRANGE_SEL(testdin));
>  
> - dw_mipi_dsi_phy_write(dsi, 0x19, PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN);
>   dw_mipi_dsi_phy_write(dsi, 0x17, INPUT_DIVIDER(dsi->input_div));
>   dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_LOW_SEL(dsi->feedback_div) |
>LOW_PROGRAM_EN);
>   dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_HIGH_SEL(dsi->feedback_div) |
>HIGH_PROGRAM_EN);
> + dw_mipi_dsi_phy_write(dsi, 0x19, PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN);
> +
> + dw_mipi_dsi_phy_write(dsi, 0x22, LOW_PROGRAM_EN |
> +  BIASEXTR_SEL(BIASEXTR_127_7));
> + dw_mipi_dsi_phy_write(dsi, 0x22, HIGH_PROGRAM_EN |
> +  BANDGAP_SEL(BANDGAP_96_10));
>  
>   dw_mipi_dsi_phy_write(dsi, 0x20, POWER_CONTROL | INTERNAL_REG_CURRENT |
>BIAS_BLOCK_ON | BANDGAP_ON);
> @@ -429,10 +434,6 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>SETRD_MAX | POWER_MANAGE |
>TER_RESISTORS_ON);
>  
> - dw_mipi_dsi_phy_write(dsi, 0x22, LOW_PROGRAM_EN |
> -  BIASEXTR_SEL(BIASEXTR_127_7));
> - dw_mipi_dsi_phy_write(dsi, 0x22, HIGH_PROGRAM_EN |
> -  BANDGAP_SEL(BANDGAP_96_10));
>  
>   dw_mipi_dsi_phy_write(dsi, 0x70, TLP_PROGRAM_EN | 0xf);
>   dw_mipi_dsi_phy_write(dsi, 0x71, THS_PRE_PROGRAM_EN | 0x55);
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 2/4] drm/msm: drop qcom,chipid

[Rob Clark]

On Mon, Jan 30, 2017 at 2:54 PM, Eric Anholt  wrote:
> Rob Clark  writes:
>
>> On Mon, Jan 30, 2017 at 1:09 PM, Eric Anholt  wrote:
>>> Rob Clark  writes:
>>>
 The original way we determined the gpu version was based on downstream
 bindings from android kernel.  A cleaner way is to get the version from
 the compatible string.

 Note that no upstream dtb uses these bindings.  But the code still
 supports falling back to the legacy bindings (with a warning), so that
 we are still compatible with the gpu dt node from android device
 kernels.
>>>
>>> The gpulist[] seems pretty short, like you could just have 7 compatible
>>> strings in dt_match[] and point them directly at corresponding the
>>> gpulist[] entry.  Or are there lots of patch levels, with the same
>>> struct adreno_info values?
>>
>> The list currently is rather incomplete (which may or may not matter,
>> I guess it comes down to how many different phones/tablets out there
>> people try to get an upstream kernel working on).  And it has those
>> ANY_ID wildcard entries.
>>
>> A full list of all the gpu's including all their patchlevels would be
>> quite long.
>>
>> We might end up wanting to split the quirks out differently, since
>> those tend to apply to specific patchlevel's.. I had to change the
>> existing entry for a530 from a530.* to a530.2 for this reason.  But
>> that won't effect the bindings so that is an implementation detail we
>> can worry about later.
>
> I think that using the of_match_device() mechanism from the specific
> strings listed as the compatible would make more sense than this string
> parsing.  You have to write a gpulist[] entry anyway for the module to
> load.  So that means adding about 7 values today to the compatible
> string dt_match, and the code to use of_match_device() to get your
> struct adreno_info.  Then the current version number lookup loop would
> be just for the old DT compatibility and there's no string parsing.

well, the problem is still that we would end up needing entries for
each gpu version + patchlevel, which I was trying to avoid.. I think
otherwise, if we started adding more adreno variants the table would
get quite large.  The ANY_ID entries keep the table size down
currently.

> However, it's your driver.  The code seems correct, and using specific
> compatible strings is an obviously good step.

I may possibly re-work this in the future, but was leaning more
towards perhaps introducing some sort of of_match_device_wildcard(id,
dev, ...) type function, and allowing a compat string match like
"qcom,adreno-%1u%1u%1u.%u"

I guess maybe re-arranging this so multiple compat table entries point
back to the same 'struct adreno_info' might be workable, but that list
could still grow quite long.  At any rate, that doesn't change how the
bindings look so that can come later.

> Reviewed-by: Eric Anholt 

thanks

BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 95306] Random Blank(black) screens on "Carrizo"

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=95306

--- Comment #54 from Patrick Laurin  ---
Interesting enough, I started having screen blackouts again on RC6, but RC4
is fine, I can use it all day without issues.

On 24 January 2017 at 20:13,  wrote:

> *Comment # 53  on
> bug 95306  from Michel
> Dänzer  *
>
> (In reply to Patrick Laurin from comment #52 
> )> There's something 
> wrong tough, that's probably unrelated, is that the mouse
> > gets invisible after coming back from sleep.
>
> That should be fixed 
> byhttps://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=69bcc0b7140c30de552aa3ef08322295862e8e2f
> , which will be in 4.10-rc6.
>
> --
> You are receiving this mail because:
>
>- You are on the CC list for the bug.
>
>

-- 
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 v3 14/24] drm/rockchip: dw-mipi-dsi: ensure PHY is reset

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:34PM +, John Keeping wrote:
> Also don't power up the DSI host at this point since this is not
> necessary in order to configure the PHY and we do so later when
> selecting video or command mode.
> 

Reviewed-by: Sean Paul 

> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index c2e0ba96e0a0..5b3068e9e8db 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -397,7 +397,10 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>   return testdin;
>   }
>  
> - dsi_write(dsi, DSI_PWR_UP, POWERUP);
> + /* Start by clearing PHY state */
> + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
> + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLR);
> + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
>  
>   dw_mipi_dsi_phy_write(dsi, 0x10, BYPASS_VCO_RANGE |
>VCO_RANGE_CON_SEL(vco) |
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 v3 13/24] drm/rockchip: dw-mipi-dsi: fix escape clock rate

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:33PM +, John Keeping wrote:
> This clock rate is derived from the PHY PLL, so it should be calculated
> dynamically.  Use the same calculation as the vendor kernel to derive
> the escape clock speed.
> 

Nit below, but

Reviewed-by: Sean Paul 

> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Improve the commit message a bit
> - Add Chris' Reviewed-by
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 290282e86d16..c2e0ba96e0a0 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -710,11 +710,13 @@ static void dw_mipi_dsi_disable(struct dw_mipi_dsi *dsi)
>  
>  static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
>  {

Nit: It would be nice to add a comment to the effect of "You are not meant to
understand this, it comes from the vendor kernel"

> + u32 esc_clk_division = (dsi->lane_mbps >> 3) / 20 + 1;
> +
>   dsi_write(dsi, DSI_PWR_UP, RESET);
>   dsi_write(dsi, DSI_PHY_RSTZ, PHY_DISFORCEPLL | PHY_DISABLECLK
> | PHY_RSTZ | PHY_SHUTDOWNZ);
>   dsi_write(dsi, DSI_CLKMGR_CFG, TO_CLK_DIVIDSION(10) |
> -   TX_ESC_CLK_DIVIDSION(7));
> +   TX_ESC_CLK_DIVIDSION(esc_clk_division));
>  }
>  
>  static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 v3 12/24] drm/rockchip: dw-mipi-dsi: allow commands in panel_disable

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:32PM +, John Keeping wrote:
> Panel drivers may want to sent commands during the disable function, for
> example MIPI_DCS_SET_DISPLAY_OFF before the video signal ends.  In order
> to send commands we need to write to registers, so pclk must be enabled.
> 
> While changing this, remove the unnecessary code after the panel
> unprepare call which seems to be a workaround for a specific panel and
> thus belongs in the panel driver.

Do you know which panel? If the panel driver is upstream, we should make sure we
migrate this hack before removing it here. If it's downstream somewhere,

Reviewed-by: Sean Paul 

> 
> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 12 ++--
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 7ada6d8ed143..290282e86d16 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -846,24 +846,16 @@ static void dw_mipi_dsi_encoder_disable(struct 
> drm_encoder *encoder)
>  {
>   struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
>  
> - drm_panel_disable(dsi->panel);
> -
>   if (clk_prepare_enable(dsi->pclk)) {
>   dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__);
>   return;
>   }
>  
> + drm_panel_disable(dsi->panel);
> +
>   dw_mipi_dsi_set_mode(dsi, DW_MIPI_DSI_CMD_MODE);
>   drm_panel_unprepare(dsi->panel);
> - dw_mipi_dsi_set_mode(dsi, DW_MIPI_DSI_VID_MODE);
>  
> - /*
> -  * This is necessary to make sure the peripheral will be driven
> -  * normally when the display is enabled again later.
> -  */
> - msleep(120);
> -
> - dw_mipi_dsi_set_mode(dsi, DW_MIPI_DSI_CMD_MODE);
>   dw_mipi_dsi_disable(dsi);
>   clk_disable_unprepare(dsi->pclk);
>  }
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 01/10] drm/etnaviv: add uapi for register read feature

[Thierry Reding]

On Fri, Dec 09, 2016 at 12:21:22PM +0100, Christian Gmeiner wrote:
> We need to readout some registers _after_ the submited command
> buffer got executed in order to support perf counters.
> There is no way to read register via command stream - even the
> Vivante kernel driver does it via a special ioctl.
> 
> Signed-off-by: Christian Gmeiner 
> ---
>  include/uapi/drm/etnaviv_drm.h | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
> index 2584c1c..0d30604 100644
> --- a/include/uapi/drm/etnaviv_drm.h
> +++ b/include/uapi/drm/etnaviv_drm.h
> @@ -150,6 +150,13 @@ struct drm_etnaviv_gem_submit_bo {
>   __u64 presumed;   /* in/out, presumed buffer address */
>  };
>  
> +struct drm_etnaviv_gem_submit_readback {
> + __u32 readback_offset;/* in, offset from readback_bo */
> + __u32 readback_idx;   /* in, index of readback_bo buffer */
> + __u32 reg;/* in, register to read */
> + __u32 flags;  /* in, needs to be 0 */
> +};
> +
>  /* Each cmdstream submit consists of a table of buffers involved, and
>   * one or more cmdstream buffers.  This allows for conditional execution
>   * (context-restore), and IB buffers needed for per tile/bin draw cmds.
> @@ -167,6 +174,9 @@ struct drm_etnaviv_gem_submit {
>   __u64 bos;/* in, ptr to array of submit_bo's */
>   __u64 relocs; /* in, ptr to array of submit_reloc's */
>   __u64 stream; /* in, ptr to cmdstream */
> + __u64 readbacks;  /* in, ptr to array of submit_readback's */
> + __u32 nr_readbacks;   /* in, number of submit_readback's */
> + __u32 padding;
>  };

What about ABI stability? How's this going to work when userspace uses
old headers but runs against a new kernel? What about userspace using
newer headers than the kernel?

Thierry


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 11/24] drm/rockchip: dw-mipi-dsi: prepare panel after phy init

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:31PM +, John Keeping wrote:
> Some panels need to be configured with commands sent over the MIPI link,
> which they will do in the prepare hook.  Call this after the PHY has
> been initialized so that we are able to send commands to the panel.
> 

Reviewed-by: Sean Paul 

> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index ddbc037e7ced..7ada6d8ed143 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -896,12 +896,14 @@ static void dw_mipi_dsi_encoder_enable(struct 
> drm_encoder *encoder)
>   dw_mipi_dsi_dphy_timing_config(dsi);
>   dw_mipi_dsi_dphy_interface_config(dsi);
>   dw_mipi_dsi_clear_err(dsi);
> - if (drm_panel_prepare(dsi->panel))
> - dev_err(dsi->dev, "failed to prepare panel\n");
>  
>   dw_mipi_dsi_phy_init(dsi);
>   dw_mipi_dsi_wait_for_two_frames(mode);
>  
> + dw_mipi_dsi_set_mode(dsi, DW_MIPI_DSI_CMD_MODE);
> + if (drm_panel_prepare(dsi->panel))
> + dev_err(dsi->dev, "failed to prepare panel\n");
> +
>   dw_mipi_dsi_set_mode(dsi, DW_MIPI_DSI_VID_MODE);
>   drm_panel_enable(dsi->panel);
>  
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 v3 24/24] drm/rockchip: dw-mipi-dsi: support read commands

[Sean Paul]

On Mon, Jan 30, 2017 at 06:14:27PM +, John Keeping wrote:
> On Mon, 30 Jan 2017 10:26:11 -0500, Sean Paul wrote:
> 
> > On Sun, Jan 29, 2017 at 01:24:44PM +, John Keeping wrote:
> > > I haven't found any method for getting the length of a response, so this
> > > just uses the requested rx_len
> > > 
> > > Signed-off-by: John Keeping 
> > > ---
> > > v3:
> > > - Fix checkpatch warnings
> > > Unchanged in v2
> > > 
> > >  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 56 
> > > ++
> > >  1 file changed, 56 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> > > b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > > index cf3ca6b0cbdb..cc58ada75425 100644
> > > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > > @@ -678,6 +678,56 @@ static int dw_mipi_dsi_dcs_long_write(struct 
> > > dw_mipi_dsi *dsi,
> > >   return dw_mipi_dsi_gen_pkt_hdr_write(dsi, hdr_val);
> > >  }
> > >  
> > > +static int dw_mipi_dsi_dcs_read(struct dw_mipi_dsi *dsi,
> > > + const struct mipi_dsi_msg *msg)
> > > +{
> > > + const u8 *tx_buf = msg->tx_buf;
> > > + u8 *rx_buf = msg->rx_buf;
> > > + size_t i;
> > > + int ret, val;
> > > +
> > > + dsi_write(dsi, DSI_PCKHDL_CFG, EN_CRC_RX | EN_ECC_RX | EN_BTA);
> > > + dsi_write(dsi, DSI_GEN_HDR,
> > > +   GEN_HDATA(tx_buf[0]) | GEN_HTYPE(msg->type));
> > > +
> > > + ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> > > +  val, !(val & GEN_RD_CMD_BUSY), 1000,
> > > +  CMD_PKT_STATUS_TIMEOUT_US);
> > > + if (ret < 0) {
> > > + dev_err(dsi->dev, "failed to read command response\n");
> > > + return ret;
> > > + }
> > > +
> > > + for (i = 0; i < msg->rx_len;) {
> > > + u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
> > > +
> > > + while (i < msg->rx_len) {
> > > + rx_buf[i] = pld & 0xff;
> > > + pld >>= 8;
> > > + i++;
> > > + }
> > > + }  
> > 
> > AFAICT, the outer for loop just initializes i and ensures msg->rx_len is
> > non-zero? 
> > 
> > I think the following would be easier to read (and safe against the case 
> > where
> > msg->rx_len > sizeof(pld) (even though this shouldn't happen according to 
> > DCS
> > spec)).
> > 
> > if (msg->rx_len > 0) {
> > u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
> > memcpy(rx_buf, , MIN(msg->rx_len, sizeof(pld));
> > }
> 
> I think the intent was to handle rx_len > 4, but the patch is obvously
> completely broken regarding that.  As far as I can tell, rx_len is
> limited by the maximum return packet size which can be any value up to
> the maximum size of a long packet, so we may need to read from the FIFO
> multiple times.
> 
> The loop should be something like this:
> 
>   for (i = 0; i < msg->rx_len;) {
>   u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
>   int j;
> 
>   for (j = 0; j < 4 && i < msg->rx_len; i++, j++) {
>   rx_buf[i] = pld & 0xff;
>   pld >>= 8;
>   }
>   }

Short packets should never exceed 32 bits, so I don't think you need to add the
nested loop.

Sean


> 
> I have successfully read 5 bytes from a DSI display using this code, but
> I'm tempted to just drop this patch since I only used it for debugging
> while bringing up a new panel.
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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: [v3 PATCH 1/3] drm/atomic: Save flip flags in drm_crtct_state

[Grodzovsky, Andrey]

> -Original Message-
> From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
> Sent: Monday, January 30, 2017 6:05 AM
> To: Grodzovsky, Andrey
> Cc: dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org;
> nouv...@lists.freedesktop.org; daniel.vet...@intel.com; dc_upstream
> Subject: Re: [v3 PATCH 1/3] drm/atomic: Save flip flags in drm_crtct_state
> 
> Hi Andrey,
> 
> Thank you for the patch.
> 
> On Saturday 28 Jan 2017 21:26:49 Andrey Grodzovsky wrote:
> > Allows using atomic flip helpers for drivers using ASYNC flip.
> > Remove ASYNC_FLIP restriction in helpers and caches the page flip
> > flags in drm_crtc_state to be used in the low level drivers.
> >
> > v2:
> > Resending the patch since the original was broken.
> >
> > v3:
> > Save flag in crtc_state instead of plane_state
> >
> > Signed-off-by: Andrey Grodzovsky 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 19 +--
> >  include/drm/drm_crtc.h  |  8 +++-
> >  include/drm/drm_plane.h |  1 +
> >  3 files changed, 13 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..28065ee 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -2737,7 +2737,8 @@ static int page_flip_common(
> > struct drm_atomic_state *state,
> > struct drm_crtc *crtc,
> > struct drm_framebuffer *fb,
> > -   struct drm_pending_vblank_event *event)
> > +   struct drm_pending_vblank_event *event,
> > +   uint32_t flags)
> >  {
> > struct drm_plane *plane = crtc->primary;
> > struct drm_plane_state *plane_state; @@ -2749,12 +2750,12 @@
> static
> > int page_flip_common(
> > return PTR_ERR(crtc_state);
> >
> > crtc_state->event = event;
> > +   crtc_state->pflip_flags = flags;
> >
> > plane_state = drm_atomic_get_plane_state(state, plane);
> > if (IS_ERR(plane_state))
> > return PTR_ERR(plane_state);
> >
> > -
> > ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> > if (ret != 0)
> > return ret;
> > @@ -2781,10 +2782,6 @@ static int page_flip_common(
> >   * Provides a default _crtc_funcs.page_flip implementation
> >   * using the atomic driver interface.
> >   *
> > - * Note that for now so called async page flips (i.e. updates which
> > are not
> > - * synchronized to vblank) are not supported, since the atomic
> > interfaces have - * no provisions for this yet.
> > - *
> >   * Returns:
> >   * Returns 0 on success, negative errno numbers on failure.
> >   *
> > @@ -2800,9 +2797,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc
> > *crtc, struct drm_atomic_state *state;
> > int ret = 0;
> >
> > -   if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> > -   return -EINVAL;
> > -
> > state = drm_atomic_state_alloc(plane->dev);
> > if (!state)
> > return -ENOMEM;
> > @@ -2810,7 +2804,7 @@ int drm_atomic_helper_page_flip(struct drm_crtc
> > *crtc,
> > state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> >
> >  retry:
> > -   ret = page_flip_common(state, crtc, fb, event);
> > +   ret = page_flip_common(state, crtc, fb, event, flags);
> > if (ret != 0)
> > goto fail;
> >
> > @@ -2865,9 +2859,6 @@ int drm_atomic_helper_page_flip_target(
> > struct drm_crtc_state *crtc_state;
> > int ret = 0;
> >
> > -   if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> > -   return -EINVAL;
> > -
> > state = drm_atomic_state_alloc(plane->dev);
> > if (!state)
> > return -ENOMEM;
> > @@ -2875,7 +2866,7 @@ int drm_atomic_helper_page_flip_target(
> > state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> >
> >  retry:
> > -   ret = page_flip_common(state, crtc, fb, event);
> > +   ret = page_flip_common(state, crtc, fb, event, flags);
> > if (ret != 0)
> > goto fail;
> >
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index
> > 5c77c3f..76457a4 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -162,10 +162,16 @@ struct drm_crtc_state {
> >  * Target vertical blank period when a page flip
> >  * should take effect.
> >  */
> > -
> > u32 target_vblank;
> >
> > /**
> > +* @pflip_flags:
> > +*
> > +* Flip related config options
> 
> This isn't detailed enough. I propose something along the lines of
> 
> "DRM_MODE_PAGE_FLIP_* page flip flags, as passed to the page flip ioctl.
> Always zero for atomic commits that don't originate from a page flip ioctl."
> 
> You will then also need to reset the field to 0 at an appropriate point, as 
> it's
> more an atomic commit transaction information than a state. Apart from that
> this patch looks good to me.
Thanks for your 

Re: [PATCH v3 06/24] drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf

[Sean Paul]

On Mon, Jan 30, 2017 at 06:16:36PM +, John Keeping wrote:
> On Mon, 30 Jan 2017 13:01:46 -0500, Sean Paul wrote:
> 
> > On Sun, Jan 29, 2017 at 01:24:26PM +, John Keeping wrote:
> > > As a side-effect of this, encode the endianness explicitly rather than
> > > casting a u16.
> > > 
> > > Signed-off-by: John Keeping 
> > > Reviewed-by: Chris Zhong 
> > > ---
> > > v3:
> > > - Add Chris' Reviewed-by
> > > Unchanged in v2
> > > 
> > >  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 9 +++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> > > b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > > index 4be1ff3a42bb..2e6ad4591ebf 100644
> > > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > > @@ -572,8 +572,13 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct 
> > > dw_mipi_dsi *dsi, u32 hdr_val)
> > >  static int dw_mipi_dsi_dcs_short_write(struct dw_mipi_dsi *dsi,
> > >  const struct mipi_dsi_msg *msg)
> > >  {
> > > - const u16 *tx_buf = msg->tx_buf;
> > > - u32 val = GEN_HDATA(*tx_buf) | GEN_HTYPE(msg->type);
> > > + const u8 *tx_buf = msg->tx_buf;
> > > + u32 val = GEN_HTYPE(msg->type);
> > > +
> > > + if (msg->tx_len > 0)
> > > + val |= GEN_HDATA(tx_buf[0]);
> > > + if (msg->tx_len > 1)
> > > + val |= GEN_HDATA(tx_buf[1] << 8);  
> > 
> > You should probably update the mask inside GEN_HDATA to mask off 8 bits 
> > instead of
> > 16.
> 
> Won't that mask off the data written by "tx_buf[1] << 8"?

I would move the shift outside the mask, ie:

val |= GEN_HDATA(tx_buf[1]) << 8;

Sean

> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 v3 10/24] drm/rockchip: dw-mipi-dsi: don't assume buffer is aligned

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:30PM +, John Keeping wrote:
> By dereferencing the MIPI command buffer as a u32* we rely on it being
> correctly aligned on ARM, but this may not be the case.  Copy it into a
> stack variable that will be correctly aligned.
> 
> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 03fc096fe1bd..ddbc037e7ced 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -607,10 +607,10 @@ static int dw_mipi_dsi_dcs_short_write(struct 
> dw_mipi_dsi *dsi,
>  static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi *dsi,
> const struct mipi_dsi_msg *msg)
>  {
> - const u32 *tx_buf = msg->tx_buf;
> - int len = msg->tx_len, pld_data_bytes = sizeof(*tx_buf), ret;
> + const u8 *tx_buf = msg->tx_buf;
> + int len = msg->tx_len, pld_data_bytes = sizeof(u32), ret;
>   u32 hdr_val = GEN_HDATA(msg->tx_len) | GEN_HTYPE(msg->type);
> - u32 remainder = 0;
> + u32 remainder;
>   u32 val;
>  
>   if (msg->tx_len < 3) {
> @@ -621,12 +621,14 @@ static int dw_mipi_dsi_dcs_long_write(struct 
> dw_mipi_dsi *dsi,
>  
>   while (DIV_ROUND_UP(len, pld_data_bytes)) {
>   if (len < pld_data_bytes) {
> + remainder = 0;
>   memcpy(, tx_buf, len);
>   dsi_write(dsi, DSI_GEN_PLD_DATA, remainder);
>   len = 0;
>   } else {
> - dsi_write(dsi, DSI_GEN_PLD_DATA, *tx_buf);
> - tx_buf++;
> + memcpy(, tx_buf, pld_data_bytes);
> + dsi_write(dsi, DSI_GEN_PLD_DATA, remainder);
> + tx_buf += pld_data_bytes;
>   len -= pld_data_bytes;
>   }


You can clean this up further by removing a couple of the locals, the
conditional and the division:

while(len < msg->tx_len) {
size_t to_write = MIN(msg->tx_len - len, sizeof(remainder));

memcpy(, msg->tx_buf + len, to_write);
dsi_write(dsi, DSI_GEN_PLD_DATA, remainder);
len += to_write;

...
}

Sean


>  
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/nouveau: gk20a: Turn instmem lock into mutex

[Thierry Reding]

From: Thierry Reding 

The gk20a implementation of instance memory uses vmap()/vunmap() to map
memory regions into the kernel's virtual address space. These functions
may sleep, so protecting them by a spin lock is not safe. This triggers
a warning if the DEBUG_ATOMIC_SLEEP Kconfig option is enabled. Fix this
by using a mutex instead.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
index a6a7fa0d7679..7f5244d57d2f 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
@@ -94,7 +94,7 @@ struct gk20a_instmem {
struct nvkm_instmem base;
 
/* protects vaddr_* and gk20a_instobj::vaddr* */
-   spinlock_t lock;
+   struct mutex lock;
 
/* CPU mappings LRU */
unsigned int vaddr_use;
@@ -184,11 +184,10 @@ gk20a_instobj_acquire_iommu(struct nvkm_memory *memory)
struct gk20a_instmem *imem = node->base.imem;
struct nvkm_ltc *ltc = imem->base.subdev.device->ltc;
const u64 size = nvkm_memory_size(memory);
-   unsigned long flags;
 
nvkm_ltc_flush(ltc);
 
-   spin_lock_irqsave(>lock, flags);
+   mutex_lock(>lock);
 
if (node->base.vaddr) {
if (!node->use_cpt) {
@@ -216,7 +215,7 @@ gk20a_instobj_acquire_iommu(struct nvkm_memory *memory)
 
 out:
node->use_cpt++;
-   spin_unlock_irqrestore(>lock, flags);
+   mutex_unlock(>lock);
 
return node->base.vaddr;
 }
@@ -239,9 +238,8 @@ gk20a_instobj_release_iommu(struct nvkm_memory *memory)
struct gk20a_instobj_iommu *node = gk20a_instobj_iommu(memory);
struct gk20a_instmem *imem = node->base.imem;
struct nvkm_ltc *ltc = imem->base.subdev.device->ltc;
-   unsigned long flags;
 
-   spin_lock_irqsave(>lock, flags);
+   mutex_lock(>lock);
 
/* we should at least have one user to release... */
if (WARN_ON(node->use_cpt == 0))
@@ -252,7 +250,7 @@ gk20a_instobj_release_iommu(struct nvkm_memory *memory)
list_add_tail(>vaddr_node, >vaddr_lru);
 
 out:
-   spin_unlock_irqrestore(>lock, flags);
+   mutex_unlock(>lock);
 
wmb();
nvkm_ltc_invalidate(ltc);
@@ -306,19 +304,18 @@ gk20a_instobj_dtor_iommu(struct nvkm_memory *memory)
struct gk20a_instmem *imem = node->base.imem;
struct device *dev = imem->base.subdev.device->dev;
struct nvkm_mm_node *r;
-   unsigned long flags;
int i;
 
if (unlikely(list_empty(>base.mem.regions)))
goto out;
 
-   spin_lock_irqsave(>lock, flags);
+   mutex_lock(>lock);
 
/* vaddr has already been recycled */
if (node->base.vaddr)
gk20a_instobj_iommu_recycle_vaddr(node);
 
-   spin_unlock_irqrestore(>lock, flags);
+   mutex_unlock(>lock);
 
r = list_first_entry(>base.mem.regions, struct nvkm_mm_node,
 rl_entry);
@@ -580,7 +577,7 @@ gk20a_instmem_new(struct nvkm_device *device, int index,
if (!(imem = kzalloc(sizeof(*imem), GFP_KERNEL)))
return -ENOMEM;
nvkm_instmem_ctor(_instmem, device, index, >base);
-   spin_lock_init(>lock);
+   mutex_init(>lock);
*pimem = >base;
 
/* do not allow more than 1MB of CPU-mapped instmem */
-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[git pull] vmwgfx-next

[Sinclair Yeh]

Hi Dave,

This is to address what we've discussed, moving some of the minor changes
into a drm-next request.

-

The following changes since commit f0493e653f9679114d1dfd54ab88b54ce95576e1:

  drm/mgag200: Added support for the new device G200eH3 (2017-01-23 11:57:08 
+1000)

are available in the git repository at:

  git://people.freedesktop.org/~syeh/repos_linux drm-vmwgfx-next

for you to fetch changes up to 5d25fde23b3176c7f94d2a992cb9762707d7c2a0:

  drm/vmwgfx: Use kmemdup instead of kmalloc and memcpy (2017-01-26 21:26:17 
-0800)


Shyam Saini (1):
  drm/vmwgfx: Use kmemdup instead of kmalloc and memcpy

Sinclair Yeh (1):
  drm/vmwgfx: Fix depth input into drm_mode_legacy_fb_format

Thomas Hellstrom (4):
  drm/vmwgfx: Clear uninitialized fields of a parameter
  drm/vmwgfx: Annotate ignored return values
  drm/vmwgfx: Clear an uninitialized struct member
  drm/vmwgfx: Fix a potential integer overflow

 drivers/gpu/drm/vmwgfx/device_include/svga3d_surfacedefs.h | 2 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 3 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c| 4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_mob.c| 7 +++
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   | 4 ++--
 7 files changed, 12 insertions(+), 12 deletions(-)


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] MAINTAINERS: add dma-fence* files to Sync File maintainership

[Gustavo Padovan]

2017-01-30 Sumit Semwal :

> Hi Gustavo, Daniel,
> 
> On 30 January 2017 at 14:07, Daniel Vetter  wrote:
> 
> > On Fri, Jan 27, 2017 at 03:54:44PM -0200, Gustavo Padovan wrote:
> > > From: Gustavo Padovan 
> > >
> > > As Sync File is highly dependent on dma-fence* tracks it
> > > under SYNC FILE_FRAMEWORK as well.
> > >
> > > Signed-off-by: Gustavo Padovan 
> >
> > Acked-by: Daniel Vetter 
> >
> > Of course, feel free to add my
> Acked-by: Sumit Semwal 

applied to drm-misc-next. Thanks.

Gustavo

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/msm: remove qcom,gpu-pwrlevels bindings

[Eric Anholt]

Rob Clark  writes:

> On Mon, Jan 30, 2017 at 1:21 PM, Eric Anholt  wrote:
>> Rob Clark  writes:
>>
>>> The plan is to use the OPP bindings.  For now, remove the documentation
>>> for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
>>> clock if the node is not present.
>>>
>>> Note that no upstream dtb use this node.  For now we keep compatibility
>>> with this node to avoid breaking compatibility with downstream android
>>> dt files.
>>>
>>> Signed-off-by: Rob Clark 
>>
>> Will we need the bus frequency knobs that I see in the old pwrlevels
>> node?  If so, what would the plan be for doing that within OPP?
>
> So, that I think is one of the open questions.  Jordan knows this
> stuff a lot better than I, but my understanding is that bus and clk
> scale *basically* independently, except that a given gpu clk we want a
> different minimum bus clk.
>
> (I'm not sure if that is a functional requirement, or just what qcom
> arrived at after performance tuning..)
>
> There is some work ongoing to get some sort of upstream bus scaling
> scaling, although I'm not really sure yet what the bindings for this
> would look like.
>
> So basically short answer is "I don't know.. there are too many open
> questions".  Maybe in the end we re-introduce qcom,gpu-pwrlevels.  But
> I think for now the approach of not documenting it and have safe/slow
> clk fallback in the driver is a reasonable way to move forward with
> getting some basic gpu nodes into upstream dts files.

Yeah, letting the upstream DT bind without the custom OPP stuff for now
seems like progress.  If we find that the safe fast freq is too high
then we can drop it down later, and that would just put more pressure on
getting the OPP work finished.

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/4] drm/msm: drop qcom,chipid

[Eric Anholt]

Rob Clark  writes:

> On Mon, Jan 30, 2017 at 1:09 PM, Eric Anholt  wrote:
>> Rob Clark  writes:
>>
>>> The original way we determined the gpu version was based on downstream
>>> bindings from android kernel.  A cleaner way is to get the version from
>>> the compatible string.
>>>
>>> Note that no upstream dtb uses these bindings.  But the code still
>>> supports falling back to the legacy bindings (with a warning), so that
>>> we are still compatible with the gpu dt node from android device
>>> kernels.
>>
>> The gpulist[] seems pretty short, like you could just have 7 compatible
>> strings in dt_match[] and point them directly at corresponding the
>> gpulist[] entry.  Or are there lots of patch levels, with the same
>> struct adreno_info values?
>
> The list currently is rather incomplete (which may or may not matter,
> I guess it comes down to how many different phones/tablets out there
> people try to get an upstream kernel working on).  And it has those
> ANY_ID wildcard entries.
>
> A full list of all the gpu's including all their patchlevels would be
> quite long.
>
> We might end up wanting to split the quirks out differently, since
> those tend to apply to specific patchlevel's.. I had to change the
> existing entry for a530 from a530.* to a530.2 for this reason.  But
> that won't effect the bindings so that is an implementation detail we
> can worry about later.

I think that using the of_match_device() mechanism from the specific
strings listed as the compatible would make more sense than this string
parsing.  You have to write a gpulist[] entry anyway for the module to
load.  So that means adding about 7 values today to the compatible
string dt_match, and the code to use of_match_device() to get your
struct adreno_info.  Then the current version number lookup loop would
be just for the old DT compatibility and there's no string parsing.

However, it's your driver.  The code seems correct, and using specific
compatible strings is an obviously good step.

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/2] drm: Add support for tiny LCD displays

[Noralf Trønnes]

Den 30.01.2017 19.06, skrev Daniel Vetter:

On Mon, Jan 30, 2017 at 04:03:53PM +0100, Noralf Trønnes wrote:

Den 30.01.2017 09.44, skrev Daniel Vetter:

Hi Noralf,

On Fri, Jan 27, 2017 at 08:56:29PM +0100, Noralf Trønnes wrote:

This is an attempt at providing a DRM version of drivers/staging/fbtft.

The tinydrm library provides a very simplified view of DRM in particular
for tiny displays that has onboard video memory and is connected through
a slow bus like SPI/I2C.

Only core patches this time.


Noralf.


Changes since version 1:
- Add tinydrm.rst
- Set tdev->fbdev_cma=NULL on unregister (lastclose is called after that).





Either way, I think this all looks good, pls submit a pull request to Dave
with these two patches as soon as latest drm-misc has landed (I'll send a
pull request for that later today).

I'm confused, I thought you wanted the core patches through drm-misc
and the rest as a pull request to Dave.

This is what you said:

 Looks all pretty. A bunch of ideas below, but all optional. For merging
I
 think simplest to first get the core patches in through drm-misc, and
then
 you can submit a pull request to Dave for tinydrm+backends (just needs
an
 ack for the dt parts from dt maintainers), including MAINTAINERS entry.
 Ack from my side.

Ah, I thought we already have all the prep patches you need merged into
drm-misc.  Still the same plan.


Oh, sorry, I mixed up core drm with core tinydrm.
Yes, the core drm patches are in. I'll send a pull request for tinydrm
when I get an ack from the DT maintainer.

It all makes sense now :-)

Noralf.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/msm: remove qcom,gpu-pwrlevels bindings

[Rob Clark]

On Mon, Jan 30, 2017 at 1:21 PM, Eric Anholt  wrote:
> Rob Clark  writes:
>
>> The plan is to use the OPP bindings.  For now, remove the documentation
>> for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
>> clock if the node is not present.
>>
>> Note that no upstream dtb use this node.  For now we keep compatibility
>> with this node to avoid breaking compatibility with downstream android
>> dt files.
>>
>> Signed-off-by: Rob Clark 
>
> Will we need the bus frequency knobs that I see in the old pwrlevels
> node?  If so, what would the plan be for doing that within OPP?

So, that I think is one of the open questions.  Jordan knows this
stuff a lot better than I, but my understanding is that bus and clk
scale *basically* independently, except that a given gpu clk we want a
different minimum bus clk.

(I'm not sure if that is a functional requirement, or just what qcom
arrived at after performance tuning..)

There is some work ongoing to get some sort of upstream bus scaling
scaling, although I'm not really sure yet what the bindings for this
would look like.

So basically short answer is "I don't know.. there are too many open
questions".  Maybe in the end we re-introduce qcom,gpu-pwrlevels.  But
I think for now the approach of not documenting it and have safe/slow
clk fallback in the driver is a reasonable way to move forward with
getting some basic gpu nodes into upstream dts files.

BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/msm: drop _clk suffix from clk names

[Rob Clark]

On Mon, Jan 30, 2017 at 1:15 PM, Eric Anholt  wrote:
> Rob Clark  writes:
>
>> Suggested by Rob Herring.  We still support the old names for
>> compatibility with downstream android dt files.
>>
>> Cc: Rob Herring 
>> Signed-off-by: Rob Clark 
>
> Huh, I don't think I would have cleaned up DT bindings in exchange for
> adding driver code like this.  But the code seems correct, so other than
> one optional suggestion:
>
> Reviewed-by: Eric Anholt 
>
>> +struct clk *msm_clk_get(struct platform_device *pdev, const char *name)
>> +{
>> + struct clk *clk;
>> + char name2[32];
>> +
>> + clk = devm_clk_get(>dev, name);
>> + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
>> + return clk;
>> +
>> + snprintf(name2, sizeof(name2), "%s_clk", name);
>> +
>> + clk = devm_clk_get(>dev, name2);
>> + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
>> + dev_warn(>dev, "Using legacy clk name binding.  Use "
>> + "\"%s\" instead of \"%s\"\n", name, name2);
>
> Drop the second "|| PTR_ERR(clk)" case, so that you only get one warning
> printed at boot if deferring happens?

oh, good point.. I've fixed that up locally.  I don't think we'd
actually hit this case currently for gpu clks, since for gpu this
codepath happens on first open() of the device file.  But I should
mention I have a slighly sneaky ulterior motive, which is that we
could use the same cleanup for display related clks (which do
currently upstream use the _clk suffix).

BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/msm: remove qcom,gpu-pwrlevels bindings

[Eric Anholt]

Rob Clark  writes:

> The plan is to use the OPP bindings.  For now, remove the documentation
> for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
> clock if the node is not present.
>
> Note that no upstream dtb use this node.  For now we keep compatibility
> with this node to avoid breaking compatibility with downstream android
> dt files.
>
> Signed-off-by: Rob Clark 

Will we need the bus frequency knobs that I see in the old pwrlevels
node?  If so, what would the plan be for doing that within OPP?


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 09/24] drm/rockchip: dw-mipi-dsi: only request HS clock when required

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:29PM +, John Keeping wrote:
> Requesting the HS clock from the PHY before we initialize it causes an
> invalid signal to be sent out since the input clock is not yet
> configured.  The PHY databook suggests only asserting this signal when
> performing HS transfers, so let's do that.
> 

Reviewed-by: Sean Paul 

> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 15d33c3c8cb7..03fc096fe1bd 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -545,13 +545,15 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host 
> *host,
>  static void dw_mipi_message_config(struct dw_mipi_dsi *dsi,
>  const struct mipi_dsi_msg *msg)
>  {
> + bool lpm = msg->flags & MIPI_DSI_MSG_USE_LPM;
>   u32 val = 0;
>  
>   if (msg->flags & MIPI_DSI_MSG_REQ_ACK)
>   val |= EN_ACK_RQST;
> - if (msg->flags & MIPI_DSI_MSG_USE_LPM)
> + if (lpm)
>   val |= CMD_MODE_ALL_LP;
>  
> + dsi_write(dsi, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
>   dsi_write(dsi, DSI_CMD_MODE_CFG, val);
>  }
>  
> @@ -693,6 +695,7 @@ static void dw_mipi_dsi_set_mode(struct dw_mipi_dsi *dsi,
>   dsi_write(dsi, DSI_PWR_UP, RESET);
>   dsi_write(dsi, DSI_MODE_CFG, ENABLE_VIDEO_MODE);
>   dw_mipi_dsi_video_mode_config(dsi);
> + dsi_write(dsi, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS);
>   dsi_write(dsi, DSI_PWR_UP, POWERUP);
>   }
>  }
> @@ -710,7 +713,6 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
> | PHY_RSTZ | PHY_SHUTDOWNZ);
>   dsi_write(dsi, DSI_CLKMGR_CFG, TO_CLK_DIVIDSION(10) |
> TX_ESC_CLK_DIVIDSION(7));
> - dsi_write(dsi, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS);
>  }
>  
>  static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 v3 08/24] drm/rockchip: dw-mipi-dsi: respect message flags

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:28PM +, John Keeping wrote:
> Instead of always sending commands in LP mode, respect the
> MIPI_DSI_MSG_USE_LPM flag to decide how to send each message.  Also
> request acks if MIPI_DSI_MSG_REQ_ACK is set.
> 

Reviewed-by: Sean Paul 

> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 92dbc3e56603..15d33c3c8cb7 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -542,6 +542,19 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host 
> *host,
>   return 0;
>  }
>  
> +static void dw_mipi_message_config(struct dw_mipi_dsi *dsi,
> +const struct mipi_dsi_msg *msg)
> +{
> + u32 val = 0;
> +
> + if (msg->flags & MIPI_DSI_MSG_REQ_ACK)
> + val |= EN_ACK_RQST;
> + if (msg->flags & MIPI_DSI_MSG_USE_LPM)
> + val |= CMD_MODE_ALL_LP;
> +
> + dsi_write(dsi, DSI_CMD_MODE_CFG, val);
> +}
> +
>  static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 
> hdr_val)
>  {
>   int ret;
> @@ -634,6 +647,8 @@ static ssize_t dw_mipi_dsi_host_transfer(struct 
> mipi_dsi_host *host,
>   struct dw_mipi_dsi *dsi = host_to_dsi(host);
>   int ret;
>  
> + dw_mipi_message_config(dsi, msg);
> +
>   switch (msg->type) {
>   case MIPI_DSI_DCS_SHORT_WRITE:
>   case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
> @@ -745,7 +760,6 @@ static void dw_mipi_dsi_command_mode_config(struct 
> dw_mipi_dsi *dsi)
>  {
>   dsi_write(dsi, DSI_TO_CNT_CFG, HSTX_TO_CNT(1000) | LPRX_TO_CNT(1000));
>   dsi_write(dsi, DSI_BTA_TO_CNT, 0xd00);
> - dsi_write(dsi, DSI_CMD_MODE_CFG, CMD_MODE_ALL_LP);
>   dsi_write(dsi, DSI_MODE_CFG, ENABLE_CMD_MODE);
>  }
>  
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 2/4] drm/msm: drop qcom,chipid

[Rob Clark]

On Mon, Jan 30, 2017 at 1:09 PM, Eric Anholt  wrote:
> Rob Clark  writes:
>
>> The original way we determined the gpu version was based on downstream
>> bindings from android kernel.  A cleaner way is to get the version from
>> the compatible string.
>>
>> Note that no upstream dtb uses these bindings.  But the code still
>> supports falling back to the legacy bindings (with a warning), so that
>> we are still compatible with the gpu dt node from android device
>> kernels.
>
> The gpulist[] seems pretty short, like you could just have 7 compatible
> strings in dt_match[] and point them directly at corresponding the
> gpulist[] entry.  Or are there lots of patch levels, with the same
> struct adreno_info values?

The list currently is rather incomplete (which may or may not matter,
I guess it comes down to how many different phones/tablets out there
people try to get an upstream kernel working on).  And it has those
ANY_ID wildcard entries.

A full list of all the gpu's including all their patchlevels would be
quite long.

We might end up wanting to split the quirks out differently, since
those tend to apply to specific patchlevel's.. I had to change the
existing entry for a530 from a530.* to a530.2 for this reason.  But
that won't effect the bindings so that is an implementation detail we
can worry about later.

BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 06/24] drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf

[John Keeping]

On Mon, 30 Jan 2017 13:01:46 -0500, Sean Paul wrote:

> On Sun, Jan 29, 2017 at 01:24:26PM +, John Keeping wrote:
> > As a side-effect of this, encode the endianness explicitly rather than
> > casting a u16.
> > 
> > Signed-off-by: John Keeping 
> > Reviewed-by: Chris Zhong 
> > ---
> > v3:
> > - Add Chris' Reviewed-by
> > Unchanged in v2
> > 
> >  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 9 +++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> > b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > index 4be1ff3a42bb..2e6ad4591ebf 100644
> > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > @@ -572,8 +572,13 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct 
> > dw_mipi_dsi *dsi, u32 hdr_val)
> >  static int dw_mipi_dsi_dcs_short_write(struct dw_mipi_dsi *dsi,
> >const struct mipi_dsi_msg *msg)
> >  {
> > -   const u16 *tx_buf = msg->tx_buf;
> > -   u32 val = GEN_HDATA(*tx_buf) | GEN_HTYPE(msg->type);
> > +   const u8 *tx_buf = msg->tx_buf;
> > +   u32 val = GEN_HTYPE(msg->type);
> > +
> > +   if (msg->tx_len > 0)
> > +   val |= GEN_HDATA(tx_buf[0]);
> > +   if (msg->tx_len > 1)
> > +   val |= GEN_HDATA(tx_buf[1] << 8);  
> 
> You should probably update the mask inside GEN_HDATA to mask off 8 bits 
> instead of
> 16.

Won't that mask off the data written by "tx_buf[1] << 8"?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/msm: drop _clk suffix from clk names

[Eric Anholt]

Rob Clark  writes:

> Suggested by Rob Herring.  We still support the old names for
> compatibility with downstream android dt files.
>
> Cc: Rob Herring 
> Signed-off-by: Rob Clark 

Huh, I don't think I would have cleaned up DT bindings in exchange for
adding driver code like this.  But the code seems correct, so other than
one optional suggestion:

Reviewed-by: Eric Anholt 

> +struct clk *msm_clk_get(struct platform_device *pdev, const char *name)
> +{
> + struct clk *clk;
> + char name2[32];
> +
> + clk = devm_clk_get(>dev, name);
> + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
> + return clk;
> +
> + snprintf(name2, sizeof(name2), "%s_clk", name);
> +
> + clk = devm_clk_get(>dev, name2);
> + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
> + dev_warn(>dev, "Using legacy clk name binding.  Use "
> + "\"%s\" instead of \"%s\"\n", name, name2);

Drop the second "|| PTR_ERR(clk)" case, so that you only get one warning
printed at boot if deferring happens?


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 24/24] drm/rockchip: dw-mipi-dsi: support read commands

[John Keeping]

On Mon, 30 Jan 2017 10:26:11 -0500, Sean Paul wrote:

> On Sun, Jan 29, 2017 at 01:24:44PM +, John Keeping wrote:
> > I haven't found any method for getting the length of a response, so this
> > just uses the requested rx_len
> > 
> > Signed-off-by: John Keeping 
> > ---
> > v3:
> > - Fix checkpatch warnings
> > Unchanged in v2
> > 
> >  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 56 
> > ++
> >  1 file changed, 56 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> > b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > index cf3ca6b0cbdb..cc58ada75425 100644
> > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > @@ -678,6 +678,56 @@ static int dw_mipi_dsi_dcs_long_write(struct 
> > dw_mipi_dsi *dsi,
> > return dw_mipi_dsi_gen_pkt_hdr_write(dsi, hdr_val);
> >  }
> >  
> > +static int dw_mipi_dsi_dcs_read(struct dw_mipi_dsi *dsi,
> > +   const struct mipi_dsi_msg *msg)
> > +{
> > +   const u8 *tx_buf = msg->tx_buf;
> > +   u8 *rx_buf = msg->rx_buf;
> > +   size_t i;
> > +   int ret, val;
> > +
> > +   dsi_write(dsi, DSI_PCKHDL_CFG, EN_CRC_RX | EN_ECC_RX | EN_BTA);
> > +   dsi_write(dsi, DSI_GEN_HDR,
> > + GEN_HDATA(tx_buf[0]) | GEN_HTYPE(msg->type));
> > +
> > +   ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> > +val, !(val & GEN_RD_CMD_BUSY), 1000,
> > +CMD_PKT_STATUS_TIMEOUT_US);
> > +   if (ret < 0) {
> > +   dev_err(dsi->dev, "failed to read command response\n");
> > +   return ret;
> > +   }
> > +
> > +   for (i = 0; i < msg->rx_len;) {
> > +   u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
> > +
> > +   while (i < msg->rx_len) {
> > +   rx_buf[i] = pld & 0xff;
> > +   pld >>= 8;
> > +   i++;
> > +   }
> > +   }  
> 
> AFAICT, the outer for loop just initializes i and ensures msg->rx_len is
> non-zero? 
> 
> I think the following would be easier to read (and safe against the case where
> msg->rx_len > sizeof(pld) (even though this shouldn't happen according to DCS
> spec)).
> 
> if (msg->rx_len > 0) {
> u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
> memcpy(rx_buf, , MIN(msg->rx_len, sizeof(pld));
> }

I think the intent was to handle rx_len > 4, but the patch is obvously
completely broken regarding that.  As far as I can tell, rx_len is
limited by the maximum return packet size which can be any value up to
the maximum size of a long packet, so we may need to read from the FIFO
multiple times.

The loop should be something like this:

for (i = 0; i < msg->rx_len;) {
u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
int j;

for (j = 0; j < 4 && i < msg->rx_len; i++, j++) {
rx_buf[i] = pld & 0xff;
pld >>= 8;
}
}

I have successfully read 5 bytes from a DSI display using this code, but
I'm tempted to just drop this patch since I only used it for debugging
while bringing up a new panel.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm/msm: drop quirks binding

[Eric Anholt]

Rob Clark  writes:

> This was never documented or used in upstream dtb.  It is used by
> downstream bindings from android device kernels.  But the quirks are
> a property of the gpu revision, and as such are redundant to be listed
> separately in dt.  Instead, move the quirks to the device table.
>
> Signed-off-by: Rob Clark 
> ---
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c  |  4 ++--
>  drivers/gpu/drm/msm/adreno/adreno_device.c | 18 --
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c|  1 -
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h|  4 +---
>  4 files changed, 7 insertions(+), 20 deletions(-)

Nice cleanup!

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/4] drm/msm: drop qcom,chipid

[Eric Anholt]

Rob Clark  writes:

> The original way we determined the gpu version was based on downstream
> bindings from android kernel.  A cleaner way is to get the version from
> the compatible string.
>
> Note that no upstream dtb uses these bindings.  But the code still
> supports falling back to the legacy bindings (with a warning), so that
> we are still compatible with the gpu dt node from android device
> kernels.

The gpulist[] seems pretty short, like you could just have 7 compatible
strings in dt_match[] and point them directly at corresponding the
gpulist[] entry.  Or are there lots of patch levels, with the same
struct adreno_info values?


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/2] drm: Add support for tiny LCD displays

[Daniel Vetter]

On Mon, Jan 30, 2017 at 04:03:53PM +0100, Noralf Trønnes wrote:
> 
> Den 30.01.2017 09.44, skrev Daniel Vetter:
> > Hi Noralf,
> > 
> > On Fri, Jan 27, 2017 at 08:56:29PM +0100, Noralf Trønnes wrote:
> > > This is an attempt at providing a DRM version of drivers/staging/fbtft.
> > > 
> > > The tinydrm library provides a very simplified view of DRM in particular
> > > for tiny displays that has onboard video memory and is connected through
> > > a slow bus like SPI/I2C.
> > > 
> > > Only core patches this time.
> > > 
> > > 
> > > Noralf.
> > > 
> > > 
> > > Changes since version 1:
> > > - Add tinydrm.rst
> > > - Set tdev->fbdev_cma=NULL on unregister (lastclose is called after that).
> > Hm, this sounds like a buglet in the drm framework ... how do we call
> > lastclose when the driver is disappearing? I do see a drm_lastclose call
> > at the beginning of drm_dev_unregister (which we might want to remove for
> > KMS drivers, it doesn't make much sense imo), but that shouldn't result in
> > troubles.
> 
> Ah, I see, I'm tearing down fbdev before unregistering drm.
> That should be reversed.
> 
> static void tinydrm_unregister(struct tinydrm_device *tdev)
> {
> drm_crtc_force_disable_all(tdev->drm);
> 
> if (tdev->fbdev_cma) {
> drm_fbdev_cma_fini(tdev->fbdev_cma);
> tdev->fbdev_cma = NULL;
> }
> 
> drm_dev_unregister(tdev->drm);
> }
> 
> > > - Remove some DRM_DEBUG*()
> > > - Write-combined memory has uncached reads, so speed up by 
> > > copying/buffering
> > >one pixel line before conversion.
> > Hm, why are you using write-combining memory? Or is that needed so that
> > you can (if available) use hw spi engines?
> 
> That comes with using the CMA helper:
> drm_fbdev_cma_create_with_funcs() -> drm_gem_cma_create() -> dma_alloc_wc()
> 
> Here's a comment I have added in the code for why I set the DMA mask on
> the SPI device and why it will work:
> 
> /*
>  * Even though it's not the SPI device that does DMA (the master does),
>  * the dma mask is necessary for the dma_alloc_wc() in
>  * drm_gem_cma_create(). The dma_addr returned will be a physical
>  * adddress which might be different from the bus address, but this is
>  * not a problem since the address will not be used.
>  * The virtual address is used in the transfer and the SPI core
>  * re-maps it on the SPI master device using the DMA streaming API
>  * (spi_map_buf()).
>  */
> 
> > Either way, I think this all looks good, pls submit a pull request to Dave
> > with these two patches as soon as latest drm-misc has landed (I'll send a
> > pull request for that later today).
> 
> I'm confused, I thought you wanted the core patches through drm-misc
> and the rest as a pull request to Dave.
> 
> This is what you said:
> 
> Looks all pretty. A bunch of ideas below, but all optional. For merging
> I
> think simplest to first get the core patches in through drm-misc, and
> then
> you can submit a pull request to Dave for tinydrm+backends (just needs
> an
> ack for the dt parts from dt maintainers), including MAINTAINERS entry.
> Ack from my side.

Ah, I thought we already have all the prep patches you need merged into
drm-misc.  Still the same plan.

> > Another one: Do you want to maintain tinydrm as part of the drm-misc
> > group, i.e. want commit rights there? That would also help a bit with
> > pushing all your great drm refactoring patches through the machinery ...
> 
> My goal is to port staging/fbtft to drm. Whether or not I will do work
> in drm core (refactoring) besides the tinydrm drivers when that is
> accomplished, I don't know. Working on drm as a hobbyist is very
> difficult (for me at least) because it is very complex, it changes all
> the time and on top of that it has a high volume mailinglist.
> It takes effort to keep up.
> 
> So I will probably end up doing only tinydrm maintanence.
> As for how that is best done, I don't know. Once I have covered a 3-4
> controller types, a new driver is just a copy of a similar one with a
> different register initialization. This means that it's easy to review
> patches. They will all look the same, more or less.
> So for me it's ofc best if I can review the patches and then commit
> them myself. As for my own patches, if I need that tit for tat
> reviewing to get them in, it will be difficult for me to review other
> drivers because I have no idea how a GPU operates, and to keep up with
> drm best pratices will be next to impossible for me.

I think you're slightly understating your knowledge about the display side
of drm here a bit :-) We're (for now) also only talking about getting
small display drivers into drm-misc.

> If the Device Tree guys allows me to add fbtft support to tinydrm, then
> there won't be much activity once the fbtft drivers have been ported
> over. The uncertainty stems from the fact that the fbtft drivers are
> written as controller drivers, but they contain panel specific 

Re: [PATCH v3 07/24] drm/rockchip: dw-mipi-dsi: include bad value in error message

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:27PM +, John Keeping wrote:
> As an aid to debugging.

Reviewed-by: Sean Paul 

> 
> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 2e6ad4591ebf..92dbc3e56603 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -644,7 +644,8 @@ static ssize_t dw_mipi_dsi_host_transfer(struct 
> mipi_dsi_host *host,
>   ret = dw_mipi_dsi_dcs_long_write(dsi, msg);
>   break;
>   default:
> - dev_err(dsi->dev, "unsupported message type\n");
> + dev_err(dsi->dev, "unsupported message type 0x%02x\n",
> + msg->type);
>   ret = -EINVAL;
>   }
>  
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 v3 06/24] drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:26PM +, John Keeping wrote:
> As a side-effect of this, encode the endianness explicitly rather than
> casting a u16.
> 
> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 4be1ff3a42bb..2e6ad4591ebf 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -572,8 +572,13 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct 
> dw_mipi_dsi *dsi, u32 hdr_val)
>  static int dw_mipi_dsi_dcs_short_write(struct dw_mipi_dsi *dsi,
>  const struct mipi_dsi_msg *msg)
>  {
> - const u16 *tx_buf = msg->tx_buf;
> - u32 val = GEN_HDATA(*tx_buf) | GEN_HTYPE(msg->type);
> + const u8 *tx_buf = msg->tx_buf;
> + u32 val = GEN_HTYPE(msg->type);
> +
> + if (msg->tx_len > 0)
> + val |= GEN_HDATA(tx_buf[0]);
> + if (msg->tx_len > 1)
> + val |= GEN_HDATA(tx_buf[1] << 8);

You should probably update the mask inside GEN_HDATA to mask off 8 bits instead 
of
16.

Sean

>  
>   if (msg->tx_len > 2) {
>   dev_err(dsi->dev, "too long tx buf length %zu for short 
> write\n",
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/atomic: Fix double kfree on crtc_state->event

[Daniel Vetter]

This is a bug Maarten reported, with the following slab debug backtrace:

[IGT] kms_rotation_crc: starting subtest primary-rotation-180
=
BUG kmalloc-128 (Tainted: G U ): Object already free
-

Disabling lock debugging due to kernel taint
INFO: Allocated in drm_atomic_helper_setup_commit+0x285/0x2f0 [drm_kms_helper] 
age=0 cpu=3 pid=1529
 ___slab_alloc+0x308/0x3b0
 __slab_alloc+0xd/0x20
 kmem_cache_alloc_trace+0x92/0x1c0
 drm_atomic_helper_setup_commit+0x285/0x2f0 [drm_kms_helper]
 intel_atomic_commit+0x35/0x4f0 [i915]
 drm_atomic_commit+0x46/0x50 [drm]
 drm_mode_atomic_ioctl+0x7d4/0xab0 [drm]
 drm_ioctl+0x2b3/0x490 [drm]
 do_vfs_ioctl+0x69c/0x700
 SyS_ioctl+0x4e/0x80
 entry_SYSCALL_64_fastpath+0x13/0x94
INFO: Freed in drm_event_cancel_free+0xa3/0xb0 [drm] age=0 cpu=3 pid=1529
 __slab_free+0x48/0x2e0
 kfree+0x159/0x1a0
 drm_event_cancel_free+0xa3/0xb0 [drm]
 drm_mode_atomic_ioctl+0x86d/0xab0 [drm]
 drm_ioctl+0x2b3/0x490 [drm]
 do_vfs_ioctl+0x69c/0x700
 SyS_ioctl+0x4e/0x80
 entry_SYSCALL_64_fastpath+0x13/0x94
INFO: Slab 0xde1f0997b080 objects=17 used=2 fp=0x92fb65ec2578 
flags=0x2008101
INFO: Object 0x92fb65ec2578 @offset=1400 fp=0x92fb65ec2ae8

Redzone 92fb65ec2570: bb bb bb bb bb bb bb bb  

Object 92fb65ec2578: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 92fb65ec2588: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 92fb65ec2598: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 92fb65ec25a8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 92fb65ec25b8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 92fb65ec25c8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 92fb65ec25d8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 92fb65ec25e8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5  
kkk.
Redzone 92fb65ec25f8: bb bb bb bb bb bb bb bb  

Padding 92fb65ec2738: 5a 5a 5a 5a 5a 5a 5a 5a  

CPU: 3 PID: 180 Comm: kworker/3:2 Tainted: GBU  4.10.0-rc6-patser+ 
#5039
Hardware name:  /NUC5PPYB, BIOS 
PYBSWCEL.86A.0031.2015.0601.1712 06/01/2015
Workqueue: events intel_atomic_helper_free_state [i915]
Call Trace:
 dump_stack+0x4d/0x6d
 print_trailer+0x20c/0x220
 free_debug_processing+0x1c6/0x330
 ? drm_atomic_state_default_clear+0xf7/0x1c0 [drm]
 __slab_free+0x48/0x2e0
 ? drm_atomic_state_default_clear+0xf7/0x1c0 [drm]
 kfree+0x159/0x1a0
 drm_atomic_state_default_clear+0xf7/0x1c0 [drm]
 ? drm_atomic_state_clear+0x30/0x30 [drm]
 intel_atomic_state_clear+0xd/0x20 [i915]
 drm_atomic_state_clear+0x1a/0x30 [drm]
 __drm_atomic_state_free+0x13/0x60 [drm]
 intel_atomic_helper_free_state+0x5d/0x70 [i915]
 process_one_work+0x260/0x4a0
 worker_thread+0x2d1/0x4f0
 kthread+0x127/0x130
 ? process_one_work+0x4a0/0x4a0
 ? kthread_stop+0x120/0x120
 ret_from_fork+0x29/0x40
FIX kmalloc-128: Object at 0x92fb65ec2578 not freed

Reported-by: Maarten Lankhorst 
Cc: Maarten Lankhorst 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_atomic.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 026be94a7d15..366b4bf88206 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -159,15 +159,29 @@ void drm_atomic_state_default_clear(struct 
drm_atomic_state *state)
if (!crtc)
continue;
 
-   crtc->funcs->atomic_destroy_state(crtc,
- state->crtcs[i].state);
-
if (state->crtcs[i].commit) {
-   kfree(state->crtcs[i].commit->event);
+   /*
+* We need to make sure we don't double-free, which we
+* do by checking for state->event, implicitly since
+* kfree can handle a NULL state->event. We also need to
+* make sure we only kfree the event if it's one created
+* for internal commit tracking (and hence won't be
+* cleared by the caller, like the atomic IOCTL or a
+* legacy pageflip. This is done by checking
+* commit->event.
+*
+* This only works if everyone else sets state->event to
+* NULL when they take it away.
+*/
+   

Re: [PATCH v3 05/24] drm/rockchip: dw-mipi-dsi: fix generic packet status check

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:25PM +, John Keeping wrote:
> We want to check that both the GEN_CMD_EMPTY and GEN_PLD_W_EMPTY bits
> are set so we can't just check "val & mask" because that will be true if
> either bit is set.
> 

Reviewed-by: Sean Paul 

> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 4cbbbcb619b7..4be1ff3a42bb 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -545,7 +545,7 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host 
> *host,
>  static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 
> hdr_val)
>  {
>   int ret;
> - u32 val;
> + u32 val, mask;
>  
>   ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS,
>val, !(val & GEN_CMD_FULL), 1000,
> @@ -557,8 +557,9 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct 
> dw_mipi_dsi *dsi, u32 hdr_val)
>  
>   dsi_write(dsi, DSI_GEN_HDR, hdr_val);
>  
> + mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
>   ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS,
> -  val, val & (GEN_CMD_EMPTY | GEN_PLD_W_EMPTY),
> +  val, (val & mask) == mask,
>1000, CMD_PKT_STATUS_TIMEOUT_US);
>   if (ret < 0) {
>   dev_err(dsi->dev, "failed to write command FIFO\n");
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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: make fbdev/fbcon switchable per driver?

[Alex Williamson]

On Mon, 30 Jan 2017 09:15:50 +0100
Gerd Hoffmann  wrote:

>   Hi,
> 
> > The vgaarb code has a concept of a vga_default_device(), it's rather
> > PCI-centric, but maybe better than nothing.  This is typically the
> > first VGA class code device found with I/O and MMIO enabled.  If fbcon
> > defaulted to running on the vga_default_device(), a user could select
> > which to use by re-ordering the VM hardware.  
> 
> The qemu drivers don't register as vgaarb clients though.  Which easily
> explains why igd always wins the primary selection, no matter how you
> order your hardware.
> 
> So, should they register?  The drivers don't need access to the vga
> registers[1][2].

The VGA arbiter sets up a notifier on the PCI bus and will add any VGA
class code devices it finds.  So even if the driver doesn't
participate, it'll still be tracked and might be marked as primary.  If
a graphics driver claims a VGA device that does not depend on VGA
region access then the driver should configure the device not to claim
VGA accesses (maybe only relevant for integrated graphics - i915 gets
this wrong), and register with the arbiter to opt-out of VGA
arbitration.  Picking a "primary" can be done w/o any of that latter if
we agree on the arbiter algorithm of picking the first device with VGA
routing to it (or it can be overridden by arch/platform code).  Thanks,

Alex
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm: qxl: Drop misleading comment

[Gerd Hoffmann]

  Hi,

> drm-misc runs with the committer model, i.e. a few maintainers to do pull
> requests and backmerges, a big pile of people directly pushing patches.

[ looked at docs too meanwhile ]

Sounds good.  I guess switching over simplifies things for all of us.
We'll avoid issues like the one at hand.  Patch flow would be faster
too.  Right now I'm only doing 1-2 drm-qemu pull requests per kernel
release due to low patch volume even for all four qemu drivers combined.

> Someone wreaked the entire 01.org domain, but you can get at all the
> tooling and documentation with
> 
> git://anongit.freedesktop.org/drm-intel maintainer-tools

Hmm.  On a quick glance most of dim (except apply-patch) seems to be
more useful for maintainers which do merges etc, not so much for
committers.

I'm used to use https://github.com/stefanha/patches for qemu, and
started using it for drm-qemu too.  It makes applying patches easier.
It manages a patch database, using notmuch mail storage, and can apply
patches and patch series from the patch database.  That way I don't have
to save the patches as mbox somewhere.  The tool also picks up
[Reviewed,Tested,Acked}-by lines from replies, and it stores the message
id (but unlike dim it doesn't build a patchwork link out of it).

See bfac9f4fb4d87881375ccdc5c85d5ad59f2f115d for example.

Would that format be acceptable for drm-misc?

> And then run make in there. We're not yet clear how exactly drivers within
> drm-misc would look like (wrt which drivers and how much review and stuff
> like that), hence the RFC.

Ok.  How quickly could I start using drm-misc?  I have some pending
patches for the 4.11 merge window.  Any chance I can push them through
drm-misc-next?  Or should I better send a pull req to Dave?

cheers,
  Gerd

PS: I'm kra...@freedesktop.org

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3 PATCH 3/3] drm/amd/display: Switch to using atomic_helper for flip.

[Harry Wentland]

On 2017-01-28 09:26 PM, Andrey Grodzovsky wrote:
> Swicth to use atomic helper.
> Start using actual user's given target_vblank value for flip 
> instead of current value.
> 
> v3:
> Update for movig pflip flags to crtc_state
> 
> Change-Id: I25dae6d8c29de5d022a42aa99a18a32674b56cda
> Signed-off-by: Andrey Grodzovsky 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h   |   1 -
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 109 
> -
>  2 files changed, 19 insertions(+), 91 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 4c0a86e..3ff3c14 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -443,7 +443,6 @@ struct amdgpu_crtc {
>   enum amdgpu_interrupt_state vsync_timer_enabled;
>  
>   int otg_inst;
> - uint32_t flip_flags;
>   /* After Set Mode target will be non-NULL */
>   struct dc_target *target;
>  };
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> index a443b70..148780d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property(
>   return 0;
>  }
>  
> -
> -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc,
> - struct drm_framebuffer *fb,
> - struct drm_pending_vblank_event *event,
> - uint32_t flags)
> -{
> - struct drm_plane *plane = crtc->primary;
> - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
> - struct drm_atomic_state *state;
> - struct drm_plane_state *plane_state;
> - struct drm_crtc_state *crtc_state;
> - int ret = 0;
> -
> - state = drm_atomic_state_alloc(plane->dev);
> - if (!state)
> - return -ENOMEM;
> -
> - ret = drm_crtc_vblank_get(crtc);
> - if (ret)
> - return ret;
> -
> - state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> -retry:
> - crtc_state = drm_atomic_get_crtc_state(state, crtc);
> - if (IS_ERR(crtc_state)) {
> - ret = PTR_ERR(crtc_state);
> - goto fail;
> - }
> - crtc_state->event = event;
> -
> - plane_state = drm_atomic_get_plane_state(state, plane);
> - if (IS_ERR(plane_state)) {
> - ret = PTR_ERR(plane_state);
> - goto fail;
> - }
> -
> - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> - if (ret != 0)
> - goto fail;
> - drm_atomic_set_fb_for_plane(plane_state, fb);
> -
> - /* Make sure we don't accidentally do a full modeset. */
> - state->allow_modeset = false;
> - if (!crtc_state->active) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n",
> -  crtc->base.id);
> - ret = -EINVAL;
> - goto fail;
> - }
> - acrtc->flip_flags = flags;
> -
> - ret = drm_atomic_nonblocking_commit(state);
> -
> -fail:
> - if (ret == -EDEADLK)
> - goto backoff;
> -
> - if (ret)
> - drm_crtc_vblank_put(crtc);
> -
> - drm_atomic_state_put(state);
> -
> - return ret;
> -backoff:
> - drm_atomic_state_clear(state);
> - drm_atomic_legacy_backoff(state);
> -
> - /*
> -  * Someone might have exchanged the framebuffer while we dropped locks
> -  * in the backoff code. We need to fix up the fb refcount tracking the
> -  * core does for us.
> -  */
> - plane->old_fb = plane->fb;
> -
> - goto retry;
> -}
> -
>  /* Implemented only the options currently availible for the driver */
>  static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = {
>   .reset = drm_atomic_helper_crtc_reset,
> @@ -1145,7 +1068,7 @@ static int amdgpu_atomic_helper_page_flip(struct 
> drm_crtc *crtc,
>   .destroy = amdgpu_dm_crtc_destroy,
>   .gamma_set = amdgpu_dm_atomic_crtc_gamma_set,
>   .set_config = drm_atomic_helper_set_config,
> - .page_flip = amdgpu_atomic_helper_page_flip,
> + .page_flip_target = drm_atomic_helper_page_flip_target,
>   .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>   .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>   .atomic_set_property = dm_crtc_funcs_atomic_set_property
> @@ -1626,7 +1549,8 @@ static void clear_unrelated_fields(struct 
> drm_plane_state *state)
>  static bool page_flip_needed(
>   const struct drm_plane_state *new_state,
>   const struct drm_plane_state *old_state,
> - bool commit_surface_required)
> + bool commit_surface_required,
> + uint32_t pflip_flags)
>  {
>   struct drm_plane_state old_state_tmp;
>   struct drm_plane_state new_state_tmp;
> @@ -1679,7 +1603,7 @@ static 

Re: [PATCH 1/2] dt-bindings: Document the VC4 DSI module nodes.

[Rob Herring]

On Fri, Jan 27, 2017 at 8:41 PM, Eric Anholt  wrote:
> Rob Herring  writes:
>
>> Need to cc DT list if you want it in my queue.
>>
>> On Mon, Jan 23, 2017 at 6:38 PM, Eric Anholt  wrote:
>>> These are part of the vc4 display pipeline.
>>>
>>> Signed-off-by: Eric Anholt 
>>> ---
>>>  .../devicetree/bindings/display/brcm,bcm-vc4.txt   | 35 
>>> ++
>>>  1 file changed, 35 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt 
>>> b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
>>> index e2768703ac2b..34c7fddcea39 100644
>>> --- a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
>>> +++ b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
>>> @@ -56,6 +56,18 @@ Required properties for V3D:
>>>  - interrupts:  The interrupt number
>>>   See 
>>> bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>>
>>> +Required properties for DSI:
>>> +- compatible:  Should be "brcm,bcm2835-dsi0" or "brcm,bcm2835-dsi1"
>>
>> Are the blocks different?
>
> They are from the same lineage, but very different (old dsi0 is 1 lane,
> dsi1 is 4 lanes).  You can see how much the registers move around and
> change in the dsi->port conditional blocks in the driver code.

Okay, can you add a note here with this detail. With that,

Acked-by: Rob Herring 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/msm: remove qcom,gpu-pwrlevels bindings

[Rob Clark]

The plan is to use the OPP bindings.  For now, remove the documentation
for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
clock if the node is not present.

Note that no upstream dtb use this node.  For now we keep compatibility
with this node to avoid breaking compatibility with downstream android
dt files.

Signed-off-by: Rob Clark 
---
 Documentation/devicetree/bindings/display/msm/gpu.txt | 15 ---
 drivers/gpu/drm/msm/adreno/adreno_device.c|  6 --
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt 
b/Documentation/devicetree/bindings/display/msm/gpu.txt
index 67d0a58..747b984 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -12,12 +12,6 @@ Required properties:
   * "mem_iface_clk"
 - qcom,chipid: gpu chip-id.  Note this may become optional for future
   devices if we can reliably read the chipid from hw
-- qcom,gpu-pwrlevels: list of operating points
-  - compatible: "qcom,gpu-pwrlevels"
-  - for each qcom,gpu-pwrlevel:
-- qcom,gpu-freq: requested gpu clock speed
-- NOTE: downstream android driver defines additional parameters to
-  configure memory bandwidth scaling per OPP.
 
 Example:
 
@@ -39,14 +33,5 @@ Example:
< GFX3D_AHB_CLK>,
< MMSS_IMEM_AHB_CLK>;
qcom,chipid = <0x03020100>;
-   qcom,gpu-pwrlevels {
-   compatible = "qcom,gpu-pwrlevels";
-   qcom,gpu-pwrlevel@0 {
-   qcom,gpu-freq = <45000>;
-   };
-   qcom,gpu-pwrlevel@1 {
-   qcom,gpu-freq = <2700>;
-   };
-   };
};
 };
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index e130b5e..7b9181b2 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -224,8 +224,10 @@ static int adreno_bind(struct device *dev, struct device 
*master, void *data)
}
 
if (!config.fast_rate) {
-   dev_err(dev, "could not find clk rates\n");
-   return -ENXIO;
+   dev_warn(dev, "could not find clk rates\n");
+   /* This is a safe low speed for all devices: */
+   config.fast_rate = 2;
+   config.slow_rate = 2700;
}
 
for (i = 0; i < ARRAY_SIZE(quirks); i++)
-- 
2.9.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm/msm: drop quirks binding

[Rob Clark]

This was never documented or used in upstream dtb.  It is used by
downstream bindings from android device kernels.  But the quirks are
a property of the gpu revision, and as such are redundant to be listed
separately in dt.  Instead, move the quirks to the device table.

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c  |  4 ++--
 drivers/gpu/drm/msm/adreno/adreno_device.c | 18 --
 drivers/gpu/drm/msm/adreno/adreno_gpu.c|  1 -
 drivers/gpu/drm/msm/adreno/adreno_gpu.h|  4 +---
 4 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 5f8b368..71b30dd 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -543,7 +543,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
/* Enable RBBM error reporting bits */
gpu_write(gpu, REG_A5XX_RBBM_AHB_CNTL0, 0x0001);
 
-   if (adreno_gpu->quirks & ADRENO_QUIRK_FAULT_DETECT_MASK) {
+   if (adreno_gpu->info->quirks & ADRENO_QUIRK_FAULT_DETECT_MASK) {
/*
 * Mask out the activity signals from RB1-3 to avoid false
 * positives
@@ -597,7 +597,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
 
gpu_write(gpu, REG_A5XX_PC_DBG_ECO_CNTL, (0x400 << 11 | 0x300 << 22));
 
-   if (adreno_gpu->quirks & ADRENO_QUIRK_TWO_PASS_USE_WFI)
+   if (adreno_gpu->info->quirks & ADRENO_QUIRK_TWO_PASS_USE_WFI)
gpu_rmw(gpu, REG_A5XX_PC_DBG_ECO_CNTL, 0, (1 << 8));
 
gpu_write(gpu, REG_A5XX_PC_DBG_ECO_CNTL, 0xc0200100);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index fdaef67..4d0745d9 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -75,12 +75,14 @@ static const struct adreno_info gpulist[] = {
.gmem  = (SZ_1M + SZ_512K),
.init  = a4xx_gpu_init,
}, {
-   .rev = ADRENO_REV(5, 3, 0, ANY_ID),
+   .rev = ADRENO_REV(5, 3, 0, 2),
.revn = 530,
.name = "A530",
.pm4fw = "a530_pm4.fw",
.pfpfw = "a530_pfp.fw",
.gmem = SZ_1M,
+   .quirks = ADRENO_QUIRK_TWO_PASS_USE_WFI |
+   ADRENO_QUIRK_FAULT_DETECT_MASK,
.init = a5xx_gpu_init,
.gpmufw = "a530v3_gpmu.fw2",
},
@@ -181,14 +183,6 @@ static void set_gpu_pdev(struct drm_device *dev,
priv->gpu_pdev = pdev;
 }
 
-static const struct {
-   const char *str;
-   uint32_t flag;
-} quirks[] = {
-   { "qcom,gpu-quirk-two-pass-use-wfi", ADRENO_QUIRK_TWO_PASS_USE_WFI },
-   { "qcom,gpu-quirk-fault-detect-mask", ADRENO_QUIRK_FAULT_DETECT_MASK },
-};
-
 static int find_chipid(struct device *dev, u32 *chipid)
 {
struct device_node *node = dev->of_node;
@@ -234,7 +228,7 @@ static int adreno_bind(struct device *dev, struct device 
*master, void *data)
static struct adreno_platform_config config = {};
struct device_node *child, *node = dev->of_node;
u32 val;
-   int ret, i;
+   int ret;
 
ret = find_chipid(dev, );
if (ret) {
@@ -270,10 +264,6 @@ static int adreno_bind(struct device *dev, struct device 
*master, void *data)
config.slow_rate = 2700;
}
 
-   for (i = 0; i < ARRAY_SIZE(quirks); i++)
-   if (of_property_read_bool(node, quirks[i].str))
-   config.quirks |= quirks[i].flag;
-
dev->platform_data = 
set_gpu_pdev(dev_get_drvdata(master), to_platform_device(dev));
 
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index bc2224b..f67e6f8 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -352,7 +352,6 @@ int adreno_gpu_init(struct drm_device *drm, struct 
platform_device *pdev,
adreno_gpu->gmem = adreno_gpu->info->gmem;
adreno_gpu->revn = adreno_gpu->info->revn;
adreno_gpu->rev = config->rev;
-   adreno_gpu->quirks = config->quirks;
 
gpu->fast_rate = config->fast_rate;
gpu->slow_rate = config->slow_rate;
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index e8d55b0..42e444a 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -75,6 +75,7 @@ struct adreno_info {
const char *pm4fw, *pfpfw;
const char *gpmufw;
uint32_t gmem;
+   enum adreno_quirks quirks;
struct msm_gpu *(*init)(struct drm_device *dev);
 };
 
@@ -116,8 +117,6 @@ struct adreno_gpu {
 * code (a3xx_gpu.c) and stored in this common location.
 */
const unsigned int *reg_offsets;
-
-   uint32_t quirks;
 };
 #define 

[PATCH 4/4] drm/msm: drop _clk suffix from clk names

[Rob Clark]

Suggested by Rob Herring.  We still support the old names for
compatibility with downstream android dt files.

Cc: Rob Herring 
Signed-off-by: Rob Clark 
---
 Documentation/devicetree/bindings/display/msm/gpu.txt | 12 ++--
 drivers/gpu/drm/msm/msm_drv.c | 19 +++
 drivers/gpu/drm/msm/msm_drv.h |  1 +
 drivers/gpu/drm/msm/msm_gpu.c |  7 +++
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt 
b/Documentation/devicetree/bindings/display/msm/gpu.txt
index 7ac3052..43fac0f 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -11,9 +11,9 @@ Required properties:
 - clocks: device clocks
   See ../clocks/clock-bindings.txt for details.
 - clock-names: the following clocks are required:
-  * "core_clk"
-  * "iface_clk"
-  * "mem_iface_clk"
+  * "core"
+  * "iface"
+  * "mem_iface"
 
 Example:
 
@@ -27,9 +27,9 @@ Example:
interrupts = ;
interrupt-names = "kgsl_3d0_irq";
clock-names =
-   "core_clk",
-   "iface_clk",
-   "mem_iface_clk";
+   "core",
+   "iface",
+   "mem_iface";
clocks =
< GFX3D_CLK>,
< GFX3D_AHB_CLK>,
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 6b85c41..a51d783 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -91,6 +91,25 @@ module_param(dumpstate, bool, 0600);
  * Util/helpers:
  */
 
+struct clk *msm_clk_get(struct platform_device *pdev, const char *name)
+{
+   struct clk *clk;
+   char name2[32];
+
+   clk = devm_clk_get(>dev, name);
+   if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
+   return clk;
+
+   snprintf(name2, sizeof(name2), "%s_clk", name);
+
+   clk = devm_clk_get(>dev, name2);
+   if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
+   dev_warn(>dev, "Using legacy clk name binding.  Use "
+   "\"%s\" instead of \"%s\"\n", name, name2);
+
+   return clk;
+}
+
 void __iomem *msm_ioremap(struct platform_device *pdev, const char *name,
const char *dbgname)
 {
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index ed4dad3..5f6f48f 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -318,6 +318,7 @@ static inline int msm_debugfs_late_init(struct drm_device 
*dev) { return 0; }
 static inline void msm_rd_dump_submit(struct msm_gem_submit *submit) {}
 #endif
 
+struct clk *msm_clk_get(struct platform_device *pdev, const char *name);
 void __iomem *msm_ioremap(struct platform_device *pdev, const char *name,
const char *dbgname);
 void msm_writel(u32 data, void __iomem *addr);
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index d8420be..7b29843 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -564,8 +564,7 @@ static irqreturn_t irq_handler(int irq, void *data)
 }
 
 static const char *clk_names[] = {
-   "core_clk", "iface_clk", "rbbmtimer_clk", "mem_clk",
-   "mem_iface_clk", "alt_mem_iface_clk",
+   "core", "iface", "rbbmtimer", "mem", "mem_iface", "alt_mem_iface",
 };
 
 int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
@@ -629,13 +628,13 @@ int msm_gpu_init(struct drm_device *drm, struct 
platform_device *pdev,
 
/* Acquire clocks: */
for (i = 0; i < ARRAY_SIZE(clk_names); i++) {
-   gpu->grp_clks[i] = devm_clk_get(>dev, clk_names[i]);
+   gpu->grp_clks[i] = msm_clk_get(pdev, clk_names[i]);
DBG("grp_clks[%s]: %p", clk_names[i], gpu->grp_clks[i]);
if (IS_ERR(gpu->grp_clks[i]))
gpu->grp_clks[i] = NULL;
}
 
-   gpu->ebi1_clk = devm_clk_get(>dev, "bus_clk");
+   gpu->ebi1_clk = msm_clk_get(pdev, "bus");
DBG("ebi1_clk: %p", gpu->ebi1_clk);
if (IS_ERR(gpu->ebi1_clk))
gpu->ebi1_clk = NULL;
-- 
2.9.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/msm: drop qcom,chipid

[Rob Clark]

The original way we determined the gpu version was based on downstream
bindings from android kernel.  A cleaner way is to get the version from
the compatible string.

Note that no upstream dtb uses these bindings.  But the code still
supports falling back to the legacy bindings (with a warning), so that
we are still compatible with the gpu dt node from android device
kernels.

Signed-off-by: Rob Clark 
---
 .../devicetree/bindings/display/msm/gpu.txt| 11 +++---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 43 +-
 drivers/gpu/drm/msm/msm_drv.c  |  1 +
 3 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt 
b/Documentation/devicetree/bindings/display/msm/gpu.txt
index 747b984..7ac3052 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -1,7 +1,11 @@
 Qualcomm adreno/snapdragon GPU
 
 Required properties:
-- compatible: "qcom,adreno-3xx"
+- compatible: "qcom,adreno-XYZ.W", "qcom,adreno"
+for example: "qcom,adreno-306.0", "qcom,adreno"
+  Note that you need to list the less specific "qcom,adreno" (since this
+  is what the device is matched on), in addition to the more specific
+  with the chip-id.
 - reg: Physical base address and length of the controller's registers.
 - interrupts: The interrupt signal from the gpu.
 - clocks: device clocks
@@ -10,8 +14,6 @@ Required properties:
   * "core_clk"
   * "iface_clk"
   * "mem_iface_clk"
-- qcom,chipid: gpu chip-id.  Note this may become optional for future
-  devices if we can reliably read the chipid from hw
 
 Example:
 
@@ -19,7 +21,7 @@ Example:
...
 
gpu: qcom,kgsl-3d0@430 {
-   compatible = "qcom,adreno-3xx";
+   compatible = "qcom,adreno-320.2", "qcom,adreno";
reg = <0x0430 0x2>;
reg-names = "kgsl_3d0_reg_memory";
interrupts = ;
@@ -32,6 +34,5 @@ Example:
< GFX3D_CLK>,
< GFX3D_AHB_CLK>,
< MMSS_IMEM_AHB_CLK>;
-   qcom,chipid = <0x03020100>;
};
 };
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 7b9181b2..fdaef67 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -189,6 +189,46 @@ static const struct {
{ "qcom,gpu-quirk-fault-detect-mask", ADRENO_QUIRK_FAULT_DETECT_MASK },
 };
 
+static int find_chipid(struct device *dev, u32 *chipid)
+{
+   struct device_node *node = dev->of_node;
+   struct property *prop;
+   const char *compat;
+   int ret;
+
+   /* first search the compat strings for qcom,adreno-XYZ.W: */
+   prop = of_find_property(node, "compatible", NULL);
+   for (compat = of_prop_next_string(prop, NULL); compat;
+compat = of_prop_next_string(prop, compat)) {
+   unsigned rev, patch;
+
+   if (sscanf(compat, "qcom,adreno-%u.%u", , ) != 2)
+   continue;
+
+   *chipid = 0;
+   *chipid |= (rev / 100) << 24;  /* core */
+   rev %= 100;
+   *chipid |= (rev / 10) << 16;   /* major */
+   rev %= 10;
+   *chipid |= rev << 8;   /* minor */
+   *chipid |= patch;
+
+   return 0;
+   }
+
+   /* and if that fails, fall back to legacy "qcom,chipid" property: */
+   ret = of_property_read_u32(node, "qcom,chipid", chipid);
+   if (ret)
+   return ret;
+
+   dev_warn(dev, "Using legacy qcom,chipid binding!\n");
+   dev_warn(dev, "Use compatible qcom,adreno-%u%u%u.%u instead.\n",
+   (*chipid >> 24) & 0xff, (*chipid >> 16) & 0xff,
+   (*chipid >> 8) & 0xff, *chipid & 0xff);
+
+   return 0;
+}
+
 static int adreno_bind(struct device *dev, struct device *master, void *data)
 {
static struct adreno_platform_config config = {};
@@ -196,7 +236,7 @@ static int adreno_bind(struct device *dev, struct device 
*master, void *data)
u32 val;
int ret, i;
 
-   ret = of_property_read_u32(node, "qcom,chipid", );
+   ret = find_chipid(dev, );
if (ret) {
dev_err(dev, "could not find chipid: %d\n", ret);
return ret;
@@ -265,6 +305,7 @@ static int adreno_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id dt_match[] = {
+   { .compatible = "qcom,adreno" },
{ .compatible = "qcom,adreno-3xx" },
/* for backwards compat w/ downstream kgsl DT files: */
{ .compatible = "qcom,kgsl-3d0" },
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index e29bb66..6b85c41 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -985,6 +985,7 @@ static 

[PATCH 0/4] drm/msm: cleanup gpu bindings

[Rob Clark]

Existing bindings for the gpu are based on the downstream android kgsl
driver (a subset of the downstream bindings).  But that isn't really
how we want the upstream bindings to look.

For now we have held off on adding gpu nodes to upstream dt files, but
now that all the dependencies are in place for some devices, it is time
to clean things up so we can start adding this missing gpu nodes.

Note that these patches preserve compatibility with downstream dt files
because, at this point, it is easy and convenient to not break people's
patchsets for upstream support of various devices.

Rob Clark (4):
  drm/msm: remove qcom,gpu-pwrlevels bindings
  drm/msm: drop qcom,chipid
  drm/msm: drop quirks binding
  drm/msm: drop _clk suffix from clk names

 .../devicetree/bindings/display/msm/gpu.txt| 38 -
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c  |  4 +-
 drivers/gpu/drm/msm/adreno/adreno_device.c | 65 --
 drivers/gpu/drm/msm/adreno/adreno_gpu.c|  1 -
 drivers/gpu/drm/msm/adreno/adreno_gpu.h|  4 +-
 drivers/gpu/drm/msm/msm_drv.c  | 20 +++
 drivers/gpu/drm/msm/msm_drv.h  |  1 +
 drivers/gpu/drm/msm/msm_gpu.c  |  7 +--
 8 files changed, 88 insertions(+), 52 deletions(-)

-- 
2.9.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: mali-dp: add custom plane ->reset hook

[Liviu Dudau]

On Mon, Jan 30, 2017 at 03:42:26PM +, Mihail Atanassov wrote:
> The reset hook needs to allocate space for a
> malidp_plane_state, which is larger than drm_plane_state.
> Otherwise, the hook is identical to the default one.
> 
> Signed-off-by: Mihail Atanassov 

Acked-by: Liviu Dudau 

Thanks,
Liviu

> ---
>  drivers/gpu/drm/arm/malidp_planes.c | 23 ++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c 
> b/drivers/gpu/drm/arm/malidp_planes.c
> index eff2fe4..686ff86 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -58,6 +58,27 @@ static void malidp_de_plane_destroy(struct drm_plane 
> *plane)
>   devm_kfree(plane->dev->dev, mp);
>  }
>  
> +/*
> + * Replicate what the default ->reset hook does: free the state pointer and
> + * allocate a new empty object. We just need enough space to store
> + * a malidp_plane_state instead of a drm_plane_state.
> + */
> +static void malidp_plane_reset(struct drm_plane *plane)
> +{
> + struct malidp_plane_state *state = to_malidp_plane_state(plane->state);
> +
> + if (state)
> + __drm_atomic_helper_plane_destroy_state(>base);
> + kfree(state);
> + plane->state = NULL;
> + state = kzalloc(sizeof(*state), GFP_KERNEL);
> + if (state) {
> + state->base.plane = plane;
> + state->base.rotation = DRM_ROTATE_0;
> + plane->state = >base;
> + }
> +}
> +
>  static struct
>  drm_plane_state *malidp_duplicate_plane_state(struct drm_plane *plane)
>  {
> @@ -92,7 +113,7 @@ static void malidp_destroy_plane_state(struct drm_plane 
> *plane,
>   .disable_plane = drm_atomic_helper_disable_plane,
>   .set_property = drm_atomic_helper_plane_set_property,
>   .destroy = malidp_de_plane_destroy,
> - .reset = drm_atomic_helper_plane_reset,
> + .reset = malidp_plane_reset,
>   .atomic_duplicate_state = malidp_duplicate_plane_state,
>   .atomic_destroy_state = malidp_destroy_plane_state,
>  };
> -- 
> 1.9.1
> 

-- 

| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---
¯\_(ツ)_/¯
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings

[Hans de Goede]

Hi,

On 30-01-17 16:38, Ville Syrjälä wrote:

On Mon, Jan 30, 2017 at 04:27:58PM +0100, Hans de Goede wrote:

Hi,

On 30-01-17 16:11, Ville Syrjälä wrote:

On Mon, Jan 30, 2017 at 04:02:19PM +0100, Hans de Goede wrote:

Hi,

On 30-01-17 14:10, Ville Syrjälä wrote:

On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:

Hi,

On 01/28/2017 05:25 PM, Hans de Goede wrote:

Hi,

On 01/27/2017 02:51 PM, Ville Syrjälä wrote:

On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:

Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
around P-Unit write accesses.


Can't we just stuff the calls into the actual punit write function
rather than sprinkling them all over the place?


punit access is acquired across sections like this:

iosf_mbi_punit_acquire();

val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
val &= ~DSPFREQGUAR_MASK;
val |= (cmd << DSPFREQGUAR_SHIFT);
vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
  DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
 50)) {
DRM_ERROR("timed out waiting for CDclk change\n");
}
iosf_mbi_punit_release();

Where we want to wait for the requested change to have taken
effect before releasing the punit.


Hmm. That's somewhat unfortunate. It also highlights a problem with the
patch wrt. RPS. We don't wait for the GPU to actually change frequencies
in set_rps() because that would slow things down too much. So I have to
wonder how much luck is needed to make this workaround really effective.


So the history of this patch-set is that I wrote this patch before
writing the patch to get FORCEWAKE_ALL before the pmic bus becomes
active (patch 12/13). Since a lot of testing was done with this
patch included in the patch-set and since it seemed a good idea
regardless (given my experience with accessing the punit vs
pmic bus accesses) I decided to leave it in.

Possibly just the patch to get FORCEWAKE_ALL is enough, that one
actually fixed things for me. That is also why I made this the
last patch in the set. I asked tagorereddy to test his system
without this patch, but he did not get around to that.

After all we do tell the punit to not touch the bus by acquiring
the pmic bus semaphore from i2c-desigware-baytrail.c, so maybe
for RPS freq changes it honors that and properly waits. Maybe it
honors that for all punit requests i915 does and the only real
problem is the forcewake stuff ?

I can try to drop this patch from my queue and run without it
for a while and see if things don't regress. And also ask
tagorereddy again to test his system that way.

Does that (dropping this patch for now) sound like a good idea?


More test results couldn't hurt at least. It also makes me wonder if
just bumping the timeouts to some ridiculously high value would fix
the problem as well.


I've already tried bumping the forcewake timeout from 50 to 250ms,
before writing the patch to just get forcewake_all before the pmic
bus access begins, that does not fix things,


And you bumped the i2c mutex timeout as well? Or does that fail somehow
gracefully if it can't get the mutex?


It will fail the i2c transfer with -ETIMEOUT, which will make the driver
report an error instead of e.g. the battery level, but it should not
affect the forcewake calls and those still failed with the large
timeout. So yes basically the i2c mutex fails gracefully.




and since we busy wait
for this timeout from non-sleeping context 250ms already is way too
high.


Sure, but I'm just trying to understand if the problem is simply caused
by proceeding with some hardware access without getting the i2c mutex.


Understood.


+ a comment would be nice why it's there.


I will add comments to the acquire calls.


Do we need a kconfig select/depends on the iosf_mbi thing? Or some
ifdefs?


No, the iosf_mbi header defines empty inline versions of
iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
this does mean that iosf_mbi must be builtin if the i915
driver is. I'll add:

depends on DRM_I915=IOSF_MBI || IOSF_MBI=y

To the i915 Kconfig to enforce this.


Hmm, ok so that does not work (long cyclic dependency through the
selection of ACPI_VIDEO).

So I've now added this instead:

# iosf_mbi needs to be builtin if we are builtin
select IOSF_MBI if DRM_I915=y


That's probably not going to help anyone since i915 is usually a module.


Right, that is fine, then either the IOSF_MBI symbols are available,
or IOSF_MBI is disabled and we get the inline nops from the header.

The problem scenario is DRM_I915=y and IOSF_MBI=m, which is not very
realistic IMHO, but will get triggered by the random-config testing
several contributors do and lead to an unresolved symbol error there.


Well, from the user POV anything with IOSF_MBI==n 

[PATCH] drm: mali-dp: add custom plane ->reset hook

[Mihail Atanassov]

The reset hook needs to allocate space for a
malidp_plane_state, which is larger than drm_plane_state.
Otherwise, the hook is identical to the default one.

Signed-off-by: Mihail Atanassov 
---
 drivers/gpu/drm/arm/malidp_planes.c | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/malidp_planes.c 
b/drivers/gpu/drm/arm/malidp_planes.c
index eff2fe4..686ff86 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -58,6 +58,27 @@ static void malidp_de_plane_destroy(struct drm_plane *plane)
devm_kfree(plane->dev->dev, mp);
 }
 
+/*
+ * Replicate what the default ->reset hook does: free the state pointer and
+ * allocate a new empty object. We just need enough space to store
+ * a malidp_plane_state instead of a drm_plane_state.
+ */
+static void malidp_plane_reset(struct drm_plane *plane)
+{
+   struct malidp_plane_state *state = to_malidp_plane_state(plane->state);
+
+   if (state)
+   __drm_atomic_helper_plane_destroy_state(>base);
+   kfree(state);
+   plane->state = NULL;
+   state = kzalloc(sizeof(*state), GFP_KERNEL);
+   if (state) {
+   state->base.plane = plane;
+   state->base.rotation = DRM_ROTATE_0;
+   plane->state = >base;
+   }
+}
+
 static struct
 drm_plane_state *malidp_duplicate_plane_state(struct drm_plane *plane)
 {
@@ -92,7 +113,7 @@ static void malidp_destroy_plane_state(struct drm_plane 
*plane,
.disable_plane = drm_atomic_helper_disable_plane,
.set_property = drm_atomic_helper_plane_set_property,
.destroy = malidp_de_plane_destroy,
-   .reset = drm_atomic_helper_plane_reset,
+   .reset = malidp_plane_reset,
.atomic_duplicate_state = malidp_duplicate_plane_state,
.atomic_destroy_state = malidp_destroy_plane_state,
 };
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 01/24] drm/rockchip: dw-mipi-dsi: don't configure hardware in mode_set for MIPI

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:21PM +, John Keeping wrote:
> With atomic modesetting the hardware will be powered off when the
> mode_set function is called.  We should configure the hardware in the
> enable function, which is the atomic version of "commit" so let's use
> the enable hook rather than commit while we're at it.
> 

Reviewed-by: Sean Paul 

> Signed-off-by: John Keeping 
> ---
> v3:
> - Squash together with the commit to s/commit/enable/
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 49 
> +++---
>  1 file changed, 21 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index d9aa382bb629..bbd992299f73 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -819,34 +819,8 @@ static void dw_mipi_dsi_encoder_mode_set(struct 
> drm_encoder *encoder,
>   struct drm_display_mode *adjusted_mode)
>  {
>   struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> - int ret;
>  
>   dsi->mode = adjusted_mode;
> -
> - ret = dw_mipi_dsi_get_lane_bps(dsi);
> - if (ret < 0)
> - return;
> -
> - if (clk_prepare_enable(dsi->pclk)) {
> - dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__);
> - return;
> - }
> -
> - dw_mipi_dsi_init(dsi);
> - dw_mipi_dsi_dpi_config(dsi, mode);
> - dw_mipi_dsi_packet_handler_config(dsi);
> - dw_mipi_dsi_video_mode_config(dsi);
> - dw_mipi_dsi_video_packet_config(dsi, mode);
> - dw_mipi_dsi_command_mode_config(dsi);
> - dw_mipi_dsi_line_timer_config(dsi);
> - dw_mipi_dsi_vertical_timing_config(dsi);
> - dw_mipi_dsi_dphy_timing_config(dsi);
> - dw_mipi_dsi_dphy_interface_config(dsi);
> - dw_mipi_dsi_clear_err(dsi);
> - if (drm_panel_prepare(dsi->panel))
> - dev_err(dsi->dev, "failed to prepare panel\n");
> -
> - clk_disable_unprepare(dsi->pclk);
>  }
>  
>  static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
> @@ -875,17 +849,36 @@ static void dw_mipi_dsi_encoder_disable(struct 
> drm_encoder *encoder)
>   clk_disable_unprepare(dsi->pclk);
>  }
>  
> -static void dw_mipi_dsi_encoder_commit(struct drm_encoder *encoder)
> +static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  {
>   struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
>   int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
>   u32 val;
> + int ret;
> +
> + ret = dw_mipi_dsi_get_lane_bps(dsi);
> + if (ret < 0)
> + return;
>  
>   if (clk_prepare_enable(dsi->pclk)) {
>   dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__);
>   return;
>   }
>  
> + dw_mipi_dsi_init(dsi);
> + dw_mipi_dsi_dpi_config(dsi, dsi->mode);
> + dw_mipi_dsi_packet_handler_config(dsi);
> + dw_mipi_dsi_video_mode_config(dsi);
> + dw_mipi_dsi_video_packet_config(dsi, dsi->mode);
> + dw_mipi_dsi_command_mode_config(dsi);
> + dw_mipi_dsi_line_timer_config(dsi);
> + dw_mipi_dsi_vertical_timing_config(dsi);
> + dw_mipi_dsi_dphy_timing_config(dsi);
> + dw_mipi_dsi_dphy_interface_config(dsi);
> + dw_mipi_dsi_clear_err(dsi);
> + if (drm_panel_prepare(dsi->panel))
> + dev_err(dsi->dev, "failed to prepare panel\n");
> +
>   dw_mipi_dsi_phy_init(dsi);
>   dw_mipi_dsi_wait_for_two_frames(dsi);
>  
> @@ -933,7 +926,7 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder 
> *encoder,
>  
>  static struct drm_encoder_helper_funcs
>  dw_mipi_dsi_encoder_helper_funcs = {
> - .commit = dw_mipi_dsi_encoder_commit,
> + .enable = dw_mipi_dsi_encoder_enable,
>   .mode_set = dw_mipi_dsi_encoder_mode_set,
>   .disable = dw_mipi_dsi_encoder_disable,
>   .atomic_check = dw_mipi_dsi_encoder_atomic_check,
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 v3 02/24] drm/rockchip: dw-mipi-dsi: pass mode in where needed

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:22PM +, John Keeping wrote:
> This shows that we only use the mode from the enable function and
> prepares us to remove the "mode" field and the mode_set hook in the next
> commit.
> 

Reviewed-by: Sean Paul 

> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Add Chris' Reviewed-by
> New in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 41 
> ++
>  1 file changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index bbd992299f73..cdbd25087e83 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -330,11 +330,11 @@ static int max_mbps_to_testdin(unsigned int max_mbps)
>   * The controller should generate 2 frames before
>   * preparing the peripheral.
>   */
> -static void dw_mipi_dsi_wait_for_two_frames(struct dw_mipi_dsi *dsi)
> +static void dw_mipi_dsi_wait_for_two_frames(struct drm_display_mode *mode)
>  {
>   int refresh, two_frames;
>  
> - refresh = drm_mode_vrefresh(dsi->mode);
> + refresh = drm_mode_vrefresh(mode);
>   two_frames = DIV_ROUND_UP(MSEC_PER_SEC, refresh) * 2;
>   msleep(two_frames);
>  }
> @@ -459,7 +459,8 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>   return ret;
>  }
>  
> -static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi *dsi)
> +static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi *dsi,
> + struct drm_display_mode *mode)
>  {
>   unsigned int i, pre;
>   unsigned long mpclk, pllref, tmp;
> @@ -474,7 +475,7 @@ static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi 
> *dsi)
>   return bpp;
>   }
>  
> - mpclk = DIV_ROUND_UP(dsi->mode->clock, MSEC_PER_SEC);
> + mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC);
>   if (mpclk) {
>   /* take 1 / 0.9, since mbps must big than bandwidth of RGB */
>   tmp = mpclk * (bpp / dsi->lanes) * 10 / 9;
> @@ -742,43 +743,44 @@ static void dw_mipi_dsi_command_mode_config(struct 
> dw_mipi_dsi *dsi)
>  
>  /* Get lane byte clock cycles. */
>  static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct dw_mipi_dsi *dsi,
> +struct drm_display_mode *mode,
>  u32 hcomponent)
>  {
>   u32 frac, lbcc;
>  
>   lbcc = hcomponent * dsi->lane_mbps * MSEC_PER_SEC / 8;
>  
> - frac = lbcc % dsi->mode->clock;
> - lbcc = lbcc / dsi->mode->clock;
> + frac = lbcc % mode->clock;
> + lbcc = lbcc / mode->clock;
>   if (frac)
>   lbcc++;
>  
>   return lbcc;
>  }
>  
> -static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi)
> +static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi,
> +   struct drm_display_mode *mode)
>  {
>   u32 htotal, hsa, hbp, lbcc;
> - struct drm_display_mode *mode = dsi->mode;
>  
>   htotal = mode->htotal;
>   hsa = mode->hsync_end - mode->hsync_start;
>   hbp = mode->htotal - mode->hsync_end;
>  
> - lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, htotal);
> + lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, htotal);
>   dsi_write(dsi, DSI_VID_HLINE_TIME, lbcc);
>  
> - lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, hsa);
> + lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hsa);
>   dsi_write(dsi, DSI_VID_HSA_TIME, lbcc);
>  
> - lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, hbp);
> + lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hbp);
>   dsi_write(dsi, DSI_VID_HBP_TIME, lbcc);
>  }
>  
> -static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi)
> +static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
> +struct drm_display_mode *mode)
>  {
>   u32 vactive, vsa, vfp, vbp;
> - struct drm_display_mode *mode = dsi->mode;
>  
>   vactive = mode->vdisplay;
>   vsa = mode->vsync_end - mode->vsync_start;
> @@ -852,11 +854,12 @@ static void dw_mipi_dsi_encoder_disable(struct 
> drm_encoder *encoder)
>  static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  {
>   struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> + struct drm_display_mode *mode = dsi->mode;
>   int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
>   u32 val;
>   int ret;
>  
> - ret = dw_mipi_dsi_get_lane_bps(dsi);
> + ret = dw_mipi_dsi_get_lane_bps(dsi, mode);
>   if (ret < 0)
>   return;
>  
> @@ -866,13 +869,13 @@ static void dw_mipi_dsi_encoder_enable(struct 
> drm_encoder *encoder)
>   }
>  
>   dw_mipi_dsi_init(dsi);
> - dw_mipi_dsi_dpi_config(dsi, dsi->mode);
> + dw_mipi_dsi_dpi_config(dsi, mode);
>   

Re: [PATCH v3 04/24] drm/rockchip: dw-mipi-dsi: fix command header writes

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:24PM +, John Keeping wrote:
> In a couple of places here we use "val" for the value that is about to
> be written to a register but then reuse the same variable for the value
> of a status register before we get around to writing it.  Rename the
> value to be written to so that we write the value we intend to and not
> what we have just read from the status register.
> 

Oh my.

Reviewed-by: Sean Paul 

> Signed-off-by: John Keeping 
> Tested-by: Chris Zhong 
> Reviewed-by: Chris Zhong 
> ---
> Unchanged in v3
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index bd92e58b64f3..4cbbbcb619b7 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -542,9 +542,10 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host 
> *host,
>   return 0;
>  }
>  
> -static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 val)
> +static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 
> hdr_val)
>  {
>   int ret;
> + u32 val;
>  
>   ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS,
>val, !(val & GEN_CMD_FULL), 1000,
> @@ -554,7 +555,7 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct 
> dw_mipi_dsi *dsi, u32 val)
>   return ret;
>   }
>  
> - dsi_write(dsi, DSI_GEN_HDR, val);
> + dsi_write(dsi, DSI_GEN_HDR, hdr_val);
>  
>   ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS,
>val, val & (GEN_CMD_EMPTY | GEN_PLD_W_EMPTY),
> @@ -587,8 +588,9 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi 
> *dsi,
>  {
>   const u32 *tx_buf = msg->tx_buf;
>   int len = msg->tx_len, pld_data_bytes = sizeof(*tx_buf), ret;
> - u32 val = GEN_HDATA(msg->tx_len) | GEN_HTYPE(msg->type);
> + u32 hdr_val = GEN_HDATA(msg->tx_len) | GEN_HTYPE(msg->type);
>   u32 remainder = 0;
> + u32 val;
>  
>   if (msg->tx_len < 3) {
>   dev_err(dsi->dev, "wrong tx buf length %zu for long write\n",
> @@ -617,7 +619,7 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi 
> *dsi,
>   }
>   }
>  
> - return dw_mipi_dsi_gen_pkt_hdr_write(dsi, val);
> + return dw_mipi_dsi_gen_pkt_hdr_write(dsi, hdr_val);
>  }
>  
>  static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 v3 03/24] drm/rockchip: dw-mipi-dsi: remove mode_set hook

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:23PM +, John Keeping wrote:
> This is not needed since we can access the mode via the CRTC from the
> enable hook.  Also remove the "mode" field that is no longer used.
> 

Reviewed-by: Sean Paul 

> Signed-off-by: John Keeping 
> Reviewed-by: Chris Zhong 
> ---
> v3:
> - Add Chris' Reviewed-by
> New in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 13 +
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index cdbd25087e83..bd92e58b64f3 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -286,7 +286,6 @@ struct dw_mipi_dsi {
>   u32 format;
>   u16 input_div;
>   u16 feedback_div;
> - struct drm_display_mode *mode;
>  
>   const struct dw_mipi_dsi_plat_data *pdata;
>  };
> @@ -816,15 +815,6 @@ static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi 
> *dsi)
>   dsi_write(dsi, DSI_INT_MSK1, 0);
>  }
>  
> -static void dw_mipi_dsi_encoder_mode_set(struct drm_encoder *encoder,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> -{
> - struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> -
> - dsi->mode = adjusted_mode;
> -}
> -
>  static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
>  {
>   struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> @@ -854,7 +844,7 @@ static void dw_mipi_dsi_encoder_disable(struct 
> drm_encoder *encoder)
>  static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  {
>   struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> - struct drm_display_mode *mode = dsi->mode;
> + struct drm_display_mode *mode = >crtc->state->adjusted_mode;
>   int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
>   u32 val;
>   int ret;
> @@ -930,7 +920,6 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder 
> *encoder,
>  static struct drm_encoder_helper_funcs
>  dw_mipi_dsi_encoder_helper_funcs = {
>   .enable = dw_mipi_dsi_encoder_enable,
> - .mode_set = dw_mipi_dsi_encoder_mode_set,
>   .disable = dw_mipi_dsi_encoder_disable,
>   .atomic_check = dw_mipi_dsi_encoder_atomic_check,
>  };
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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 v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings

[Ville Syrjälä]

On Mon, Jan 30, 2017 at 04:27:58PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 30-01-17 16:11, Ville Syrjälä wrote:
> > On Mon, Jan 30, 2017 at 04:02:19PM +0100, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 30-01-17 14:10, Ville Syrjälä wrote:
> >>> On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:
>  Hi,
> 
>  On 01/28/2017 05:25 PM, Hans de Goede wrote:
> > Hi,
> >
> > On 01/27/2017 02:51 PM, Ville Syrjälä wrote:
> >> On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:
> >>> Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
> >>> request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
> >>> around P-Unit write accesses.
> >>
> >> Can't we just stuff the calls into the actual punit write function
> >> rather than sprinkling them all over the place?
> >
> > punit access is acquired across sections like this:
> >
> > iosf_mbi_punit_acquire();
> >
> > val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> > val &= ~DSPFREQGUAR_MASK;
> > val |= (cmd << DSPFREQGUAR_SHIFT);
> > vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
> > if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
> >   DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
> >  50)) {
> > DRM_ERROR("timed out waiting for CDclk change\n");
> > }
> > iosf_mbi_punit_release();
> >
> > Where we want to wait for the requested change to have taken
> > effect before releasing the punit.
> >>>
> >>> Hmm. That's somewhat unfortunate. It also highlights a problem with the
> >>> patch wrt. RPS. We don't wait for the GPU to actually change frequencies
> >>> in set_rps() because that would slow things down too much. So I have to
> >>> wonder how much luck is needed to make this workaround really effective.
> >>
> >> So the history of this patch-set is that I wrote this patch before
> >> writing the patch to get FORCEWAKE_ALL before the pmic bus becomes
> >> active (patch 12/13). Since a lot of testing was done with this
> >> patch included in the patch-set and since it seemed a good idea
> >> regardless (given my experience with accessing the punit vs
> >> pmic bus accesses) I decided to leave it in.
> >>
> >> Possibly just the patch to get FORCEWAKE_ALL is enough, that one
> >> actually fixed things for me. That is also why I made this the
> >> last patch in the set. I asked tagorereddy to test his system
> >> without this patch, but he did not get around to that.
> >>
> >> After all we do tell the punit to not touch the bus by acquiring
> >> the pmic bus semaphore from i2c-desigware-baytrail.c, so maybe
> >> for RPS freq changes it honors that and properly waits. Maybe it
> >> honors that for all punit requests i915 does and the only real
> >> problem is the forcewake stuff ?
> >>
> >> I can try to drop this patch from my queue and run without it
> >> for a while and see if things don't regress. And also ask
> >> tagorereddy again to test his system that way.
> >>
> >> Does that (dropping this patch for now) sound like a good idea?
> >
> > More test results couldn't hurt at least. It also makes me wonder if
> > just bumping the timeouts to some ridiculously high value would fix
> > the problem as well.
> 
> I've already tried bumping the forcewake timeout from 50 to 250ms,
> before writing the patch to just get forcewake_all before the pmic
> bus access begins, that does not fix things,

And you bumped the i2c mutex timeout as well? Or does that fail somehow
gracefully if it can't get the mutex?

> and since we busy wait
> for this timeout from non-sleeping context 250ms already is way too
> high.

Sure, but I'm just trying to understand if the problem is simply caused
by proceeding with some hardware access without getting the i2c mutex.

> 
> >> + a comment would be nice why it's there.
> >
> > I will add comments to the acquire calls.
> >
> >> Do we need a kconfig select/depends on the iosf_mbi thing? Or some
> >> ifdefs?
> >
> > No, the iosf_mbi header defines empty inline versions of
> > iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
> > this does mean that iosf_mbi must be builtin if the i915
> > driver is. I'll add:
> >
> > depends on DRM_I915=IOSF_MBI || IOSF_MBI=y
> >
> > To the i915 Kconfig to enforce this.
> 
>  Hmm, ok so that does not work (long cyclic dependency through the
>  selection of ACPI_VIDEO).
> 
>  So I've now added this instead:
> 
>   # iosf_mbi needs to be builtin if we are builtin
>   select IOSF_MBI if DRM_I915=y
> >>>
> >>> That's probably not going to help anyone since i915 is usually a module.
> >>
> >> Right, that is fine, then either the IOSF_MBI symbols are available,
> >> or IOSF_MBI is disabled and we get the inline 

Re: [PATCH v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings

[Hans de Goede]

Hi,

On 30-01-17 16:11, Ville Syrjälä wrote:

On Mon, Jan 30, 2017 at 04:02:19PM +0100, Hans de Goede wrote:

Hi,

On 30-01-17 14:10, Ville Syrjälä wrote:

On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:

Hi,

On 01/28/2017 05:25 PM, Hans de Goede wrote:

Hi,

On 01/27/2017 02:51 PM, Ville Syrjälä wrote:

On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:

Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
around P-Unit write accesses.


Can't we just stuff the calls into the actual punit write function
rather than sprinkling them all over the place?


punit access is acquired across sections like this:

iosf_mbi_punit_acquire();

val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
val &= ~DSPFREQGUAR_MASK;
val |= (cmd << DSPFREQGUAR_SHIFT);
vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
  DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
 50)) {
DRM_ERROR("timed out waiting for CDclk change\n");
}
iosf_mbi_punit_release();

Where we want to wait for the requested change to have taken
effect before releasing the punit.


Hmm. That's somewhat unfortunate. It also highlights a problem with the
patch wrt. RPS. We don't wait for the GPU to actually change frequencies
in set_rps() because that would slow things down too much. So I have to
wonder how much luck is needed to make this workaround really effective.


So the history of this patch-set is that I wrote this patch before
writing the patch to get FORCEWAKE_ALL before the pmic bus becomes
active (patch 12/13). Since a lot of testing was done with this
patch included in the patch-set and since it seemed a good idea
regardless (given my experience with accessing the punit vs
pmic bus accesses) I decided to leave it in.

Possibly just the patch to get FORCEWAKE_ALL is enough, that one
actually fixed things for me. That is also why I made this the
last patch in the set. I asked tagorereddy to test his system
without this patch, but he did not get around to that.

After all we do tell the punit to not touch the bus by acquiring
the pmic bus semaphore from i2c-desigware-baytrail.c, so maybe
for RPS freq changes it honors that and properly waits. Maybe it
honors that for all punit requests i915 does and the only real
problem is the forcewake stuff ?

I can try to drop this patch from my queue and run without it
for a while and see if things don't regress. And also ask
tagorereddy again to test his system that way.

Does that (dropping this patch for now) sound like a good idea?


More test results couldn't hurt at least. It also makes me wonder if
just bumping the timeouts to some ridiculously high value would fix
the problem as well.


I've already tried bumping the forcewake timeout from 50 to 250ms,
before writing the patch to just get forcewake_all before the pmic
bus access begins, that does not fix things, and since we busy wait
for this timeout from non-sleeping context 250ms already is way too
high.


+ a comment would be nice why it's there.


I will add comments to the acquire calls.


Do we need a kconfig select/depends on the iosf_mbi thing? Or some
ifdefs?


No, the iosf_mbi header defines empty inline versions of
iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
this does mean that iosf_mbi must be builtin if the i915
driver is. I'll add:

depends on DRM_I915=IOSF_MBI || IOSF_MBI=y

To the i915 Kconfig to enforce this.


Hmm, ok so that does not work (long cyclic dependency through the
selection of ACPI_VIDEO).

So I've now added this instead:

# iosf_mbi needs to be builtin if we are builtin
select IOSF_MBI if DRM_I915=y


That's probably not going to help anyone since i915 is usually a module.


Right, that is fine, then either the IOSF_MBI symbols are available,
or IOSF_MBI is disabled and we get the inline nops from the header.

The problem scenario is DRM_I915=y and IOSF_MBI=m, which is not very
realistic IMHO, but will get triggered by the random-config testing
several contributors do and lead to an unresolved symbol error there.


Well, from the user POV anything with IOSF_MBI==n can be a problem.
So I'm not sure if we should allow that.


So you're suggesting we just add an unconditional "select IOSF_MBI"
to the i915 Kconfig entry?

Regards,

Hans




BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede 
Tested-by: tagorereddy 
---
Changes in v2:
-Spelling: P-Unit, PMIC
-Adjust for iosf_mbi_punit_lock/_unlock to _acquire/_release rename
---
 drivers/gpu/drm/i915/intel_display.c| 6 ++
 drivers/gpu/drm/i915/intel_pm.c | 9 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +
 3 files changed, 24 insertions(+)

Re: [PATCH v3 24/24] drm/rockchip: dw-mipi-dsi: support read commands

[Sean Paul]

On Sun, Jan 29, 2017 at 01:24:44PM +, John Keeping wrote:
> I haven't found any method for getting the length of a response, so this
> just uses the requested rx_len
> 
> Signed-off-by: John Keeping 
> ---
> v3:
> - Fix checkpatch warnings
> Unchanged in v2
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 56 
> ++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index cf3ca6b0cbdb..cc58ada75425 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -678,6 +678,56 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi 
> *dsi,
>   return dw_mipi_dsi_gen_pkt_hdr_write(dsi, hdr_val);
>  }
>  
> +static int dw_mipi_dsi_dcs_read(struct dw_mipi_dsi *dsi,
> + const struct mipi_dsi_msg *msg)
> +{
> + const u8 *tx_buf = msg->tx_buf;
> + u8 *rx_buf = msg->rx_buf;
> + size_t i;
> + int ret, val;
> +
> + dsi_write(dsi, DSI_PCKHDL_CFG, EN_CRC_RX | EN_ECC_RX | EN_BTA);
> + dsi_write(dsi, DSI_GEN_HDR,
> +   GEN_HDATA(tx_buf[0]) | GEN_HTYPE(msg->type));
> +
> + ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> +  val, !(val & GEN_RD_CMD_BUSY), 1000,
> +  CMD_PKT_STATUS_TIMEOUT_US);
> + if (ret < 0) {
> + dev_err(dsi->dev, "failed to read command response\n");
> + return ret;
> + }
> +
> + for (i = 0; i < msg->rx_len;) {
> + u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
> +
> + while (i < msg->rx_len) {
> + rx_buf[i] = pld & 0xff;
> + pld >>= 8;
> + i++;
> + }
> + }

AFAICT, the outer for loop just initializes i and ensures msg->rx_len is
non-zero? 

I think the following would be easier to read (and safe against the case where
msg->rx_len > sizeof(pld) (even though this shouldn't happen according to DCS
spec)).

if (msg->rx_len > 0) {
u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
memcpy(rx_buf, , MIN(msg->rx_len, sizeof(pld));
}


> +
> + return msg->rx_len;
> +}
> +
> +static int dw_mipi_dsi_set_max_return_packet_size(struct dw_mipi_dsi *dsi,
> +   size_t len)
> +{
> + u8 val[] = { len & 0xff, (len >> 8) & 0xff };
> + struct mipi_dsi_msg msg = {
> + .channel = dsi->channel,
> + .type = MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE,
> + .tx_buf = val,
> + .tx_len = 2,
> + };
> +
> + if (len > 0x)
> + return -EINVAL;
> +
> + return dw_mipi_dsi_dcs_short_write(dsi, );
> +}
> +
>  static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
>const struct mipi_dsi_msg *msg)
>  {
> @@ -695,6 +745,12 @@ static ssize_t dw_mipi_dsi_host_transfer(struct 
> mipi_dsi_host *host,
>   case MIPI_DSI_DCS_LONG_WRITE:
>   ret = dw_mipi_dsi_dcs_long_write(dsi, msg);
>   break;
> + case MIPI_DSI_DCS_READ:
> + ret = dw_mipi_dsi_set_max_return_packet_size(dsi, msg->rx_len);
> + if (ret < 0)
> + return ret;
> + ret = dw_mipi_dsi_dcs_read(dsi, msg);
> + break;
>   default:
>   dev_err(dsi->dev, "unsupported message type 0x%02x\n",
>   msg->type);
> -- 
> 2.11.0.197.gb556de5.dirty
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/i915: Skip modeset locking when atomic pageflips are used.

[Maarten Lankhorst]

With the atomic helper for pageflips there are no CS flips when
that require resetting, so on most platforms we can completely
skip the locking.

Because we may end up reverting the page_flip patch add an explicit
function pointer check so that if someone reverts the page flip patch
there will not be any issues if this is forgotten.

Signed-off-by: Maarten Lankhorst 
Testcase: kms_busy.extended-modeset-hang-oldfb-*
---
This is a standalone patch to fix modeset hangs on g4x+.

The path for gen4 and lower is simulated in 
kms_busy.extended-modeset-hang-oldfb-with-reset
and still fails.

 drivers/gpu/drm/i915/intel_display.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index d8db1caec1b8..1dd480a6752a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3529,6 +3529,8 @@ static bool gpu_reset_clobbers_display(struct 
drm_i915_private *dev_priv)
INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv);
 }
 
+static const struct drm_crtc_funcs intel_crtc_funcs;
+
 void intel_prepare_reset(struct drm_i915_private *dev_priv)
 {
struct drm_device *dev = _priv->drm;
@@ -3536,6 +3538,11 @@ void intel_prepare_reset(struct drm_i915_private 
*dev_priv)
struct drm_atomic_state *state;
int ret;
 
+   if (intel_crtc_funcs.page_flip == drm_atomic_helper_page_flip &&
+   !i915.force_reset_modeset_test &&
+   !gpu_reset_clobbers_display(dev_priv))
+   return;
+
/*
 * Need mode_config.mutex so that we don't
 * trample ongoing ->detect() and whatnot.
@@ -3584,6 +3591,11 @@ void intel_finish_reset(struct drm_i915_private 
*dev_priv)
struct drm_atomic_state *state = dev_priv->modeset_restore_state;
int ret;
 
+   if (intel_crtc_funcs.page_flip == drm_atomic_helper_page_flip &&
+   !i915.force_reset_modeset_test &&
+   !gpu_reset_clobbers_display(dev_priv))
+   return;
+
/*
 * Flips in the rings will be nuked by the reset,
 * so complete all pending flips so that user space
-- 
2.7.4


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings

[Ville Syrjälä]

On Mon, Jan 30, 2017 at 04:02:19PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 30-01-17 14:10, Ville Syrjälä wrote:
> > On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 01/28/2017 05:25 PM, Hans de Goede wrote:
> >>> Hi,
> >>>
> >>> On 01/27/2017 02:51 PM, Ville Syrjälä wrote:
>  On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:
> > Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
> > request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
> > around P-Unit write accesses.
> 
>  Can't we just stuff the calls into the actual punit write function
>  rather than sprinkling them all over the place?
> >>>
> >>> punit access is acquired across sections like this:
> >>>
> >>> iosf_mbi_punit_acquire();
> >>>
> >>> val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> >>> val &= ~DSPFREQGUAR_MASK;
> >>> val |= (cmd << DSPFREQGUAR_SHIFT);
> >>> vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
> >>> if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
> >>>   DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
> >>>  50)) {
> >>> DRM_ERROR("timed out waiting for CDclk change\n");
> >>> }
> >>> iosf_mbi_punit_release();
> >>>
> >>> Where we want to wait for the requested change to have taken
> >>> effect before releasing the punit.
> >
> > Hmm. That's somewhat unfortunate. It also highlights a problem with the
> > patch wrt. RPS. We don't wait for the GPU to actually change frequencies
> > in set_rps() because that would slow things down too much. So I have to
> > wonder how much luck is needed to make this workaround really effective.
> 
> So the history of this patch-set is that I wrote this patch before
> writing the patch to get FORCEWAKE_ALL before the pmic bus becomes
> active (patch 12/13). Since a lot of testing was done with this
> patch included in the patch-set and since it seemed a good idea
> regardless (given my experience with accessing the punit vs
> pmic bus accesses) I decided to leave it in.
> 
> Possibly just the patch to get FORCEWAKE_ALL is enough, that one
> actually fixed things for me. That is also why I made this the
> last patch in the set. I asked tagorereddy to test his system
> without this patch, but he did not get around to that.
> 
> After all we do tell the punit to not touch the bus by acquiring
> the pmic bus semaphore from i2c-desigware-baytrail.c, so maybe
> for RPS freq changes it honors that and properly waits. Maybe it
> honors that for all punit requests i915 does and the only real
> problem is the forcewake stuff ?
> 
> I can try to drop this patch from my queue and run without it
> for a while and see if things don't regress. And also ask
> tagorereddy again to test his system that way.
> 
> Does that (dropping this patch for now) sound like a good idea?

More test results couldn't hurt at least. It also makes me wonder if
just bumping the timeouts to some ridiculously high value would fix
the problem as well.

> 
>  + a comment would be nice why it's there.
> >>>
> >>> I will add comments to the acquire calls.
> >>>
>  Do we need a kconfig select/depends on the iosf_mbi thing? Or some
>  ifdefs?
> >>>
> >>> No, the iosf_mbi header defines empty inline versions of
> >>> iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
> >>> this does mean that iosf_mbi must be builtin if the i915
> >>> driver is. I'll add:
> >>>
> >>> depends on DRM_I915=IOSF_MBI || IOSF_MBI=y
> >>>
> >>> To the i915 Kconfig to enforce this.
> >>
> >> Hmm, ok so that does not work (long cyclic dependency through the
> >> selection of ACPI_VIDEO).
> >>
> >> So I've now added this instead:
> >>
> >># iosf_mbi needs to be builtin if we are builtin
> >>select IOSF_MBI if DRM_I915=y
> >
> > That's probably not going to help anyone since i915 is usually a module.
> 
> Right, that is fine, then either the IOSF_MBI symbols are available,
> or IOSF_MBI is disabled and we get the inline nops from the header.
> 
> The problem scenario is DRM_I915=y and IOSF_MBI=m, which is not very
> realistic IMHO, but will get triggered by the random-config testing
> several contributors do and lead to an unresolved symbol error there.

Well, from the user POV anything with IOSF_MBI==n can be a problem.
So I'm not sure if we should allow that.

> 
> Hmm, thinking about this, this hunk actually belongs in 12/13 as that
> is the first patch to use iosf_mbi functions.
> 
> Regards,
> 
> Hans
> 
> 
> 
> > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
> > Signed-off-by: Hans de Goede 
> > Tested-by: tagorereddy 
> > ---
> > Changes in v2:
> > -Spelling: P-Unit, PMIC
> > -Adjust for iosf_mbi_punit_lock/_unlock to _acquire/_release rename
> > ---
> >  

Re: [PATCH v2 0/2] drm: Add support for tiny LCD displays

[Noralf Trønnes]

Den 30.01.2017 09.44, skrev Daniel Vetter:

Hi Noralf,

On Fri, Jan 27, 2017 at 08:56:29PM +0100, Noralf Trønnes wrote:

This is an attempt at providing a DRM version of drivers/staging/fbtft.

The tinydrm library provides a very simplified view of DRM in particular
for tiny displays that has onboard video memory and is connected through
a slow bus like SPI/I2C.

Only core patches this time.


Noralf.


Changes since version 1:
- Add tinydrm.rst
- Set tdev->fbdev_cma=NULL on unregister (lastclose is called after that).

Hm, this sounds like a buglet in the drm framework ... how do we call
lastclose when the driver is disappearing? I do see a drm_lastclose call
at the beginning of drm_dev_unregister (which we might want to remove for
KMS drivers, it doesn't make much sense imo), but that shouldn't result in
troubles.


Ah, I see, I'm tearing down fbdev before unregistering drm.
That should be reversed.

static void tinydrm_unregister(struct tinydrm_device *tdev)
{
drm_crtc_force_disable_all(tdev->drm);

if (tdev->fbdev_cma) {
drm_fbdev_cma_fini(tdev->fbdev_cma);
tdev->fbdev_cma = NULL;
}

drm_dev_unregister(tdev->drm);
}


- Remove some DRM_DEBUG*()
- Write-combined memory has uncached reads, so speed up by copying/buffering
   one pixel line before conversion.

Hm, why are you using write-combining memory? Or is that needed so that
you can (if available) use hw spi engines?


That comes with using the CMA helper:
drm_fbdev_cma_create_with_funcs() -> drm_gem_cma_create() -> dma_alloc_wc()

Here's a comment I have added in the code for why I set the DMA mask on
the SPI device and why it will work:

/*
 * Even though it's not the SPI device that does DMA (the master does),
 * the dma mask is necessary for the dma_alloc_wc() in
 * drm_gem_cma_create(). The dma_addr returned will be a physical
 * adddress which might be different from the bus address, but this is
 * not a problem since the address will not be used.
 * The virtual address is used in the transfer and the SPI core
 * re-maps it on the SPI master device using the DMA streaming API
 * (spi_map_buf()).
 */


Either way, I think this all looks good, pls submit a pull request to Dave
with these two patches as soon as latest drm-misc has landed (I'll send a
pull request for that later today).


I'm confused, I thought you wanted the core patches through drm-misc
and the rest as a pull request to Dave.

This is what you said:

Looks all pretty. A bunch of ideas below, but all optional. For 
merging I
think simplest to first get the core patches in through drm-misc, 
and then
you can submit a pull request to Dave for tinydrm+backends (just 
needs an

ack for the dt parts from dt maintainers), including MAINTAINERS entry.
Ack from my side.


Another one: Do you want to maintain tinydrm as part of the drm-misc
group, i.e. want commit rights there? That would also help a bit with
pushing all your great drm refactoring patches through the machinery ...


My goal is to port staging/fbtft to drm. Whether or not I will do work
in drm core (refactoring) besides the tinydrm drivers when that is
accomplished, I don't know. Working on drm as a hobbyist is very
difficult (for me at least) because it is very complex, it changes all
the time and on top of that it has a high volume mailinglist.
It takes effort to keep up.

So I will probably end up doing only tinydrm maintanence.
As for how that is best done, I don't know. Once I have covered a 3-4
controller types, a new driver is just a copy of a similar one with a
different register initialization. This means that it's easy to review
patches. They will all look the same, more or less.
So for me it's ofc best if I can review the patches and then commit
them myself. As for my own patches, if I need that tit for tat
reviewing to get them in, it will be difficult for me to review other
drivers because I have no idea how a GPU operates, and to keep up with
drm best pratices will be next to impossible for me.

If the Device Tree guys allows me to add fbtft support to tinydrm, then
there won't be much activity once the fbtft drivers have been ported
over. The uncertainty stems from the fact that the fbtft drivers are
written as controller drivers, but they contain panel specific things
like register setup and how to do rotation.
So compatible = "fb_ili9341", supports a controller with a specific
panel, but it can support other panels through the 'init' DT property
which encodes register values and delays (which is a no-no).

Noralf.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings

[Hans de Goede]

Hi,

On 30-01-17 14:10, Ville Syrjälä wrote:

On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:

Hi,

On 01/28/2017 05:25 PM, Hans de Goede wrote:

Hi,

On 01/27/2017 02:51 PM, Ville Syrjälä wrote:

On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:

Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
around P-Unit write accesses.


Can't we just stuff the calls into the actual punit write function
rather than sprinkling them all over the place?


punit access is acquired across sections like this:

iosf_mbi_punit_acquire();

val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
val &= ~DSPFREQGUAR_MASK;
val |= (cmd << DSPFREQGUAR_SHIFT);
vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
  DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
 50)) {
DRM_ERROR("timed out waiting for CDclk change\n");
}
iosf_mbi_punit_release();

Where we want to wait for the requested change to have taken
effect before releasing the punit.


Hmm. That's somewhat unfortunate. It also highlights a problem with the
patch wrt. RPS. We don't wait for the GPU to actually change frequencies
in set_rps() because that would slow things down too much. So I have to
wonder how much luck is needed to make this workaround really effective.


So the history of this patch-set is that I wrote this patch before
writing the patch to get FORCEWAKE_ALL before the pmic bus becomes
active (patch 12/13). Since a lot of testing was done with this
patch included in the patch-set and since it seemed a good idea
regardless (given my experience with accessing the punit vs
pmic bus accesses) I decided to leave it in.

Possibly just the patch to get FORCEWAKE_ALL is enough, that one
actually fixed things for me. That is also why I made this the
last patch in the set. I asked tagorereddy to test his system
without this patch, but he did not get around to that.

After all we do tell the punit to not touch the bus by acquiring
the pmic bus semaphore from i2c-desigware-baytrail.c, so maybe
for RPS freq changes it honors that and properly waits. Maybe it
honors that for all punit requests i915 does and the only real
problem is the forcewake stuff ?

I can try to drop this patch from my queue and run without it
for a while and see if things don't regress. And also ask
tagorereddy again to test his system that way.

Does that (dropping this patch for now) sound like a good idea?


+ a comment would be nice why it's there.


I will add comments to the acquire calls.


Do we need a kconfig select/depends on the iosf_mbi thing? Or some
ifdefs?


No, the iosf_mbi header defines empty inline versions of
iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
this does mean that iosf_mbi must be builtin if the i915
driver is. I'll add:

depends on DRM_I915=IOSF_MBI || IOSF_MBI=y

To the i915 Kconfig to enforce this.


Hmm, ok so that does not work (long cyclic dependency through the
selection of ACPI_VIDEO).

So I've now added this instead:

# iosf_mbi needs to be builtin if we are builtin
select IOSF_MBI if DRM_I915=y


That's probably not going to help anyone since i915 is usually a module.


Right, that is fine, then either the IOSF_MBI symbols are available,
or IOSF_MBI is disabled and we get the inline nops from the header.

The problem scenario is DRM_I915=y and IOSF_MBI=m, which is not very
realistic IMHO, but will get triggered by the random-config testing
several contributors do and lead to an unresolved symbol error there.

Hmm, thinking about this, this hunk actually belongs in 12/13 as that
is the first patch to use iosf_mbi functions.

Regards,

Hans




BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede 
Tested-by: tagorereddy 
---
Changes in v2:
-Spelling: P-Unit, PMIC
-Adjust for iosf_mbi_punit_lock/_unlock to _acquire/_release rename
---
 drivers/gpu/drm/i915/intel_display.c| 6 ++
 drivers/gpu/drm/i915/intel_pm.c | 9 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +
 3 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5604701..13e5152 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -47,6 +47,7 @@
 #include 
 #include 
 #include 
+#include 

 static bool is_mmio_work(struct intel_flip_work *work)
 {
@@ -6421,6 +6422,8 @@ static void valleyview_set_cdclk(struct drm_device *dev, 
int cdclk)
 cmd = 0;

 mutex_lock(_priv->rps.hw_lock);
+iosf_mbi_punit_acquire();
+
 val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
 val &= ~DSPFREQGUAR_MASK;
 val |= (cmd << DSPFREQGUAR_SHIFT);
@@ 

Re: [Intel-gfx] [PATCH 0/3] drm/i915: Handle hanging during nonblocking modeset correctly.

[Maarten Lankhorst]

Op 30-01-17 om 09:17 schreef Daniel Vetter:
> On Fri, Jan 27, 2017 at 03:08:45PM +, Chris Wilson wrote:
>> On Fri, Jan 27, 2017 at 03:58:08PM +0100, Daniel Vetter wrote:
>>> On Fri, Jan 27, 2017 at 02:31:55PM +, Chris Wilson wrote:
 On Fri, Jan 27, 2017 at 03:21:29PM +0100, Daniel Vetter wrote:
> On Fri, Jan 27, 2017 at 09:30:50AM +, Chris Wilson wrote:
>> On Thu, Jan 26, 2017 at 04:59:21PM +0100, Maarten Lankhorst wrote:
>>> When writing some testcases for nonblocking modesets. I found out that 
>>> the
>>> infinite wait on the old fb was causing issues.
>> The crux of the issue here is the locked wait for old dependencies and
>> the inability to inject the intel_prepare_reset disabling of all planes.
>> There are a couple of locked waits on struct_mutex within the modeset
>> locks for intel_overlay and if we happen to be using the display plane
>> for the first time.
>>
>> The first I suggested solving using fences to track dependencies and
>> keep the order between atomic states. Cancelling the outstanding
>> modesets, replacing with a disable and then on restore jumping to the
>> final state look doable. It also requires avoiding the struct_mutex for
>> disabling, which is quite easy. To avoid the wait under struct_mutex,
>> we've talked about switching to mmio, but for starters we could move the
>> wait from inside intel_overlay into the fence for the atomic operation.
>> (But's that a little more surgery than we would like for intel_overlay I
>> guess - dig out Ville's patches for overlay planes?) And to prevent the
>> wait under struct_mutex for pin_to_display_plane, my plane is to move
>> that to an async fenced operation that is then naturally waited upon by
>> the atomic modeset.
> A bit more a hack, but a different idea, and I think hack for gen234.0 is
> ok:
>
> We complete all the requests before we start the hw reset with fence.error
> = -EIO. But we do this only when we need to get at the display locks. A
> slightly more elegant solution would be to trylock modeset locks, and if
> one of them fails (and only then) complete all requests with -EIO to get
> the concurrent modeset to proceed before we reset the hardware. That's
> essentially the logic we had before all the reworks, and it worked. But I
> didn't look at how scary that all would be to make it work again ...
 The modeset lock may not just be waiting on our requests (even on pnv we
 can expect that there are already users celebrating that pnv+nouveau
 finally works ;) and that the display is not the only user/observer of
 those requests. Using the requests to break the modeset lock just feels
 like the wrong approach.
>>> It's a cycle, and we need to break it somewhere. Another option might be
>>> to break the cycle the same way we do it for gem locks: Wake up everyone
>>> and restart the modeset ioctl. Since the trouble only happens for
>>> synchronous modesets where we hold the locks while waiting for fences, we
>>> can also break out of that and restart. And I also don't think that would
>>> leak to other drivers, after all our gem locking restart dances also don't
>>> leak to other drivers - it's just our own driver's lock which are affected
>>> by these special wakupe semantics.
>> It's a queue of nonblocking modesets that we need to worry about, afaik.
>> Moving the wait for blocking modeset outside of modeset lock is easily
>> achievable (and avoiding the other waits under both the modeset + 
>> struct_mutex I have at least an idea for). So the challenge is how to
>> inject all-planes-off for gen3 and then allow the queue to continue again
>> afterwards.
> Hm right, I missed the nonblocking updates which don't take locks. But
> assuming we do the display reset for gpu resets as a full modeset (i.e.
> going through ->atomic_commit) it should still work out correctly:
>
> Starting state: gpu is hung, nonblocking modeset waiting for some requests
> to complete.
Missing one evil detail here, else things would have moved forward..

A unrelated thread performs a blocking commit, and holds all locks until the 
nonblocking modeset completes.
> 1. hangcheck kicks in, fires off reset work.
>
> 2. We complete all requests with fence.error = -EIO and wake up any
> waiters. That means no re-queueing for older platforms, but oh well.
>
> 3. We grab all the display locks. Nothing happens yet.
>
> 4. We reset the chip, display dies.
>
> 5. We run ->atomic_commit to restore things. This will also force the
> nonblocking commit worker to complete before this display restore touches
> anything.
>
> The only trouble I see is that the nonblocking worker can still touch the
> display block while we kill it, which isn't awesome. But we can fix that
> by waiting for all pending nonblocking commits in step 3 manually (without
> calling into atomic_commit), as long as we do step 2.
>
> 

[Bug 193651] New: Amdgpu error messages at boot with Amd RX460

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=193651

Bug ID: 193651
   Summary: Amdgpu error messages at boot with Amd RX460
   Product: Drivers
   Version: 2.5
Kernel Version: 4.11-wip
  Hardware: x86-64
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: low
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-...@kernel-bugs.osdl.org
  Reporter: fin4...@hotmail.com
Regression: No

Created attachment 253581
  --> https://bugzilla.kernel.org/attachment.cgi?id=253581=edit
dmesg logfile

I have Gigabyte RX460 2GB gpu card, Debian testing Xfce and adg5f
drm-next-4.11-wip kernel downloaded and compiled as today. Computer works ok
but the dmesg command shows the following boot errors that might interest
amdgou driver developers. Mounting my home partiton fails amdgpu IB tests:

[7.001953] [drm] ib test on ring 12 succeeded
[7.055163] EXT4-fs (sda5): mounted filesystem with ordered data mode. Opts:
(null)
[8.011874] [drm:0xa01360ce] *ERROR* amdgpu: IB test timed out.
[8.011910] [drm:0xa00e1b4b] *ERROR* amdgpu: failed testing IB on
ring 13 (-110).
[8.011943] [drm:0xa00be574] *ERROR* ib ring test failed (-110).


Some powerplay errors:
[4.888584] amdgpu: [powerplay] [AVFS] Something is broken. See log!
[4.891452] amdgpu: [powerplay] Can't find requested voltage id in
vdd_dep_on_sclk table!
[4.894807] amdgpu: [powerplay] 
failed to send message 309 ret is 254 
[4.894824] amdgpu: [powerplay] 
failed to send pre message 14e ret is 254 


Bios recognition errors:
[4.729628] [drm] BIOS signature incorrect 20 7
[4.729635] amdgpu :01:00.0: Invalid PCI ROM header signature: expecting
0xaa55, got 0x

-- 
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

Re: DRM Atomic property for color-space conversion

[Ville Syrjälä]

On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote:
> Hi,
> 
> We're looking to enable the per-plane color management hardware in
> Mali-DP with atomic properties, which has sparked some conversation
> around how to handle YCbCr formats.
> 
> As it stands today, it's assumed that a driver will implicitly "do the
> right thing" to display a YCbCr buffer.
> 
> YCbCr data often uses different gamma curves and signal ranges (e.g.
> BT.609, BT.701, BT.2020, studio range, full-range), so its desirable
> to be able to explicitly control the YCbCr to RGB conversion process
> from userspace.
> 
> We're proposing adding a "CSC" (color-space conversion) property to
> control this - primarily per-plane for framebuffer->pipeline CSC, but
> perhaps one per CRTC too for devices which have an RGB pipeline and
> want to output in YUV to the display:
> 
> Name: "CSC"
> Type: ENUM | ATOMIC;
> Enum values (representative):
> "default":
>   Same behaviour as now. "Some kind" of YCbCr->RGB conversion
>   for YCbCr buffers, bypass for RGB buffers
> "disable":
>   Explicitly disable all colorspace conversion (i.e. use an
>   identity matrix).
> "YCbCr to RGB: BT.709":
>   Only valid for YCbCr formats. CSC in accordance with BT.709
>   using [16..235] for (8-bit) luma values, and [16..240] for
>   8-bit chroma values. For 10-bit formats, the range limits are
>   multiplied by 4.
> "YCbCr to RGB: BT.709 full-swing":
>   Only valid for YCbCr formats. CSC in accordance with BT.709,
>   but using the full range of each channel.
> "YCbCr to RGB: Use CTM":*
>   Only valid for YCbCr formats. Use the matrix applied via the
>   plane's CTM property
> "RGB to RGB: Use CTM":*
>   Only valid for RGB formats. Use the matrix applied via the
>   plane's CTM property
> "Use CTM":*
>   Valid for any format. Use the matrix applied via the plane's
>   CTM property
> ... any other values for BT.601, BT.2020, RGB to YCbCr etc. etc. as
> they are required.

Having some RGB2RGB and YCBCR2RGB things in the same property seems
weird. I would just go with something very simple like:

YCBCR_TO_RGB_CSC:
* BT.601
* BT.709
* custom matrix

And trying to use the same thing for the crtc stuff is probably not
going to end well. Like Daniel said we already have the
'Broadcast RGB' property muddying the waters there, and that stuff
also ties in with what colorspace we signal to the sink via
infoframes/whatever the DP thing was called. So my gut feeling is
that trying to use the same property everywhere will just end up
messy.

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] MAINTAINERS: add dma-fence* files to Sync File maintainership

[Sumit Semwal]

Hi Gustavo, Daniel,

On 30 January 2017 at 14:07, Daniel Vetter  wrote:

> On Fri, Jan 27, 2017 at 03:54:44PM -0200, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> >
> > As Sync File is highly dependent on dma-fence* tracks it
> > under SYNC FILE_FRAMEWORK as well.
> >
> > Signed-off-by: Gustavo Padovan 
>
> Acked-by: Daniel Vetter 
>
> Of course, feel free to add my
Acked-by: Sumit Semwal 


> I guess wait for an ack from Sumit, then push.
> -Daniel
> > ---
> >  MAINTAINERS | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index bdc4843..c1c000d 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3976,6 +3976,7 @@ S:  Maintained
> >  L:   linux-me...@vger.kernel.org
> >  L:   dri-devel@lists.freedesktop.org
> >  F:   drivers/dma-buf/sync_*
> > +F:   drivers/dma-buf/dma-fence*
> >  F:   drivers/dma-buf/sw_sync.c
> >  F:   include/linux/sync_file.h
> >  F:   include/uapi/linux/sync_file.h
> > --
> > 2.9.3
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>



-- 
Thanks and regards,

Sumit Semwal
Linaro Mobile Group - Kernel Team Lead
Linaro.org │ Open source software for ARM SoCs
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings

[Ville Syrjälä]

On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 01/28/2017 05:25 PM, Hans de Goede wrote:
> > Hi,
> >
> > On 01/27/2017 02:51 PM, Ville Syrjälä wrote:
> >> On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:
> >>> Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
> >>> request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
> >>> around P-Unit write accesses.
> >>
> >> Can't we just stuff the calls into the actual punit write function
> >> rather than sprinkling them all over the place?
> >
> > punit access is acquired across sections like this:
> >
> > iosf_mbi_punit_acquire();
> >
> > val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> > val &= ~DSPFREQGUAR_MASK;
> > val |= (cmd << DSPFREQGUAR_SHIFT);
> > vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
> > if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
> >   DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
> >  50)) {
> > DRM_ERROR("timed out waiting for CDclk change\n");
> > }
> > iosf_mbi_punit_release();
> >
> > Where we want to wait for the requested change to have taken
> > effect before releasing the punit.

Hmm. That's somewhat unfortunate. It also highlights a problem with the
patch wrt. RPS. We don't wait for the GPU to actually change frequencies
in set_rps() because that would slow things down too much. So I have to
wonder how much luck is needed to make this workaround really effective.

> >
> >>
> >> + a comment would be nice why it's there.
> >
> > I will add comments to the acquire calls.
> >
> >> Do we need a kconfig select/depends on the iosf_mbi thing? Or some
> >> ifdefs?
> >
> > No, the iosf_mbi header defines empty inline versions of
> > iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
> > this does mean that iosf_mbi must be builtin if the i915
> > driver is. I'll add:
> >
> > depends on DRM_I915=IOSF_MBI || IOSF_MBI=y
> >
> > To the i915 Kconfig to enforce this.
> 
> Hmm, ok so that does not work (long cyclic dependency through the
> selection of ACPI_VIDEO).
> 
> So I've now added this instead:
> 
>   # iosf_mbi needs to be builtin if we are builtin
>   select IOSF_MBI if DRM_I915=y

That's probably not going to help anyone since i915 is usually a module.

> 
> Regards,
> 
> Hans
> 
> 
> >>> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
> >>> Signed-off-by: Hans de Goede 
> >>> Tested-by: tagorereddy 
> >>> ---
> >>> Changes in v2:
> >>> -Spelling: P-Unit, PMIC
> >>> -Adjust for iosf_mbi_punit_lock/_unlock to _acquire/_release rename
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_display.c| 6 ++
> >>>  drivers/gpu/drm/i915/intel_pm.c | 9 +
> >>>  drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +
> >>>  3 files changed, 24 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> >>> b/drivers/gpu/drm/i915/intel_display.c
> >>> index 5604701..13e5152 100644
> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -47,6 +47,7 @@
> >>>  #include 
> >>>  #include 
> >>>  #include 
> >>> +#include 
> >>>
> >>>  static bool is_mmio_work(struct intel_flip_work *work)
> >>>  {
> >>> @@ -6421,6 +6422,8 @@ static void valleyview_set_cdclk(struct drm_device 
> >>> *dev, int cdclk)
> >>>  cmd = 0;
> >>>
> >>>  mutex_lock(_priv->rps.hw_lock);
> >>> +iosf_mbi_punit_acquire();
> >>> +
> >>>  val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> >>>  val &= ~DSPFREQGUAR_MASK;
> >>>  val |= (cmd << DSPFREQGUAR_SHIFT);
> >>> @@ -6430,6 +6433,7 @@ static void valleyview_set_cdclk(struct drm_device 
> >>> *dev, int cdclk)
> >>>   50)) {
> >>>  DRM_ERROR("timed out waiting for CDclk change\n");
> >>>  }
> >>> +iosf_mbi_punit_release();
> >>>  mutex_unlock(_priv->rps.hw_lock);
> >>>
> >>>  mutex_lock(_priv->sb_lock);
> >>> @@ -6497,6 +6501,7 @@ static void cherryview_set_cdclk(struct drm_device 
> >>> *dev, int cdclk)
> >>>  cmd = DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, cdclk) - 1;
> >>>
> >>>  mutex_lock(_priv->rps.hw_lock);
> >>> +iosf_mbi_punit_acquire();
> >>>  val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> >>>  val &= ~DSPFREQGUAR_MASK_CHV;
> >>>  val |= (cmd << DSPFREQGUAR_SHIFT_CHV);
> >>> @@ -6506,6 +6511,7 @@ static void cherryview_set_cdclk(struct drm_device 
> >>> *dev, int cdclk)
> >>>   50)) {
> >>>  DRM_ERROR("timed out waiting for CDclk change\n");
> >>>  }
> >>> +iosf_mbi_punit_release();
> >>>  mutex_unlock(_priv->rps.hw_lock);
> >>>
> >>>  intel_update_cdclk(dev_priv);
> >>> diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> >>> b/drivers/gpu/drm/i915/intel_pm.c
> >>> index 249623d..adff84a 100644
> >>> --- 

[Bug 99368] Full aspect scaling introduces interlacing on specific resolutions

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=99368

Platin  changed:

   What|Removed |Added

 Attachment #128898|0   |1
is obsolete||

--- Comment #5 from Platin  ---
Created attachment 129229
  --> https://bugs.freedesktop.org/attachment.cgi?id=129229=edit
Correct Xorg log

Didn't know that GDM doesn't store the Xorg log in the usual location. So I
accidentally uploaded an obsolete log. Replaced it with the correct log.

I tried lower resolutions with the amdgpu driver and the problem shows even
there. So I'm not sure if the problem exists on both drivers or if the source
of the problem lies elsewhere.

Does anyone have an idea?

-- 
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] Gpu: drm: exynos - Fix possible NULL derefrence.

[Javier Martinez Canillas]

Hello Shailendra,

The subject line is wrong, please always use the convention used in
previous commits, i.e: git log --oneline drivers/gpu/drm/exynos/

On 01/30/2017 02:02 AM, Shailendra Verma wrote:
> of_device_get_match_data could return NULL, and so can cause
> a NULL pointer dereference later.
> 
> Signed-off-by: Shailendra Verma 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |4 
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |4 
>  drivers/gpu/drm/exynos/exynos_hdmi.c |4 
>  drivers/gpu/drm/exynos/exynos_mixer.c|4 
>  4 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index e07cb1f..fba0ffc 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1765,6 +1765,10 @@ static int exynos_dsi_probe(struct platform_device 
> *pdev)
>  
>   dsi->dev = dev;
>   dsi->driver_data = of_device_get_match_data(dev);
> + if (!dsi->driver_data) {
> + dev_err(dev, "no device match found\n");
> + return -ENODEV;
> + }
>

I don't think this makes sense for the Exynos driver since Exynos is a
DT-only platform and so the probe callback can only be called if a dev
node with a compatible string in the OF device id table was registered.

All the struct of_device_id in the table have a .data and so this can't
be NULL in this driver.
 
Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes()

[Jyri Sarha]

On 01/30/17 13:15, Laurent Pinchart wrote:
>>> rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and
>>> zpos" ?
>> Thanks, I'll do that.
> If you intend on merging this patch as a v4.10 fix then there's no need to 
> rebase. If it targets v4.11, the above-mentioned series will likely go in 
> first.

Well perhaps this patch should go to v4.10 already. Because of the bug a
plane commit may fail if the screen is blanked and there is no pclk
available at the time the plane HW is configured.

Tomi, what do you think?

BR,
Jyri
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/6] Etnaviv cmdbuf suballocator

[Christian Gmeiner]

Hi Lucas,

2017-01-30 12:48 GMT+01:00 Lucas Stach :
> Am Mittwoch, den 18.01.2017, 12:25 +0100 schrieb Lucas Stach:
>> Hi all,
>>
>> the following patches introduce a cmduf suballocator in the Etnaviv
>> kernel driver, which has the following benefits:
>>
>> 1. Allocating and freeing a CMA buffer for each user command submission
>>is taking a big toll on the CPU, as CMA is not exactly low overhead.
>>By suballocating a single buffer we avoid all this overhead.
>>
>> 2. Less TLB flushes on MMUv2. Each time a new buffer gets mapped into
>>the GPU address space on MMUv2 the TLBs need to be flushed. Mapping
>>the suballocated area once allows to skip the TLB flushes (at least
>>as long as userspace re-uses existing buffers).
>>
>> 3. No workarounds for GC3000 required anymore. The FE TLB flush on
>>GC3000 doesn't work reliably, which required us to map the cmdbufs
>>into the GPU address space at specific positions, which also isn't
>>guaranteed to work if the address space is already crowded. Having
>>a single static area for the cmdbufs side-steps the erratum completely.
>>
>> If I can get reviews and/or enough testing, I would like to include this
>> code in kernel 4.11.
>
> If this helps in testing this change, I have pushed my queue out into my
> public git repo at:
>
> https://git.pengutronix.de/git/lst/linux drm-etnaviv-next
>
> The branch is based on v4.10-rc1, but is trivial to rebase onto
> something more recent.
>

Will have a look at it later the day.

greets
--
Christian Gmeiner, MSc

https://www.youtube.com/user/AloryOFFICIAL
https://soundcloud.com/christian-gmeiner
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel