Re: [Intel-gfx] [PATCH] drm/atomic: Allow for holes in connector state.

2016-02-22 Thread Maarten Lankhorst 
Op 19-02-16 om 04:21 schreef Dave Airlie:
> On 16 February 2016 at 21:37, Ville Syrjälä
>  wrote:
>> On Mon, Feb 15, 2016 at 02:17:01PM +0100, Maarten Lankhorst wrote:
>>> Because we record connector_mask using 1 << drm_connector_index now
>>> the connector_mask should stay the same even when other connectors
>>> are removed. This was not the case with MST, in that case when removing
>>> a connector all other connectors may change their index.
>>>
>>> This is fixed by waiting until the first get_connector_state to allocate
>>> connector_state, and force reallocation when state is too small.
>>>
>>> As a side effect connector arrays no longer have to be preallocated,
>>> and can be allocated on first use which means a less allocations in
>>> the page flip only path.
> Daniel you said something on irc about v2 of this for -fixes? Did I miss v2?
>
> Dave.
"[PATCH v2] drm/atomic: Allow for holes in connector state, v2."

It wasn't sent in this thread to give CI a chance to run.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/atomic: Allow for holes in connector state.

2016-02-18 Thread Dave Airlie 
On 16 February 2016 at 21:37, Ville Syrjälä
 wrote:
> On Mon, Feb 15, 2016 at 02:17:01PM +0100, Maarten Lankhorst wrote:
>> Because we record connector_mask using 1 << drm_connector_index now
>> the connector_mask should stay the same even when other connectors
>> are removed. This was not the case with MST, in that case when removing
>> a connector all other connectors may change their index.
>>
>> This is fixed by waiting until the first get_connector_state to allocate
>> connector_state, and force reallocation when state is too small.
>>
>> As a side effect connector arrays no longer have to be preallocated,
>> and can be allocated on first use which means a less allocations in
>> the page flip only path.

Daniel you said something on irc about v2 of this for -fixes? Did I miss v2?

Dave.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/atomic: Allow for holes in connector state.

2016-02-16 Thread Ville Syrjälä 
On Mon, Feb 15, 2016 at 02:17:01PM +0100, Maarten Lankhorst wrote:
> Because we record connector_mask using 1 << drm_connector_index now
> the connector_mask should stay the same even when other connectors
> are removed. This was not the case with MST, in that case when removing
> a connector all other connectors may change their index.
> 
> This is fixed by waiting until the first get_connector_state to allocate
> connector_state, and force reallocation when state is too small.
> 
> As a side effect connector arrays no longer have to be preallocated,
> and can be allocated on first use which means a less allocations in
> the page flip only path.
> 
> Fixes: 14de6c44d149 ("drm/atomic: Remove drm_atomic_connectors_for_crtc.")
> Signed-off-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/drm_atomic.c| 45 
> +
>  drivers/gpu/drm/drm_atomic_helper.c |  2 +-
>  drivers/gpu/drm/drm_crtc.c  | 45 
> +
>  include/drm/drm_crtc.h  |  8 ++-
>  4 files changed, 45 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 8fb469c4e4b8..5d4774450540 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -65,8 +65,6 @@ drm_atomic_state_init(struct drm_device *dev, struct 
> drm_atomic_state *state)
>*/
>   state->allow_modeset = true;
>  
> - state->num_connector = ACCESS_ONCE(dev->mode_config.num_connector);
> -
>   state->crtcs = kcalloc(dev->mode_config.num_crtc,
>  sizeof(*state->crtcs), GFP_KERNEL);
>   if (!state->crtcs)
> @@ -83,16 +81,6 @@ drm_atomic_state_init(struct drm_device *dev, struct 
> drm_atomic_state *state)
> sizeof(*state->plane_states), GFP_KERNEL);
>   if (!state->plane_states)
>   goto fail;
> - state->connectors = kcalloc(state->num_connector,
> - sizeof(*state->connectors),
> - GFP_KERNEL);
> - if (!state->connectors)
> - goto fail;
> - state->connector_states = kcalloc(state->num_connector,
> -   sizeof(*state->connector_states),
> -   GFP_KERNEL);
> - if (!state->connector_states)
> - goto fail;
>  
>   state->dev = dev;
>  
> @@ -823,19 +811,28 @@ drm_atomic_get_connector_state(struct drm_atomic_state 
> *state,
>  
>   index = drm_connector_index(connector);
>  
> - /*
> -  * Construction of atomic state updates can race with a connector
> -  * hot-add which might overflow. In this case flip the table and just
> -  * restart the entire ioctl - no one is fast enough to livelock a cpu
> -  * with physical hotplug events anyway.
> -  *
> -  * Note that we only grab the indexes once we have the right lock to
> -  * prevent hotplug/unplugging of connectors. So removal is no problem,
> -  * at most the array is a bit too large.
> -  */
>   if (index >= state->num_connector) {
> - DRM_DEBUG_ATOMIC("Hot-added connector would overflow state 
> array, restarting\n");
> - return ERR_PTR(-EAGAIN);
> + struct drm_connector **c;
> + struct drm_connector_state **cs;
> +
> + u32 alloc = max(index + 1, config->num_connector);

Why u32?

> +
> + c = krealloc(state->connectors, alloc * 
> sizeof(*state->connectors), GFP_KERNEL);
> + if (!c)
> + return ERR_PTR(-ENOMEM);
> +
> + state->connectors = c;
> + memset(>connectors[state->num_connector], 0,
> +sizeof(*state->connectors) * (alloc - 
> state->num_connector));
> +
> + cs = krealloc(state->connector_states, alloc * 
> sizeof(*state->connector_states), GFP_KERNEL);
> + if (!cs)
> + return ERR_PTR(-ENOMEM);
> +
> + state->connector_states = cs;
> + memset(>connector_states[state->num_connector], 0,
> +sizeof(*state->connector_states) * (alloc - 
> state->num_connector));
> + state->num_connector = alloc;
>   }
>  
>   if (state->connector_states[index])
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 2b430b05f35d..4da4f2a49078 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1535,7 +1535,7 @@ void drm_atomic_helper_swap_state(struct drm_device 
> *dev,
>  {
>   int i;
>  
> - for (i = 0; i < dev->mode_config.num_connector; i++) {
> + for (i = 0; i < state->num_connector; i++) {
>   struct drm_connector *connector = state->connectors[i];
>  
>   if (!connector)
> diff --git a/drivers/gpu/drm/drm_crtc.c

Re: [Intel-gfx] [PATCH] drm/atomic: Allow for holes in connector state.

2016-02-15 Thread kbuild test robot 
Hi Maarten,

[auto build test WARNING on drm/drm-next]
[also build test WARNING on v4.5-rc4 next-20160215]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Maarten-Lankhorst/drm-atomic-Allow-for-holes-in-connector-state/20160215-212056
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for 
parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for 
parameter 'fmt'
   include/drm/drm_crtc.h:359: warning: No description found for parameter 
'mode_blob'
   include/drm/drm_crtc.h:774: warning: No description found for parameter 
'name'
>> include/drm/drm_crtc.h:1233: warning: No description found for parameter 
>> 'connector_id'
   include/drm/drm_crtc.h:1233: warning: No description found for parameter 
'tile_blob_ptr'
   include/drm/drm_crtc.h:1272: warning: No description found for parameter 
'rotation'
   include/drm/drm_crtc.h:1534: warning: No description found for parameter 
'name'
   include/drm/drm_crtc.h:1534: warning: No description found for parameter 
'mutex'
   include/drm/drm_crtc.h:1534: warning: No description found for parameter 
'helper_private'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tile_idr'
>> include/drm/drm_crtc.h:2144: warning: No description found for parameter 
>> 'connector_ida'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'delayed_event'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'edid_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'dpms_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'path_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tile_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'plane_type_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'rotation_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_src_x'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_src_y'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_src_w'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_src_h'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_crtc_x'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_crtc_y'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_crtc_w'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_crtc_h'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_fb_id'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_crtc_id'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_active'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'prop_mode_id'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'dvi_i_subconnector_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'dvi_i_select_subconnector_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_subconnector_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_select_subconnector_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_mode_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_left_margin_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_right_margin_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_top_margin_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_bottom_margin_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_brightness_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_contrast_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_flicker_reduction_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_overscan_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_saturation_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'tv_hue_property'
   include/drm/drm_crtc.h:2144: warning: No description found for parameter 
'scaling_mode_property'
   include/drm/drm_crtc.h:2144: warning:

[Intel-gfx] [PATCH] drm/atomic: Allow for holes in connector state.

2016-02-15 Thread Maarten Lankhorst 
Because we record connector_mask using 1 << drm_connector_index now
the connector_mask should stay the same even when other connectors
are removed. This was not the case with MST, in that case when removing
a connector all other connectors may change their index.

This is fixed by waiting until the first get_connector_state to allocate
connector_state, and force reallocation when state is too small.

As a side effect connector arrays no longer have to be preallocated,
and can be allocated on first use which means a less allocations in
the page flip only path.

Fixes: 14de6c44d149 ("drm/atomic: Remove drm_atomic_connectors_for_crtc.")
Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic.c| 45 +
 drivers/gpu/drm/drm_atomic_helper.c |  2 +-
 drivers/gpu/drm/drm_crtc.c  | 45 +
 include/drm/drm_crtc.h  |  8 ++-
 4 files changed, 45 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8fb469c4e4b8..5d4774450540 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -65,8 +65,6 @@ drm_atomic_state_init(struct drm_device *dev, struct 
drm_atomic_state *state)
 */
state->allow_modeset = true;
 
-   state->num_connector = ACCESS_ONCE(dev->mode_config.num_connector);
-
state->crtcs = kcalloc(dev->mode_config.num_crtc,
   sizeof(*state->crtcs), GFP_KERNEL);
if (!state->crtcs)
@@ -83,16 +81,6 @@ drm_atomic_state_init(struct drm_device *dev, struct 
drm_atomic_state *state)
  sizeof(*state->plane_states), GFP_KERNEL);
if (!state->plane_states)
goto fail;
-   state->connectors = kcalloc(state->num_connector,
-   sizeof(*state->connectors),
-   GFP_KERNEL);
-   if (!state->connectors)
-   goto fail;
-   state->connector_states = kcalloc(state->num_connector,
- sizeof(*state->connector_states),
- GFP_KERNEL);
-   if (!state->connector_states)
-   goto fail;
 
state->dev = dev;
 
@@ -823,19 +811,28 @@ drm_atomic_get_connector_state(struct drm_atomic_state 
*state,
 
index = drm_connector_index(connector);
 
-   /*
-* Construction of atomic state updates can race with a connector
-* hot-add which might overflow. In this case flip the table and just
-* restart the entire ioctl - no one is fast enough to livelock a cpu
-* with physical hotplug events anyway.
-*
-* Note that we only grab the indexes once we have the right lock to
-* prevent hotplug/unplugging of connectors. So removal is no problem,
-* at most the array is a bit too large.
-*/
if (index >= state->num_connector) {
-   DRM_DEBUG_ATOMIC("Hot-added connector would overflow state 
array, restarting\n");
-   return ERR_PTR(-EAGAIN);
+   struct drm_connector **c;
+   struct drm_connector_state **cs;
+
+   u32 alloc = max(index + 1, config->num_connector);
+
+   c = krealloc(state->connectors, alloc * 
sizeof(*state->connectors), GFP_KERNEL);
+   if (!c)
+   return ERR_PTR(-ENOMEM);
+
+   state->connectors = c;
+   memset(>connectors[state->num_connector], 0,
+  sizeof(*state->connectors) * (alloc - 
state->num_connector));
+
+   cs = krealloc(state->connector_states, alloc * 
sizeof(*state->connector_states), GFP_KERNEL);
+   if (!cs)
+   return ERR_PTR(-ENOMEM);
+
+   state->connector_states = cs;
+   memset(>connector_states[state->num_connector], 0,
+  sizeof(*state->connector_states) * (alloc - 
state->num_connector));
+   state->num_connector = alloc;
}
 
if (state->connector_states[index])
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 2b430b05f35d..4da4f2a49078 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1535,7 +1535,7 @@ void drm_atomic_helper_swap_state(struct drm_device *dev,
 {
int i;
 
-   for (i = 0; i < dev->mode_config.num_connector; i++) {
+   for (i = 0; i < state->num_connector; i++) {
struct drm_connector *connector = state->connectors[i];
 
if (!connector)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 65258acddb90..ea00aea88de7 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -918,12 +918,18 @@ int drm_connector_init(struct drm_device *dev,