Re: [Outreachy kernel] [PATCH] drm/gem: use new idr deletion interface to cleanup drm_gem_handle_delete()

2017-09-26 Thread Daniel Vetter 
On Tue, Sep 26, 2017 at 12:17:28AM +0530, Aishwarya Pant wrote:
> The IDR deletion interface now returns the deleted entry or NULL if it was not
> present. So we don't have to do the extra work of checking if we have a
> reference on the drm_gem_object, this can be handled by checking the return
> value from idr_remove() and the extra locks can be dropped.
> 
> Signed-off-by: Aishwarya Pant 

Haneen is already working on this task, with the patch just merged. Please
coordinate with mentors (me or Sean Paul) before starting on something to
avoid such clashes in the future.

Note also that some todo items are just ideas, and need confirmation with
relevant maintainers first.

Thanks, Daniel
> ---
>  drivers/gpu/drm/drm_gem.c | 21 ++---
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index c55f338..f62757a 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -282,29 +282,12 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
>  {
>   struct drm_gem_object *obj;
>  
> - /* This is gross. The idr system doesn't let us try a delete and
> -  * return an error code.  It just spews if you fail at deleting.
> -  * So, we have to grab a lock around finding the object and then
> -  * doing the delete on it and dropping the refcount, or the user
> -  * could race us to double-decrement the refcount and cause a
> -  * use-after-free later.  Given the frequency of our handle lookups,
> -  * we may want to use ida for number allocation and a hash table
> -  * for the pointers, anyway.
> -  */
>   spin_lock(>table_lock);
> -
> - /* Check if we currently have a reference on the object */
> - obj = idr_replace(>object_idr, NULL, handle);
> - spin_unlock(>table_lock);
> - if (IS_ERR_OR_NULL(obj))
> + obj = idr_remove(>object_idr, handle);
> + if (!obj)
>   return -EINVAL;
> -
>   /* Release driver's reference and decrement refcount. */
>   drm_gem_object_release_handle(handle, obj, filp);
> -
> - /* And finally make the handle available for future allocations. */
> - spin_lock(>table_lock);
> - idr_remove(>object_idr, handle);
>   spin_unlock(>table_lock);
>  
>   return 0;
> -- 
> 2.7.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20170925184728.GA8861%40aishwarya.
> For more options, visit https://groups.google.com/d/optout.

-- 
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: Kernel GPU subsytem: Better manual upload for atomic

2017-09-26 Thread Daniel Vetter 
On Tue, Sep 26, 2017 at 03:47:56PM +0530, Harsha Sharma wrote:
> Hello,
> I am working on task "Better manual upload for atomic" in kernel gpu
> subsystem.
> When duplicating the crtc state I need to change drm_rect x2 and y2 to
> MAX_INT.
> I didn't get what is MAX_INT over here. Also, how am I supposed to test my
> code?
> It will be really helpful if you can suggest something.
> Thanks a lot for your time.

This is a very involved task which is more appropriate for an entire
internship, not really good stuff for the application period.

Please chat with Sean or me on irc to pick a better task.

Thanks, 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 v2 2/2] drm/tilcdc: replace reference/unreference() with get/put

2017-09-26 Thread Daniel Vetter 
On Tue, Sep 26, 2017 at 12:18:06PM +0300, Jyri Sarha wrote:
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> On 09/26/17 11:30, Aishwarya Pant wrote:
> > For maintaining consistency with kernel coding style replace
> > reference/unreference in ref counting functions with get/put.
> > 
> > The following cocci script was used to generate the tilcdc patch:
> > 
> > @@
> > expression ex;
> > @@
> > 
> > (
> > -drm_framebuffer_unreference(ex);
> > +drm_framebuffer_put(ex);
> > |
> > -drm_dev_unref(ex);
> > +drm_dev_put(ex);
> > |
> > -drm_framebuffer_reference(ex);
> > +drm_framebuffer_get(ex);
> > )
> > 
> > Signed-off-by: Aishwarya Pant 
> 
> Acked-by: Jyri Sarha 
> 
> I guess this should go in via drm-misc at the same time with
> "drm: introduce drm_dev_{get/put} functions".

Yup, this one needs the previous one, both pushed to drm-misc-next.

Aishwarya, while reviewing your patches I've noticed that you've missed to
case of drm_dev_unref() in the drm core code, one in drm_pci.c and one in
drm_prime.c. Can you pls do a follow-up patch to address these two? Fixing
up the core completely is nice, drivers can be done later on (also by
others, this is a prefect newbies tasks). But making sure the core is
consistent is good I think.
-Daniel

> 
> Best regards,
> Jyri
> 
> 
> > ---
> > No changes in v2
> > 
> > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 6 +++---
> >  drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 2 +-
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
> > b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > index 406fe45..d2589f310 100644
> > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > @@ -75,7 +75,7 @@ static void unref_worker(struct drm_flip_work *work, void 
> > *val)
> > struct drm_device *dev = tilcdc_crtc->base.dev;
> >  
> > mutex_lock(>mode_config.mutex);
> > -   drm_framebuffer_unreference(val);
> > +   drm_framebuffer_put(val);
> > mutex_unlock(>mode_config.mutex);
> >  }
> >  
> > @@ -456,7 +456,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
> >  
> > set_scanout(crtc, fb);
> >  
> > -   drm_framebuffer_reference(fb);
> > +   drm_framebuffer_get(fb);
> >  
> > crtc->hwmode = crtc->state->adjusted_mode;
> >  }
> > @@ -633,7 +633,7 @@ int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
> > return -EBUSY;
> > }
> >  
> > -   drm_framebuffer_reference(fb);
> > +   drm_framebuffer_get(fb);
> >  
> > crtc->primary->fb = fb;
> > tilcdc_crtc->event = event;
> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
> > b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > index b0d70f9..74276ef 100644
> > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > @@ -225,7 +225,7 @@ static void tilcdc_fini(struct drm_device *dev)
> >  
> > pm_runtime_disable(dev->dev);
> >  
> > -   drm_dev_unref(dev);
> > +   drm_dev_put(dev);
> >  }
> >  
> >  static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
> > 
> 
> 
> ___
> 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

Re: [PATCH v2] drm/tinydrm: Move backlight helpers to a separate file

2017-09-26 Thread Meghana Madhyastha 
On Mon, Sep 25, 2017 at 06:31:58PM +0200, Noralf Trønnes wrote:
> 
> Den 25.09.2017 16.56, skrev Noralf Trønnes:
> >Hi Meghana,
> >
> >
> >Den 22.09.2017 17.09, skrev Meghana Madhyastha:
> >>Move backlight helpers from tinydrm-helpers.c to
> >>tinydrm-backlight.c. This is because it is organizationally
> >>simpler to understand and advantageous to group functions
> >>performing a similar function to a separate file as opposed to
> >>having one helper file with heteregenous helper functions.
> >>
> >>Signed-off-by: Meghana Madhyastha 
> >>---
> >
> >I don't think there is much gain in just moving the code like this.
> >
> >The idea is to add a drm_backlight helper that can be useful for all
> >DRM drivers that use the backlight subsystem.

Yes I agree. That definitely makes more sense.
> 
> The full path to that helper would be:
> drivers/gpu/drm/drm_backlight.c
> 
> >This is what the TODO says:
> >https://dri.freedesktop.org/docs/drm/gpu/todo.html#tinydrm
> >
> >- backlight helpers, probably best to put them into a new drm_backlight.c.
> >  This is because drivers/video is de-facto unmaintained. We could also
> >  move drivers/video/backlight to drivers/gpu/backlight and take it all
> >  over within drm-misc, but that’s more work.
> >
> >There is also this discussion to take into account:
> >KMS backlight ABI proposition
> >https://lists.freedesktop.org/archives/dri-devel/2017-February/133206.html
> >
> >
> >I don't remember what came out of that discussion.
> >
> >Noralf.

After having discussed this with Daniel on the #dri-devel irc channel, 
here are some of the points suggested.

Daniel suggested that I first look into the usage of shared backlight
helpers in drm (specifically backlight_update_status to begin with). The idea
was to see whether there is any pattern in usage and/or code dupication.
If there is, then the next step would be to write helper functions which
can be used by other drivers (and not just tinydrm). 

To start with, I went through the instances of backlight_update_status
in the drm code, and made the following observations(most of them are
very simple/naive observations).

- backlight_update_status is usually called in backlight init (and
  sometimes exit) functions of the drivers just after the device is registered.
  So backlight_update_status is called with the registered device as the
  parameter.

Here are the following cases of properties changed/set before 
backlight_update_status is called.

- CASE 1: Brightness is changed (either a macro BRIGHTNESS_MAX_LEVEL 100
  is defined or it is manually set) This happens in the following files:

  gma500/cdv_device.c, gma500/mdfld_device.c, gma500/oaktrail_device.c,
  gma500/psb_device.c, noveau/noveau_backlight.c(here brightness is determined 
by fuction
  static int nv50_get_intensity)

- CASE 2: Power property is set (to FB_BLANK_UNBLANK mostly)
  This happens in the following files:

  omapdrm/displays/panel-dpi.c, panel/panel-innolux-p079zca.c,
  panel/panel-jdi-lt070me05000.c, panel/panel-sharp-lq101r1sx01.c, 
  panel/panel-sharp-ls043t1le01.c, tilcdc/tilcdc_panel.c
  
- CASE 3: State is set
  This happens in the following files:

  tinydrm/tinydrm-helpers.c

- CASE 4: Power and brightness properties are set
  This happens in the following files:

  atombios_encoders.c, radeon/radeon_legacy_encoders.c, 
  shmobile/shmob_drm_backlight.c

- CASE 5: Power and the state properties are set
  This happens in the following files:

  panel/panel-lvds.c, panel/panel-panasonic-vvx10f034n00.c,
  panel/panel-simple.c, panel/panel-sitronix-st7789v.c

Please let me know if I am wrong / missed something. As for next steps,
wouldn't it be an overkill to have a separate helper function for each
of these cases ? Perhaps a generic helper function which would somehow
address these cases would be more appropriate ? Thank you for your
time/patience.

-Meghana

> >>Changes in v2:
> >>  -Improved commit message by explaining why the changes were made.
> >>
> >>  drivers/gpu/drm/tinydrm/core/Makefile    |   2 +-
> >>  drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c | 103
> >>+++
> >>  drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c   |  94
> >>-
> >>  drivers/gpu/drm/tinydrm/mi0283qt.c   |   1 +
> >>  drivers/gpu/drm/tinydrm/mipi-dbi.c   |   1 +
> >>  include/drm/tinydrm/tinydrm-backlight.h  |  18 
> >>  6 files changed, 124 insertions(+), 95 deletions(-)
> >>  create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c
> >>  create mode 100644 include/drm/tinydrm/tinydrm-backlight.h
> >>
> >>diff --git a/drivers/gpu/drm/tinydrm/core/Makefile
> >>b/drivers/gpu/drm/tinydrm/core/Makefile
> >>index fb221e6..389ca7a 100644
> >>--- a/drivers/gpu/drm/tinydrm/core/Makefile
> >>+++ b/drivers/gpu/drm/tinydrm/core/Makefile
> >>@@ -1,3 +1,3 @@
> >>-tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-helpers.o
> >>+tinydrm-y := tinydrm-core.o

Re: [Intel-gfx] [RFC V1 0/6] Add Plane Color Properties

2017-09-26 Thread Daniel Vetter 
On Tue, Sep 26, 2017 at 01:32:52PM +0530, Uma Shankar wrote:
> This patch series adds properties for plane color features. It adds
> properties for degamma used to linearize data, CSC used for gamut
> conversion, and gamma used to again non-linearize data as per panel
> supported color space. These can be utilize by user space to convert
> planes from one format to another, one color space to another etc.
> 
> Usersapce can take smart blending decisions and utilize these hardware
> supported plane color features to get accurate color profile. The same
> can help in consistent color quality from source to panel taking
> advantage of advanced color features in hardware.
> 
> These patches just add the property interfaces and enable helper functions.
> Based on community feedabck on this one, we can build up and add hardware
> specific implementation on top of this series.
> 
> Note: This is just to get a design feedback whether these interfaces look ok. 
> Once, designed is agreed will re-send the series with a hardware specific
> implementation along with IGT tests for plane color. 

What's missing from this is the property documentation for the userspace
abi, like we have for the pipe color manager stuff:

https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties

Otherwise looks like a reasonable series, but the real challenges here is
properly enabling this in a HDR (or at least color space) aware compositor.
-Daniel

> 
> Uma Shankar (6):
>   drm: Add Plane Degamma properties
>   drm: Add Plane CTM property
>   drm: Add Plane Gamma properties
>   drm: Define helper function for plane color enabling
>   drm: Define helper to set legacy gamma table size
>   drm/i915: Enable plane color features
> 
>  drivers/gpu/drm/drm_atomic.c |   29 
>  drivers/gpu/drm/drm_color_mgmt.c |   41 +
>  drivers/gpu/drm/drm_mode_config.c|   35 +
>  drivers/gpu/drm/drm_plane.c  |   48 
> ++
>  drivers/gpu/drm/i915/i915_drv.h  |8 ++
>  drivers/gpu/drm/i915/intel_color.c   |   14 ++
>  drivers/gpu/drm/i915/intel_display.c |4 +++
>  drivers/gpu/drm/i915/intel_drv.h |9 +++
>  drivers/gpu/drm/i915/intel_sprite.c  |4 +++
>  include/drm/drm_color_mgmt.h |8 ++
>  include/drm/drm_mode_config.h|   28 
>  include/drm/drm_plane.h  |   31 ++
>  12 files changed, 259 insertions(+)
> 
> -- 
> 1.7.9.5
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

2017-09-26 Thread vathsala nagaraju 
Set frames before SU entry value for max resync frame count of
dpcd register 2009, bit field 0:3.

v2 :
 - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
 - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
 - add check ==1 for dpcd_read call (ville)

v3 : (Rodrigo)
 - move macro EDP_PSR2_FRAME_BEFORE_SU after EDP_PSR2_FRAME_BEFORE_SU
 - replace with &=

v4 :
 - change the macro to shift value (jani)
 - updated register names

Cc: Rodrigo Vivi 
CC: Puthikorn Voravootivat 
Reviewed-by: Rodrigo Vivi 
Signed-off-by: Vathsala Nagaraju 
---
 drivers/gpu/drm/i915/i915_reg.h  |  2 +-
 drivers/gpu/drm/i915/intel_psr.c | 13 +++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 82f36dd..7e7aa60 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4047,7 +4047,7 @@ enum {
 #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
 #define   EDP_PSR2_FRAME_BEFORE_SU_MASK(0xf<<4)
 #define   EDP_PSR2_IDLE_MASK   0xf
-#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
+#define   EDP_PSR2_FRAME_BEFORE_SU(a)  ((a)<<4)
 
 #define EDP_PSR2_STATUS_CTL_MMIO(0x6f940)
 #define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0a17d1f..5419cda 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 */
uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
uint32_t val;
+   uint8_t sink_latency;
 
val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
@@ -334,8 +335,16 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 * good enough. */
val |= EDP_PSR2_ENABLE |
-   EDP_SU_TRACK_ENABLE |
-   EDP_FRAMES_BEFORE_SU_ENTRY;
+   EDP_SU_TRACK_ENABLE;
+
+   if (drm_dp_dpcd_readb(_dp->aux,
+   DP_SYNCHRONIZATION_LATENCY_IN_SINK,
+   _latency) == 1) {
+   sink_latency &= DP_MAX_RESYNC_FRAME_COUNT_MASK;
+   } else {
+   sink_latency = 0;
+   }
+   val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
 
if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
val |= EDP_PSR2_TP2_TIME_2500;
-- 
1.9.1

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

Re: [PATCH v2 06/13] drm/sun4i: hdmi: Allow using second PLL as TMDS clk parent

2017-09-26 Thread Maxime Ripard 
On Tue, Sep 26, 2017 at 06:59:12AM +, Chen-Yu Tsai wrote:
> Allwinner SoCs typically have two PLLs reserved for video related usage.
> At the moment we only support using the first one to feed the HDMI
> transmitter block's TMDS clock.
> 
> Let the HDMI encoder's TMDS clock go through all of its parents when
> calculating possible clock rates. This allows usage of the second video
> PLL as its parent.
> 
> Note that this does not handle conflicting pixel clocks. It is entirely
> possible to have an LCD panel use one pixel clock rate, only to be
> overridden by the HDMI transmitter's clock rate request when the second
> display pipeline is enabled.
> 
> This should be handled by having all the clock drivers honor clock rate
> ranges, and have the consumers use clk_set_rate_min/clk_set_rate_max.

That, or relying on clk_set_rate_protect

Acked-by: Maxime Ripard 

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


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 10/13] drm/sun4i: hdmi: Add A31 specific DDC register definitions

2017-09-26 Thread Maxime Ripard 
On Tue, Sep 26, 2017 at 06:59:16AM +, Chen-Yu Tsai wrote:
> The DDC block for the HDMI controller is different on the A31.
> 
> This patch adds the register definitions.
> 
> Signed-off-by: Chen-Yu Tsai 

Acked-by: Maxime Ripard 

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


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

Re: [RFC v1 5/6] drm: Define helper to set legacy gamma table size

2017-09-26 Thread Lankhorst, Maarten 
Hey,

Uma Shankar schreef op di 26-09-2017 om 13:32 [+0530]:
> Define a helper function to set legacy gamma table
> size for planes.
> 
> Signed-off-by: Uma Shankar 
> ---
>  drivers/gpu/drm/drm_color_mgmt.c |   41
> ++
>  include/drm/drm_color_mgmt.h |3 +++
>  include/drm/drm_plane.h  |4 
>  3 files changed, 48 insertions(+)

Is this needed? I'm not aware of legacy tables for planes.

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

[Bug 102981] [amdgpu][tahiti xt][vulkan][GL] dota2 tournament drafting screen misses skys when antialiasing is on

2017-09-26 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=102981

Sylvain BERTRAND  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |NOTOURBUG

--- Comment #3 from Sylvain BERTRAND  ---
Since it happens on dota2 windows, this is dota2 specific. Closing.

-- 
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 v2 04/13] drm/sun4i: hdmi: Disable clks in bind function error path and unbind function

2017-09-26 Thread Maxime Ripard 
On Tue, Sep 26, 2017 at 06:59:10AM +, Chen-Yu Tsai wrote:
> The HDMI driver enables the bus and mod clocks in the bind function, but
> does not disable them if it then bails our due to any errors. Neither
> does it disable the clocks in the unbind function.
> 
> Fix this by adding a proper error path to the bind function, and
> clk_disable_unprepare calls to the unbind function.
> 
> Also rename the err_cleanup_connector label to err_cleanup_encoder,
> since it is the encoder that gets cleaned up.
> 
> Fixes: 9c5681011a0c ("drm/sun4i: Add HDMI support")
> Signed-off-by: Chen-Yu Tsai 

Acked-by: Maxime Ripard 

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


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 03/13] drm/sun4i: tcon: Add support for demuxing TCON output on A31

2017-09-26 Thread Maxime Ripard 
Hi,

On Tue, Sep 26, 2017 at 06:59:09AM +, Chen-Yu Tsai wrote:
> On systems with 2 TCONs such as the A31, it is possible to demux the
> output of the TCONs to one encoder.
> 
> Add support for this for the A31.
> 
> Signed-off-by: Chen-Yu Tsai 
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 61 
> ++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index e853dfe51389..770b843a6fa9 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -14,9 +14,12 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> +#include 
> +
>  #include 
>  #include 
>  #include 
> @@ -109,11 +112,69 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, 
> bool enable)
>  }
>  EXPORT_SYMBOL(sun4i_tcon_enable_vblank);
>  
> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm)
> +{
> + struct sun4i_drv *drv = drm->dev_private;
> + struct sun4i_tcon *tcon;
> +
> + list_for_each_entry(tcon, >tcon_list, list)
> + if (tcon->id == 0)
> + return tcon;
> +
> + dev_warn(drm->dev,
> +  "TCON0 not found, display output muxing may not work\n");
> +
> + return tcon;
> +}
> +
> +static int _sun6i_tcon_set_mux(struct drm_encoder *encoder)
> +{
> + struct sun4i_tcon *tcon = sun4i_get_first_tcon(encoder->dev);
> + int tcon_id = drm_crtc_to_sun4i_crtc(encoder->crtc)->tcon->id;
> + u32 shift;
> +
> + DRM_DEBUG_DRIVER("Muxing encoder %s to CRTC %s (TCON %d)\n",
> +  encoder->name, encoder->crtc->name, tcon_id);
> +
> + /* Only 2 TCONs */
> + if (tcon_id >= 2)
> + return -EINVAL;
> +
> + switch (encoder->encoder_type) {
> + case DRM_MODE_ENCODER_TMDS:
> + /* HDMI */
> + shift = 8;
> + break;
> + case DRM_MODE_ENCODER_DSI:
> + /* No MIPI DSI on A31s */
> + if (of_device_is_compatible(tcon->dev->of_node,
> + "allwinner,sun6i-a31s-tcon"))

I'm not sure that test is needed.

We won't end up in that case if we don't have a connected DSI block,
which isn't going to be the case on the A31. And I guess we can tackle
DSI later (when I'll send my patches...).

> + return -EINVAL;
> + shift = 0;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + regmap_update_bits(tcon->regs, SUN4I_TCON_MUX_CTRL_REG,
> +0x3 << shift, tcon_id << shift);
> +
> + return 0;
> +}
> +
>  void sun4i_tcon_set_mux(struct sun4i_tcon *tcon, int channel,
>   struct drm_encoder *encoder)
>  {
> + /* Get the device node of the display engine */
> + struct device_node *node = encoder->dev->dev->of_node;
>   u32 val;
>  
> + if (of_device_is_compatible(node, "allwinner,sun6i-a31-display-engine") 
> ||
> + of_device_is_compatible(node, 
> "allwinner,sun6i-a31s-display-engine")) {
> + _sun6i_tcon_set_mux(encoder);
> + return;
> + }
> +

I'd really like to avoid mix and matching the structure defined
behaviour and those of_device_is_compatible calls spread out
everywhere.

You can either add a flag or a function pointer.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


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 05/13] drm/sun4i: hdmi: create a regmap for later use

2017-09-26 Thread Maxime Ripard 
On Tue, Sep 26, 2017 at 06:59:11AM +, Chen-Yu Tsai wrote:
> The HDMI driver is written with readl/writel I/O to the registers.
> However, to support the A31 variant, which has a different layout
> for the DDC registers, it was recommended to use regfields to have
> a cleaner implementation. To use regfields, we need to create an
> underlying regmap.
> 
> This patch only adds the regmap. It does not convert the existing
> driver accesses to use regmap.
> 
> Signed-off-by: Chen-Yu Tsai 

Acked-by: Maxime Ripard 

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


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

[PATCH 1/2] drm/dp: Add defines for latency in sink

2017-09-26 Thread vathsala nagaraju 
Add defines for dpcd register 2009 (synchronization latency
in sink).

v2:
 - add spec version (Daniel)
 - use register name as is in spec,only drop excess
   from end (jani)
 - add the full register contents (jani)

Cc: Rodrigo Vivi 
CC: Puthikorn Voravootivat 
Reviewed-by: Rodrigo Vivi 
Signed-off-by: Vathsala Nagaraju 
---
 include/drm/drm_dp_helper.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 11c39f1..f58dcb9 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -735,6 +735,12 @@
 # define DP_PSR_SINK_INTERNAL_ERROR 7
 # define DP_PSR_SINK_STATE_MASK 0x07
 
+#define DP_SYNCHRONIZATION_LATENCY_IN_SINK 0x2009 /* edp 1.4b */
+# define DP_MAX_RESYNC_FRAME_COUNT_MASK(0xf << 0)
+# define DP_MAX_RESYNC_FRAME_COUNT_SHIFT   0
+# define DP_LAST_ACTUAL_SYNCHRONIZATION_LATENCY_MASK   (0xf << 4)
+# define DP_LAST_ACTUAL_SYNCHRONIZATION_LATENCY_SHIFT  4
+
 #define DP_RECEIVER_ALPM_STATUS0x200b  /* eDP 1.4 */
 # define DP_ALPM_LOCK_TIMEOUT_ERROR(1 << 0)
 
-- 
1.9.1

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

Re: [PATCH v2 11/13] drm/sun4i: hdmi: Add support for A31's HDMI controller

2017-09-26 Thread Maxime Ripard 
On Tue, Sep 26, 2017 at 06:59:17AM +, Chen-Yu Tsai wrote:
> The HDMI controller found in the A31 SoCs is slightly different
> from the one already supported, which is found in the A10s:
> 
>   - Need different initial values for the PLL related registers
> 
>   - Different behavior of the DDC and TMDS clocks
> 
>   - Different register layout for the DDC portion
> 
>   - Separate DDC parent clock
> 
> This patch adds support for it.
> 
> Signed-off-by: Chen-Yu Tsai 

Acked-by: Maxime Ripard 

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


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 09/13] drm/sun4i: hdmi: Add support for controller hardware variants

2017-09-26 Thread Maxime Ripard 
On Tue, Sep 26, 2017 at 06:59:15AM +, Chen-Yu Tsai wrote:
> The HDMI controller found in earlier Allwinner SoCs have slight
> differences between the A10, A10s, and the A31:
> 
>   - Need different initial values for the PLL related registers
> 
>   - Different behavior of the DDC and TMDS clocks
> 
>   - Different register layout for the DDC portion
> 
>   - Separate DDC parent clock on the A31
> 
>   - Explicit reset control
> 
> For the A31, the HDMI TMDS clock has a different value offset for
> the divider. The HDMI DDC block is different from the one in the
> other SoCs. As far as the DDC clock goes, it has no pre-divider,
> as it is clocked from a slower parent clock, not the TMDS clock.
> The divider offset from the register value is different. And the
> clock control register is at a different offset.
> 
> A new variant data structure is created to store pointers to the
> above functions, structures, and the different initial values.
> Another flag notates whether there is a separate DDC parent clock.
> If not, the TMDS clock is passed to the DDC clock create function,
> as before.
> 
> Regmap fields are used to deal with the different register layout
> of the DDC block.
> 
> Signed-off-by: Chen-Yu Tsai 

Acked-by: Maxime Ripard 

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


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

RE: [RFC v1 5/6] drm: Define helper to set legacy gamma table size

2017-09-26 Thread Shankar, Uma 


>-Original Message-
>From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of
>Lankhorst, Maarten
>Sent: Tuesday, September 26, 2017 3:36 PM
>To: Shankar, Uma ; intel-...@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Syrjala, Ville 
>Subject: Re: [RFC v1 5/6] drm: Define helper to set legacy gamma table size
>
>Hey,
>
>Uma Shankar schreef op di 26-09-2017 om 13:32 [+0530]:
>> Define a helper function to set legacy gamma table size for planes.
>>
>> Signed-off-by: Uma Shankar 
>> ---
>>  drivers/gpu/drm/drm_color_mgmt.c |   41
>> ++
>>  include/drm/drm_color_mgmt.h |3 +++
>>  include/drm/drm_plane.h  |4 
>>  3 files changed, 48 insertions(+)
>
>Is this needed? I'm not aware of legacy tables for planes.

I was not getting very concrete info on this. So kept it as per pipe gamma 
implementation.
I will try to get some more info and drop this in case it's not required.

Regards,
Uma Shankar

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

Re: [RFC v1 5/6] drm: Define helper to set legacy gamma table size

2017-09-26 Thread Lankhorst, Maarten 
Shankar, Uma schreef op di 26-09-2017 om 15:41 [+0530]:
> > -Original Message-
> > From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On
> > Behalf Of
> > Lankhorst, Maarten
> > Sent: Tuesday, September 26, 2017 3:36 PM
> > To: Shankar, Uma ; intel-gfx@lists.freedeskt
> > op.org;
> > dri-devel@lists.freedesktop.org
> > Cc: Syrjala, Ville 
> > Subject: Re: [RFC v1 5/6] drm: Define helper to set legacy gamma
> > table size
> > 
> > Hey,
> > 
> > Uma Shankar schreef op di 26-09-2017 om 13:32 [+0530]:
> > > Define a helper function to set legacy gamma table size for
> > > planes.
> > > 
> > > Signed-off-by: Uma Shankar 
> > > ---
> > >  drivers/gpu/drm/drm_color_mgmt.c |   41
> > > ++
> > >  include/drm/drm_color_mgmt.h |3 +++
> > >  include/drm/drm_plane.h  |4 
> > >  3 files changed, 48 insertions(+)
> > 
> > Is this needed? I'm not aware of legacy tables for planes.
> 
> I was not getting very concrete info on this. So kept it as per pipe
> gamma implementation.
> I will try to get some more info and drop this in case it's not
> required.
> 

It's not, legacy gamma would only be used in drm_mode_gamma_get_ioctl,
which if you look at it only works for a crtc. :)

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

Re: [Outreachy kernel] [PATCH] drm/gem: use new idr deletion interface to cleanup drm_gem_handle_delete()

2017-09-26 Thread Julia Lawall 


On Tue, 26 Sep 2017, Daniel Vetter wrote:

> On Tue, Sep 26, 2017 at 10:47 AM, Daniel Vetter  wrote:
> > On Tue, Sep 26, 2017 at 10:38 AM, Julia Lawall  wrote:
> >>
> >>
> >> On Tue, 26 Sep 2017, Daniel Vetter wrote:
> >>
> >>> On Tue, Sep 26, 2017 at 12:17:28AM +0530, Aishwarya Pant wrote:
> >>> > The IDR deletion interface now returns the deleted entry or NULL if it 
> >>> > was not
> >>> > present. So we don't have to do the extra work of checking if we have a
> >>> > reference on the drm_gem_object, this can be handled by checking the 
> >>> > return
> >>> > value from idr_remove() and the extra locks can be dropped.
> >>> >
> >>> > Signed-off-by: Aishwarya Pant 
> >>>
> >>> Haneen is already working on this task, with the patch just merged. Please
> >>> coordinate with mentors (me or Sean Paul) before starting on something to
> >>> avoid such clashes in the future.
> >>>
> >>> Note also that some todo items are just ideas, and need confirmation with
> >>> relevant maintainers first.
> >>
> >> Not sure where the mixup happened, but anyone who starts on a project
> >> specific task should note that on the outreachy kernel wiki and before
> >> starting on a project specific task one should check that no one is
> >> working on it.  If someone started some time ago, and the code doesn't
> >> seem to have changed, ask the person who claimed the task or the mentor.
> >
> > Hm, the dri-devel tasks aren't on the wiki, but in the kernel sources.
> > Where should we put the scratch-pad to avoid such conflicts in the
> > future? E.g. the IIO subsytems has it's task on the wiki, where this
> > works much better.
>
> Ok, I found it. Looks like a few signed up for stuff, but without
> chatting with us first, and ended up picking tasks that need some
> serious GPU expertise. I.e. maybe suitable for a full internship,
> definitely not as starter tasks. I'll send out mails.

Thanks.  Everyone, please discuss with the mentor before picking a task.

julia

> -Daniel
>
>
> > -Daniel
> >
> >>
> >> thanks,
> >>
> >> julia
> >>
> >>>
> >>> Thanks, Daniel
> >>> > ---
> >>> >  drivers/gpu/drm/drm_gem.c | 21 ++---
> >>> >  1 file changed, 2 insertions(+), 19 deletions(-)
> >>> >
> >>> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> >>> > index c55f338..f62757a 100644
> >>> > --- a/drivers/gpu/drm/drm_gem.c
> >>> > +++ b/drivers/gpu/drm/drm_gem.c
> >>> > @@ -282,29 +282,12 @@ drm_gem_handle_delete(struct drm_file *filp, u32 
> >>> > handle)
> >>> >  {
> >>> > struct drm_gem_object *obj;
> >>> >
> >>> > -   /* This is gross. The idr system doesn't let us try a delete and
> >>> > -* return an error code.  It just spews if you fail at deleting.
> >>> > -* So, we have to grab a lock around finding the object and then
> >>> > -* doing the delete on it and dropping the refcount, or the user
> >>> > -* could race us to double-decrement the refcount and cause a
> >>> > -* use-after-free later.  Given the frequency of our handle lookups,
> >>> > -* we may want to use ida for number allocation and a hash table
> >>> > -* for the pointers, anyway.
> >>> > -*/
> >>> > spin_lock(>table_lock);
> >>> > -
> >>> > -   /* Check if we currently have a reference on the object */
> >>> > -   obj = idr_replace(>object_idr, NULL, handle);
> >>> > -   spin_unlock(>table_lock);
> >>> > -   if (IS_ERR_OR_NULL(obj))
> >>> > +   obj = idr_remove(>object_idr, handle);
> >>> > +   if (!obj)
> >>> > return -EINVAL;
> >>> > -
> >>> > /* Release driver's reference and decrement refcount. */
> >>> > drm_gem_object_release_handle(handle, obj, filp);
> >>> > -
> >>> > -   /* And finally make the handle available for future allocations. */
> >>> > -   spin_lock(>table_lock);
> >>> > -   idr_remove(>object_idr, handle);
> >>> > spin_unlock(>table_lock);
> >>> >
> >>> > return 0;
> >>> > --
> >>> > 2.7.4
> >>> >
> >>> > --
> >>> > You received this message because you are subscribed to the Google 
> >>> > Groups "outreachy-kernel" group.
> >>> > To unsubscribe from this group and stop receiving emails from it, send 
> >>> > an email to outreachy-kernel+unsubscr...@googlegroups.com.
> >>> > To post to this group, send email to outreachy-ker...@googlegroups.com.
> >>> > To view this discussion on the web visit 
> >>> > https://groups.google.com/d/msgid/outreachy-kernel/20170925184728.GA8861%40aishwarya.
> >>> > For more options, visit https://groups.google.com/d/optout.
> >>>
> >>> --
> >>> Daniel Vetter
> >>> Software Engineer, Intel Corporation
> >>> http://blog.ffwll.ch
> >>>
> >>> --
> >>> You received this message because you are subscribed to the Google Groups 
> >>> "outreachy-kernel" group.
> >>> To unsubscribe from this group and stop receiving emails from it, send an 
> >>> email to outreachy-kernel+unsubscr...@googlegroups.com.
> >>> To post to this group, send email to

RE: [RFC v1 5/6] drm: Define helper to set legacy gamma table size

2017-09-26 Thread Shankar, Uma 


>-Original Message-
>From: Lankhorst, Maarten
>Sent: Tuesday, September 26, 2017 3:45 PM
>To: Shankar, Uma ; intel-...@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Syrjala, Ville 
>Subject: Re: [RFC v1 5/6] drm: Define helper to set legacy gamma table size
>
>Shankar, Uma schreef op di 26-09-2017 om 15:41 [+0530]:
>> > -Original Message-
>> > From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On
>> > Behalf Of Lankhorst, Maarten
>> > Sent: Tuesday, September 26, 2017 3:36 PM
>> > To: Shankar, Uma ; intel-gfx@lists.freedeskt
>> > op.org; dri-devel@lists.freedesktop.org
>> > Cc: Syrjala, Ville 
>> > Subject: Re: [RFC v1 5/6] drm: Define helper to set legacy gamma
>> > table size
>> >
>> > Hey,
>> >
>> > Uma Shankar schreef op di 26-09-2017 om 13:32 [+0530]:
>> > > Define a helper function to set legacy gamma table size for
>> > > planes.
>> > >
>> > > Signed-off-by: Uma Shankar 
>> > > ---
>> > >  drivers/gpu/drm/drm_color_mgmt.c |   41
>> > > ++
>> > >  include/drm/drm_color_mgmt.h |3 +++
>> > >  include/drm/drm_plane.h  |4 
>> > >  3 files changed, 48 insertions(+)
>> >
>> > Is this needed? I'm not aware of legacy tables for planes.
>>
>> I was not getting very concrete info on this. So kept it as per pipe
>> gamma implementation.
>> I will try to get some more info and drop this in case it's not
>> required.
>>
>
>It's not, legacy gamma would only be used in drm_mode_gamma_get_ioctl,
>which if you look at it only works for a crtc. :)
>

Yeah I thought that it was just added for crtc, and implementation for plane 
was not done. 
Was not sure if it can also be extended for plane.

Will drop this change. Thanks for clarifying.

Regards,
Uma Shankar

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

Re: [PATCH v2] drm/tinydrm: Move backlight helpers to a separate file

2017-09-26 Thread Daniel Vetter 
On Tue, Sep 26, 2017 at 04:46:53PM +0530, Meghana Madhyastha wrote:
> On Mon, Sep 25, 2017 at 06:31:58PM +0200, Noralf Trønnes wrote:
> > 
> > Den 25.09.2017 16.56, skrev Noralf Trønnes:
> > >Hi Meghana,
> > >
> > >
> > >Den 22.09.2017 17.09, skrev Meghana Madhyastha:
> > >>Move backlight helpers from tinydrm-helpers.c to
> > >>tinydrm-backlight.c. This is because it is organizationally
> > >>simpler to understand and advantageous to group functions
> > >>performing a similar function to a separate file as opposed to
> > >>having one helper file with heteregenous helper functions.
> > >>
> > >>Signed-off-by: Meghana Madhyastha 
> > >>---
> > >
> > >I don't think there is much gain in just moving the code like this.
> > >
> > >The idea is to add a drm_backlight helper that can be useful for all
> > >DRM drivers that use the backlight subsystem.
> 
> Yes I agree. That definitely makes more sense.
> > 
> > The full path to that helper would be:
> > drivers/gpu/drm/drm_backlight.c
> > 
> > >This is what the TODO says:
> > >https://dri.freedesktop.org/docs/drm/gpu/todo.html#tinydrm
> > >
> > >- backlight helpers, probably best to put them into a new drm_backlight.c.
> > >  This is because drivers/video is de-facto unmaintained. We could also
> > >  move drivers/video/backlight to drivers/gpu/backlight and take it all
> > >  over within drm-misc, but that’s more work.
> > >
> > >There is also this discussion to take into account:
> > >KMS backlight ABI proposition
> > >https://lists.freedesktop.org/archives/dri-devel/2017-February/133206.html
> > >
> > >
> > >I don't remember what came out of that discussion.
> > >
> > >Noralf.
> 
> After having discussed this with Daniel on the #dri-devel irc channel, 
> here are some of the points suggested.
> 
> Daniel suggested that I first look into the usage of shared backlight
> helpers in drm (specifically backlight_update_status to begin with). The idea
> was to see whether there is any pattern in usage and/or code dupication.
> If there is, then the next step would be to write helper functions which
> can be used by other drivers (and not just tinydrm). 
> 
> To start with, I went through the instances of backlight_update_status
> in the drm code, and made the following observations(most of them are
> very simple/naive observations).
> 
> - backlight_update_status is usually called in backlight init (and
>   sometimes exit) functions of the drivers just after the device is 
> registered.
>   So backlight_update_status is called with the registered device as the
>   parameter.
> 
> Here are the following cases of properties changed/set before 
> backlight_update_status is called.
> 
> - CASE 1: Brightness is changed (either a macro BRIGHTNESS_MAX_LEVEL 100
>   is defined or it is manually set) This happens in the following files:
> 
>   gma500/cdv_device.c, gma500/mdfld_device.c, gma500/oaktrail_device.c,
>   gma500/psb_device.c, noveau/noveau_backlight.c(here brightness is 
> determined by fuction
>   static int nv50_get_intensity)
> 
> - CASE 2: Power property is set (to FB_BLANK_UNBLANK mostly)
>   This happens in the following files:
> 
>   omapdrm/displays/panel-dpi.c, panel/panel-innolux-p079zca.c,
>   panel/panel-jdi-lt070me05000.c, panel/panel-sharp-lq101r1sx01.c, 
>   panel/panel-sharp-ls043t1le01.c, tilcdc/tilcdc_panel.c
>   
> - CASE 3: State is set
>   This happens in the following files:
> 
>   tinydrm/tinydrm-helpers.c
> 
> - CASE 4: Power and brightness properties are set
>   This happens in the following files:
> 
>   atombios_encoders.c, radeon/radeon_legacy_encoders.c, 
>   shmobile/shmob_drm_backlight.c
> 
> - CASE 5: Power and the state properties are set
>   This happens in the following files:
> 
>   panel/panel-lvds.c, panel/panel-panasonic-vvx10f034n00.c,
>   panel/panel-simple.c, panel/panel-sitronix-st7789v.c
> 
> Please let me know if I am wrong / missed something. As for next steps,
> wouldn't it be an overkill to have a separate helper function for each
> of these cases ? Perhaps a generic helper function which would somehow
> address these cases would be more appropriate ? Thank you for your
> time/patience.

I suspect that a lot of these combinations are just plain wrong, but
happen to kinda work with the combination of gpu driver and backlight
driver they're used on. tbh I have no idea which one is the correct
version for enabling a backlight correctly ...

So definitely a good task to refactor this into a proper helper, but looks
a lot more involved than at first sight.

Do you have any of the hardware supported by any of these drivers? lsmod
and then comparing with the modules you're building in your own tree
should help you figure this out.
-Daniel

> 
> -Meghana
> 
> > >>Changes in v2:
> > >>  -Improved commit message by explaining why the changes were made.
> > >>
> > >>  drivers/gpu/drm/tinydrm/core/Makefile    |   2 +-
> > >>  drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c | 103
> >

Re: [PATCH] drm/gem: use new idr deletion interface to cleanup drm_gem_handle_delete()

2017-09-26 Thread Aishwarya Pant 
On Tue, Sep 26, 2017 at 10:12:12AM +0100, Chris Wilson wrote:
> Quoting Aishwarya Pant (2017-09-25 19:47:28)
> > The IDR deletion interface now returns the deleted entry or NULL if it was 
> > not
> > present. So we don't have to do the extra work of checking if we have a
> > reference on the drm_gem_object, this can be handled by checking the return
> > value from idr_remove() and the extra locks can be dropped.
> > 
> > Signed-off-by: Aishwarya Pant 
> 
> This reintroduces the bug were the idr entry is available for reuse
> before the drivers have had the change to release their resources. So a
> new handle may be reused that is then hooked up to the old private data.
> See commit f6cd7daecff558fab2c45d15283d3e52f688342d
> Author: Chris Wilson 
> Date:   Fri Apr 15 12:55:08 2016 +0100
> 
> drm: Release driver references to handle before making it available again

Thanks, this makes sense now.

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

[PATCH v2 2/2] drm/tilcdc: replace reference/unreference() with get/put

2017-09-26 Thread Aishwarya Pant 
For maintaining consistency with kernel coding style replace
reference/unreference in ref counting functions with get/put.

The following cocci script was used to generate the tilcdc patch:

@@
expression ex;
@@

(
-drm_framebuffer_unreference(ex);
+drm_framebuffer_put(ex);
|
-drm_dev_unref(ex);
+drm_dev_put(ex);
|
-drm_framebuffer_reference(ex);
+drm_framebuffer_get(ex);
)

Signed-off-by: Aishwarya Pant 
---
No changes in v2

drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 6 +++---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 406fe45..d2589f310 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -75,7 +75,7 @@ static void unref_worker(struct drm_flip_work *work, void 
*val)
struct drm_device *dev = tilcdc_crtc->base.dev;
 
mutex_lock(>mode_config.mutex);
-   drm_framebuffer_unreference(val);
+   drm_framebuffer_put(val);
mutex_unlock(>mode_config.mutex);
 }
 
@@ -456,7 +456,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
 
set_scanout(crtc, fb);
 
-   drm_framebuffer_reference(fb);
+   drm_framebuffer_get(fb);
 
crtc->hwmode = crtc->state->adjusted_mode;
 }
@@ -633,7 +633,7 @@ int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
return -EBUSY;
}
 
-   drm_framebuffer_reference(fb);
+   drm_framebuffer_get(fb);
 
crtc->primary->fb = fb;
tilcdc_crtc->event = event;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index b0d70f9..74276ef 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -225,7 +225,7 @@ static void tilcdc_fini(struct drm_device *dev)
 
pm_runtime_disable(dev->dev);
 
-   drm_dev_unref(dev);
+   drm_dev_put(dev);
 }
 
 static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
-- 
2.7.4

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

Re: [Outreachy kernel] [PATCH] drm/gem: use new idr deletion interface to cleanup drm_gem_handle_delete()

2017-09-26 Thread Aishwarya Pant 
On Tue, Sep 26, 2017 at 10:20:47AM +0200, Daniel Vetter wrote:
> On Tue, Sep 26, 2017 at 12:17:28AM +0530, Aishwarya Pant wrote:
> > The IDR deletion interface now returns the deleted entry or NULL if it was 
> > not
> > present. So we don't have to do the extra work of checking if we have a
> > reference on the drm_gem_object, this can be handled by checking the return
> > value from idr_remove() and the extra locks can be dropped.
> > 
> > Signed-off-by: Aishwarya Pant 
> 
> Haneen is already working on this task, with the patch just merged. Please
> coordinate with mentors (me or Sean Paul) before starting on something to
> avoid such clashes in the future.

Apologies for the mix-up, I'll make sure to check in with the mentors before
starting a task.

I looked over the other patch sent by Haneen today, there is one thing that I
could use some clarity on. I'm not sure how the -this is gross- comment is
obsolete since we can drop the check to idr_replace() and use the return value
from idr_remove() which seems neater in my opinion.

> 
> Note also that some todo items are just ideas, and need confirmation with
> relevant maintainers first.
> 
> Thanks, Daniel
> > ---
> >  drivers/gpu/drm/drm_gem.c | 21 ++---
> >  1 file changed, 2 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index c55f338..f62757a 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -282,29 +282,12 @@ drm_gem_handle_delete(struct drm_file *filp, u32 
> > handle)
> >  {
> > struct drm_gem_object *obj;
> >  
> > -   /* This is gross. The idr system doesn't let us try a delete and
> > -* return an error code.  It just spews if you fail at deleting.
> > -* So, we have to grab a lock around finding the object and then
> > -* doing the delete on it and dropping the refcount, or the user
> > -* could race us to double-decrement the refcount and cause a
> > -* use-after-free later.  Given the frequency of our handle lookups,
> > -* we may want to use ida for number allocation and a hash table
> > -* for the pointers, anyway.
> > -*/
> > spin_lock(>table_lock);
> > -
> > -   /* Check if we currently have a reference on the object */
> > -   obj = idr_replace(>object_idr, NULL, handle);
> > -   spin_unlock(>table_lock);
> > -   if (IS_ERR_OR_NULL(obj))
> > +   obj = idr_remove(>object_idr, handle);
> > +   if (!obj)
> > return -EINVAL;
> > -
> > /* Release driver's reference and decrement refcount. */
> > drm_gem_object_release_handle(handle, obj, filp);
> > -
> > -   /* And finally make the handle available for future allocations. */
> > -   spin_lock(>table_lock);
> > -   idr_remove(>object_idr, handle);
> > spin_unlock(>table_lock);
> >  
> > return 0;
> > -- 
> > 2.7.4
> > 
> > -- 
> > You received this message because you are subscribed to the Google Groups 
> > "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an 
> > email to outreachy-kernel+unsubscr...@googlegroups.com.
> > To post to this group, send email to outreachy-ker...@googlegroups.com.
> > To view this discussion on the web visit 
> > https://groups.google.com/d/msgid/outreachy-kernel/20170925184728.GA8861%40aishwarya.
> > For more options, visit https://groups.google.com/d/optout.
> 
> -- 
> 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

[PATCH v2 0/2] drm/tilcdc: replace reference/unreference with get/put

2017-09-26 Thread Aishwarya Pant 
This patchset introduces drm_dev_get() and drm_dev_put() functions that are
intented to be replacements for drm_dev_{ref/unref}.

Then all usages of ref/reference and unref/unreference suffixes are replaced by
get/put reference count functions in tilcdc. The following cocci script was used
to make the tilcdc patch:

@@
expression ex;
@@

(
-drm_framebuffer_unreference(ex);
+drm_framebuffer_put(ex);
|
-drm_dev_unref(ex);
+drm_dev_put(ex);
|
-drm_framebuffer_reference(ex);
+drm_framebuffer_get(ex);
)

Changes in v2
- Drop drm_dev_ref after replacing all its usages
- Update the drm subsystem wide cocci script drm-get-put with the new helper for
  drm_dev_unref to drm_dev_get conversion

Aishwarya Pant (2):
  drm: introduce drm_dev_{get/put} functions
  drm/tilcdc: replace reference/unreference() with get/put

 drivers/gpu/drm/drm_drv.c| 51 
 drivers/gpu/drm/drm_prime.c  |  2 +-
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c |  6 ++--
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  2 +-
 include/drm/drm_drv.h|  5 ++--
 scripts/coccinelle/api/drm-get-put.cocci |  5 
 6 files changed, 45 insertions(+), 26 deletions(-)

-- 
2.7.4

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

Kernel GPU subsytem: Better manual upload for atomic

2017-09-26 Thread Harsha Sharma 
Hello,
I am working on task "Better manual upload for atomic" in kernel gpu
subsystem.
When duplicating the crtc state I need to change drm_rect x2 and y2 to
MAX_INT.
I didn't get what is MAX_INT over here. Also, how am I supposed to test my
code?
It will be really helpful if you can suggest something.
Thanks a lot for your time.

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

[PATCH v2 1/2] drm: introduce drm_dev_{get/put} functions

2017-09-26 Thread Aishwarya Pant 
Reference counting functions in the kernel typically use get/put suffixes. For
maintaining coding style consistency, introduce drm_dev_{get/put} functions. All
callers of drm_dev_ref() API have been converted in this patch and hence it has
been dropped while the drm_dev_unref() API with non-trivial number of users
remains for compatibility.

The semantic patch scripts/coccinelle/api/drm-get-put.cocci has been updated
with the new helper for conversion of drm_dev_unref() to drm_dev_put()

Signed-off-by: Aishwarya Pant 
---
Changes in v2
- Drop drm_dev_ref after replacing all its usages
- Update the drm subsystem wide cocci script drm-get-put with the new helper for
  drm_dev_unref to drm_dev_get conversion

 drivers/gpu/drm/drm_drv.c| 51 
 drivers/gpu/drm/drm_prime.c  |  2 +-
 include/drm/drm_drv.h|  5 ++--
 scripts/coccinelle/api/drm-get-put.cocci |  5 
 4 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index be38ac7..c0292e5 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -286,13 +286,13 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
spin_lock_irqsave(_minor_lock, flags);
minor = idr_find(_minors_idr, minor_id);
if (minor)
-   drm_dev_ref(minor->dev);
+   drm_dev_get(minor->dev);
spin_unlock_irqrestore(_minor_lock, flags);
 
if (!minor) {
return ERR_PTR(-ENODEV);
} else if (drm_dev_is_unplugged(minor->dev)) {
-   drm_dev_unref(minor->dev);
+   drm_dev_put(minor->dev);
return ERR_PTR(-ENODEV);
}
 
@@ -301,7 +301,7 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
 
 void drm_minor_release(struct drm_minor *minor)
 {
-   drm_dev_unref(minor->dev);
+   drm_dev_put(minor->dev);
 }
 
 /**
@@ -326,11 +326,11 @@ void drm_minor_release(struct drm_minor *minor)
  * When cleaning up a device instance everything needs to be done in reverse:
  * First unpublish the device instance with drm_dev_unregister(). Then clean up
  * any other resources allocated at device initialization and drop the driver's
- * reference to _device using drm_dev_unref().
+ * reference to _device using drm_dev_put().
  *
  * Note that the lifetime rules for _device instance has still a lot of
  * historical baggage. Hence use the reference counting provided by
- * drm_dev_ref() and drm_dev_unref() only carefully.
+ * drm_dev_get() and drm_dev_put() only carefully.
  *
  * It is recommended that drivers embed  drm_device into their own 
device
  * structure, which is supported through drm_dev_init().
@@ -345,7 +345,7 @@ void drm_minor_release(struct drm_minor *minor)
  * Cleans up all DRM device, calling drm_lastclose().
  *
  * Note: Use of this function is deprecated. It will eventually go away
- * completely.  Please use drm_dev_unregister() and drm_dev_unref() explicitly
+ * completely.  Please use drm_dev_unregister() and drm_dev_put() explicitly
  * instead to make sure that the device isn't userspace accessible any more
  * while teardown is in progress, ensuring that userspace can't access an
  * inconsistent state.
@@ -360,7 +360,7 @@ void drm_put_dev(struct drm_device *dev)
}
 
drm_dev_unregister(dev);
-   drm_dev_unref(dev);
+   drm_dev_put(dev);
 }
 EXPORT_SYMBOL(drm_put_dev);
 
@@ -386,7 +386,7 @@ void drm_dev_unplug(struct drm_device *dev)
mutex_lock(_global_mutex);
drm_device_set_unplugged(dev);
if (dev->open_count == 0)
-   drm_dev_unref(dev);
+   drm_dev_put(dev);
mutex_unlock(_global_mutex);
 }
 EXPORT_SYMBOL(drm_dev_unplug);
@@ -475,8 +475,8 @@ static void drm_fs_inode_free(struct inode *inode)
  * initialization sequence to make sure userspace can't access an inconsistent
  * state.
  *
- * The initial ref-count of the object is 1. Use drm_dev_ref() and
- * drm_dev_unref() to take and drop further ref-counts.
+ * The initial ref-count of the object is 1. Use drm_dev_get() and
+ * drm_dev_put() to take and drop further ref-counts.
  *
  * Note that for purely virtual devices @parent can be NULL.
  *
@@ -626,8 +626,8 @@ EXPORT_SYMBOL(drm_dev_fini);
  * initialization sequence to make sure userspace can't access an inconsistent
  * state.
  *
- * The initial ref-count of the object is 1. Use drm_dev_ref() and
- * drm_dev_unref() to take and drop further ref-counts.
+ * The initial ref-count of the object is 1. Use drm_dev_get() and
+ * drm_dev_put() to take and drop further ref-counts.
  *
  * Note that for purely virtual devices @parent can be NULL.
  *
@@ -670,36 +670,49 @@ static void drm_dev_release(struct kref *ref)
 }
 
 /**
- * drm_dev_ref - Take reference of a DRM device
+ * drm_dev_get - Take reference of a DRM device
  * @dev: device to take reference of or NULL
  *
  *

Re: [PATCH v3 0/3] Split default display handling out from VGA arbiter

2017-09-26 Thread Daniel Axtens 
Hi Bjorn,

Yes, this works:

Tested-by: Daniel Axtens  # arm64, ppc64-qemu-tcg


Regards,
Daniel
> On Fri, Sep 01, 2017 at 05:27:41PM +1000, Daniel Axtens wrote:
>> This patch set:
>> 
>>  - splits the default display handling out from VGA arbiter, into its
>>own file and behind its own Kconfig option (and gives the functions
>>better names).
>> 
>>  - adds extra detection of default devices. To be nominated, the vga
>>arbiter and platform hooks must not have nominated a default. A
>>card will then only be nominated if it has a driver attached and
>>has IO or memory decoding enabled.
>> 
>>  - adds relevant documentation.
>> 
>> The practical impact of this is improved X autoconfiguration on some
>> arm64 systems.
>
> I think I gave you bad advice about trying to separate the "default
> device" idea from the VGA arbiter.
>
> It is true that the "VGA arbiter" per se is related to routing the
> legacy VGA resources, and the arbiter currently only selects a default
> device if it finds a device to which those resources are routed.
>
> We have some cases where we want to select a default device that may
> not support the legacy VGA resources, or where they might not be
> routed to the device:
>
>   - systems where we match the EFI framebuffer address with a BAR, and
> select that device as default,
>
>   - powerpc systems where there may be no host bridge window that maps
> to the legacy VGA resources,
>
>   - your ARM64 systems where the default device may be behind a bridge
> that doesn't support legacy VGA routing (PCI_BRIDGE_CTL_VGA)
>
> But I think trying to split the "default device" part out from the VGA
> arbiter ends up being overkill and making things more complicated
> instead of simpler.
>
> Would something like the following work for you as well as the powerpc
> case?  On powerpc, we already use vga_set_default_device() to select a
> device that doesn't use legacy VGA resources, so maybe we can just do
> the same on ARM64?
>
> I suppose there might be wrinkles in how the arbiter deals with
> multiple graphics devices on those systems, since I don't think it
> identifies these devices that don't use the legacy resources, but it
> seems like we live with whatever those on are powerpc and probably can
> on ARM64 as well.
>
>
> diff --git a/arch/powerpc/kernel/pci-common.c 
> b/arch/powerpc/kernel/pci-common.c
> index 02831a396419..0ac7aa346c69 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1740,15 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct 
> pci_dev *dev)
>  }
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, 
> fixup_hide_host_resource_fsl);
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, 
> fixup_hide_host_resource_fsl);
> -
> -static void fixup_vga(struct pci_dev *pdev)
> -{
> - u16 cmd;
> -
> - pci_read_config_word(pdev, PCI_COMMAND, );
> - if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || 
> !vga_default_device())
> - vga_set_default_device(pdev);
> -
> -}
> -DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> -   PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index 76875f6299b8..9df4802c5f04 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -1468,6 +1468,21 @@ static int __init vga_arb_device_init(void)
>   vgaarb_info(dev, "no bridge control possible\n");
>   }
>  
> + if (!vga_default_device()) {
> + list_for_each_entry(vgadev, _list, list) {
> + struct device *dev = >pdev->dev;
> + u16 cmd;
> +
> + pdev = vgadev->pdev;
> + pci_read_config_word(pdev, PCI_COMMAND, );
> + if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
> + vgaarb_info(dev, "setting as boot device\n");
> + vga_set_default_device(pdev);
> + break;
> + }
> + }
> + }
> +
>   pr_info("loaded\n");
>   return rc;
>  }


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

Re: [PATCH v2 03/13] drm/sun4i: tcon: Add support for demuxing TCON output on A31

2017-09-26 Thread Chen-Yu Tsai 
On Tue, Sep 26, 2017 at 5:56 PM, Maxime Ripard
 wrote:
> Hi,
>
> On Tue, Sep 26, 2017 at 06:59:09AM +, Chen-Yu Tsai wrote:
>> On systems with 2 TCONs such as the A31, it is possible to demux the
>> output of the TCONs to one encoder.
>>
>> Add support for this for the A31.
>>
>> Signed-off-by: Chen-Yu Tsai 
>> ---
>>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 61 
>> ++
>>  1 file changed, 61 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
>> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> index e853dfe51389..770b843a6fa9 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> @@ -14,9 +14,12 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>
>> +#include 
>> +
>>  #include 
>>  #include 
>>  #include 
>> @@ -109,11 +112,69 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, 
>> bool enable)
>>  }
>>  EXPORT_SYMBOL(sun4i_tcon_enable_vblank);
>>
>> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm)
>> +{
>> + struct sun4i_drv *drv = drm->dev_private;
>> + struct sun4i_tcon *tcon;
>> +
>> + list_for_each_entry(tcon, >tcon_list, list)
>> + if (tcon->id == 0)
>> + return tcon;
>> +
>> + dev_warn(drm->dev,
>> +  "TCON0 not found, display output muxing may not work\n");
>> +
>> + return tcon;
>> +}
>> +
>> +static int _sun6i_tcon_set_mux(struct drm_encoder *encoder)
>> +{
>> + struct sun4i_tcon *tcon = sun4i_get_first_tcon(encoder->dev);
>> + int tcon_id = drm_crtc_to_sun4i_crtc(encoder->crtc)->tcon->id;
>> + u32 shift;
>> +
>> + DRM_DEBUG_DRIVER("Muxing encoder %s to CRTC %s (TCON %d)\n",
>> +  encoder->name, encoder->crtc->name, tcon_id);
>> +
>> + /* Only 2 TCONs */
>> + if (tcon_id >= 2)
>> + return -EINVAL;
>> +
>> + switch (encoder->encoder_type) {
>> + case DRM_MODE_ENCODER_TMDS:
>> + /* HDMI */
>> + shift = 8;
>> + break;
>> + case DRM_MODE_ENCODER_DSI:
>> + /* No MIPI DSI on A31s */
>> + if (of_device_is_compatible(tcon->dev->of_node,
>> + "allwinner,sun6i-a31s-tcon"))
>
> I'm not sure that test is needed.
>
> We won't end up in that case if we don't have a connected DSI block,
> which isn't going to be the case on the A31. And I guess we can tackle
> DSI later (when I'll send my patches...).

OK. I'll leave a comment instead.

>
>> + return -EINVAL;
>> + shift = 0;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + regmap_update_bits(tcon->regs, SUN4I_TCON_MUX_CTRL_REG,
>> +0x3 << shift, tcon_id << shift);
>> +
>> + return 0;
>> +}
>> +
>>  void sun4i_tcon_set_mux(struct sun4i_tcon *tcon, int channel,
>>   struct drm_encoder *encoder)
>>  {
>> + /* Get the device node of the display engine */
>> + struct device_node *node = encoder->dev->dev->of_node;
>>   u32 val;
>>
>> + if (of_device_is_compatible(node, 
>> "allwinner,sun6i-a31-display-engine") ||
>> + of_device_is_compatible(node, 
>> "allwinner,sun6i-a31s-display-engine")) {
>> + _sun6i_tcon_set_mux(encoder);
>> + return;
>> + }
>> +
>
> I'd really like to avoid mix and matching the structure defined
> behaviour and those of_device_is_compatible calls spread out
> everywhere.
>
> You can either add a flag or a function pointer.

Function pointer it is!

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

Re: [PATCH v2 02/13] clk: sunxi-ng: sun6i: Rename HDMI DDC clock to avoid name collision

2017-09-26 Thread Chen-Yu Tsai 
On Tue, Sep 26, 2017 at 5:32 PM, Maxime Ripard
 wrote:
> On Tue, Sep 26, 2017 at 06:59:08AM +, Chen-Yu Tsai wrote:
>> The HDMI DDC clock found in the CCU is the parent of the actual DDC
>> clock within the HDMI controller. That clock is also named "hdmi-ddc".
>>
>> Rename the one in the CCU to "hdmi-ddc-parent". This makes more sense
>> than renaming the one in the HDMI controller to something else.
>>
>> Fixes: c6e6c96d8fa6 ("clk: sunxi-ng: Add A31/A31s clocks")
>> Signed-off-by: Chen-Yu Tsai 
>
> I'd rather stick to the datasheet names. What about "DDC" ?

Works for me.

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

[radeon-alex:drm-next-4.15-dc 148/1063] drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/rv_hwmgr.c:845:49-50: asic_setup: first occurrence line 848, second occurrence line 875

2017-09-26 Thread Julia Lawall 
There are two initializations of .asic_setup in this structure.

julia

-- Forwarded message --
Date: Wed, 27 Sep 2017 10:16:08 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: [radeon-alex:drm-next-4.15-dc 148/1063]
drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/rv_hwmgr.c:845:49-50:
asic_setup: first occurrence line 848, second occurrence line 875


tree:   git://people.freedesktop.org/~agd5f/linux.git drm-next-4.15-dc
head:   dfbf0c14dd75d3b15f65478f10f373aa83042a50
commit: cf2623d951c1c52923a776e01cf2e2afc9d042a0 [148/1063] drm/amd/powerplay: 
refine powerplay code for RV
:: branch date: 4 hours ago
:: commit date: 8 days ago

>> drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/rv_hwmgr.c:845:49-50: 
>> asic_setup: first occurrence line 848, second occurrence line 875

git remote add radeon-alex git://people.freedesktop.org/~agd5f/linux.git
git remote update radeon-alex
git checkout cf2623d951c1c52923a776e01cf2e2afc9d042a0
vim +845 drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/rv_hwmgr.c

a960d61cb Rex Zhu 2017-05-11  844
a960d61cb Rex Zhu 2017-05-11 @845  static const struct pp_hwmgr_func 
rv_hwmgr_funcs = {
a960d61cb Rex Zhu 2017-05-11  846   .backend_init = rv_hwmgr_backend_init,
a960d61cb Rex Zhu 2017-05-11  847   .backend_fini = rv_hwmgr_backend_fini,
a960d61cb Rex Zhu 2017-05-11 @848   .asic_setup = NULL,
a960d61cb Rex Zhu 2017-05-11  849   .apply_state_adjust_rules = 
rv_apply_state_adjust_rules,
a960d61cb Rex Zhu 2017-05-11  850   .force_dpm_level = 
rv_dpm_force_dpm_level,
a960d61cb Rex Zhu 2017-05-11  851   .get_power_state_size = 
rv_get_power_state_size,
a960d61cb Rex Zhu 2017-05-11  852   .powerdown_uvd = NULL,
a960d61cb Rex Zhu 2017-05-11  853   .powergate_uvd = NULL,
a960d61cb Rex Zhu 2017-05-11  854   .powergate_vce = NULL,
a960d61cb Rex Zhu 2017-05-11  855   .get_mclk = rv_dpm_get_mclk,
a960d61cb Rex Zhu 2017-05-11  856   .get_sclk = rv_dpm_get_sclk,
a960d61cb Rex Zhu 2017-05-11  857   .patch_boot_state = 
rv_dpm_patch_boot_state,
a960d61cb Rex Zhu 2017-05-11  858   .get_pp_table_entry = 
rv_dpm_get_pp_table_entry,
a960d61cb Rex Zhu 2017-05-11  859   .get_num_of_pp_table_entries = 
rv_dpm_get_num_of_pp_table_entries,
a960d61cb Rex Zhu 2017-05-11  860   .set_cpu_power_state = 
rv_set_cpu_power_state,
a960d61cb Rex Zhu 2017-05-11  861   .store_cc6_data = rv_store_cc6_data,
a960d61cb Rex Zhu 2017-05-11  862   .force_clock_level = 
rv_force_clock_level,
a960d61cb Rex Zhu 2017-05-11  863   .print_clock_levels = 
rv_print_clock_levels,
a960d61cb Rex Zhu 2017-05-11  864   .get_dal_power_level = 
rv_get_dal_power_level,
a960d61cb Rex Zhu 2017-05-11  865   .get_performance_level = 
rv_get_performance_level,
a960d61cb Rex Zhu 2017-05-11  866   .get_current_shallow_sleep_clocks = 
rv_get_current_shallow_sleep_clocks,
a960d61cb Rex Zhu 2017-05-11  867   .get_clock_by_type_with_latency = 
rv_get_clock_by_type_with_latency,
a960d61cb Rex Zhu 2017-05-11  868   .get_clock_by_type_with_voltage = 
rv_get_clock_by_type_with_voltage,
a960d61cb Rex Zhu 2017-05-11  869   .get_max_high_clocks = 
rv_get_max_high_clocks,
a960d61cb Rex Zhu 2017-05-11  870   .read_sensor = rv_read_sensor,
841e3be12 Rex Zhu 2017-08-25  871   .set_active_display_count = 
rv_set_active_display_count,
841e3be12 Rex Zhu 2017-08-25  872   .set_deep_sleep_dcefclk = 
rv_set_deep_sleep_dcefclk,
cf2623d95 Rex Zhu 2017-09-04  873   .dynamic_state_management_enable = 
rv_enable_dpm_tasks,
cf2623d95 Rex Zhu 2017-09-04  874   .power_off_asic = rv_power_off_asic,
cf2623d95 Rex Zhu 2017-09-04 @875   .asic_setup = rv_setup_asic_task,
cf2623d95 Rex Zhu 2017-09-04  876   .power_state_set = 
rv_set_power_state_tasks,
cf2623d95 Rex Zhu 2017-09-04  877   .dynamic_state_management_disable = 
rv_disable_dpm_tasks,
a960d61cb Rex Zhu 2017-05-11  878  };
a960d61cb Rex Zhu 2017-05-11  879

:: The code at line 845 was first introduced by commit
:: a960d61cbd62544c04adb4fe6513577601ff4535 drm/amd/powerplay: add raven 
support in hwmgr. (v2)

:: TO: Rex Zhu 
:: CC: Alex Deucher 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

<    1   2