Re: [PATCH v2 03/12] drm/i915/fbdev: Move intel_fb_initial_config() to fbdev helper
Den 23.04.2019 16.17, skrev Thomas Zimmermann:
> Hi
>
> Am 07.04.19 um 18:52 schrieb Noralf Trønnes:
>> It is generic code and having it in the helper will let other drivers
>> benefit from it.
>>
>> One change was necessary assuming this to be true:
>> INTEL_INFO(dev_priv)->num_pipes == dev->mode_config.num_crtc
>>
>> Suggested-by: Daniel Vetter
>> Cc: Jani Nikula
>> Cc: Joonas Lahtinen
>> Cc: Rodrigo Vivi
>> Cc: intel-...@lists.freedesktop.org
>> Signed-off-by: Noralf Trønnes
>> Reviewed-by: Jani Nikula
>> ---
>> drivers/gpu/drm/drm_fb_helper.c| 194 -
>> drivers/gpu/drm/i915/intel_fbdev.c | 218 -
>> include/drm/drm_fb_helper.h| 23 ---
>> 3 files changed, 190 insertions(+), 245 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index a6be09ae899b..eda8b63f350d 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -2556,6 +2556,194 @@ static void drm_setup_crtc_rotation(struct
>> drm_fb_helper *fb_helper,
>> fb_helper->sw_rotations |= DRM_MODE_ROTATE_0;
>> }
>>
>> +static struct drm_fb_helper_crtc *
>> +drm_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
>> +{
>> +int i;
>> +
>> +for (i = 0; i < fb_helper->crtc_count; i++)
>> +if (fb_helper->crtc_info[i].mode_set.crtc == crtc)
>> +return _helper->crtc_info[i];
>> +
>> +return NULL;
>> +}
>> +
>> +/* Try to read the BIOS display configuration and use it for the initial
>> config */
>> +static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper,
>> + struct drm_fb_helper_crtc **crtcs,
>> + struct drm_display_mode **modes,
>> + struct drm_fb_offset *offsets,
>> + bool *enabled, int width, int height)
>> +{
>> +struct drm_device *dev = fb_helper->dev;
>> +unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG);
>> +unsigned long conn_configured, conn_seq;
>> +int i, j;
>> +bool *save_enabled;
>> +bool fallback = true, ret = true;
>> +int num_connectors_enabled = 0;
>> +int num_connectors_detected = 0;
>> +struct drm_modeset_acquire_ctx ctx;
>> +
>> +save_enabled = kcalloc(count, sizeof(bool), GFP_KERNEL);
>> +if (!save_enabled)
>> +return false;
>> +
>> +drm_modeset_acquire_init(, 0);
>> +
>> +while (drm_modeset_lock_all_ctx(dev, ) != 0)
>> +drm_modeset_backoff();
>> +
>> +memcpy(save_enabled, enabled, count);
>> +conn_seq = GENMASK(count - 1, 0);
>> +conn_configured = 0;
>> +retry:
>> +for (i = 0; i < count; i++) {
>> +struct drm_fb_helper_connector *fb_conn;
>> +struct drm_connector *connector;
>> +struct drm_encoder *encoder;
>> +struct drm_fb_helper_crtc *new_crtc;
>> +
>> +fb_conn = fb_helper->connector_info[i];
>> +connector = fb_conn->connector;
>> +
>> +if (conn_configured & BIT(i))
>> +continue;
>> +
>> +/* First pass, only consider tiled connectors */
>> +if (conn_seq == GENMASK(count - 1, 0) && !connector->has_tile)
>> +continue;
>> +
>> +if (connector->status == connector_status_connected)
>> +num_connectors_detected++;
>> +
>> +if (!enabled[i]) {
>> +DRM_DEBUG_KMS("connector %s not enabled, skipping\n",
>> + connector->name);
>> +conn_configured |= BIT(i);
>> +continue;
>> +}
>> +
>> +if (connector->force == DRM_FORCE_OFF) {
>> +DRM_DEBUG_KMS("connector %s is disabled by user,
>> skipping\n",
>> + connector->name);
>> +enabled[i] = false;
>> +continue;
>> +}
>> +
>> +encoder = connector->state->best_encoder;
>
> This patch breaks ast right here. Ast is a non-atomic driver and has
> connector->state set to NULL. Reading best_encoder gives a NULL-pointer
> dereference and the display turns black. Stack trace is below.
>
Thanks for the report, I've sent a fix.
Noralf.
> Best regards
> Thomas
>
>
> [ 29.583012] [drm:drm_fb_helper_firmware_config.isra.37
> [drm_kms_helper]] *ERROR* drivers/gpu/drm/drm_fb_helper.c:2649
> connector=db73d0b1
> [ 29.583048] [drm:drm_fb_helper_firmware_config.isra.37
> [drm_kms_helper]] *ERROR* drivers/gpu/drm/drm_fb_helper.c:2650
> connector->state= (null)
> 0m] Found device[ 29.609593] BUG: unable to handle kernel NULL pointer
> dereference at 0010
> [ 29.609594] #PF error: [normal kernel read fault]
> [ 29.609595] PGD 0 P4D 0
> [ 29.609597] Oops:
Re: [PATCH v2 03/12] drm/i915/fbdev: Move intel_fb_initial_config() to fbdev helper
Hi
Am 07.04.19 um 18:52 schrieb Noralf Trønnes:
> It is generic code and having it in the helper will let other drivers
> benefit from it.
>
> One change was necessary assuming this to be true:
> INTEL_INFO(dev_priv)->num_pipes == dev->mode_config.num_crtc
>
> Suggested-by: Daniel Vetter
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Rodrigo Vivi
> Cc: intel-...@lists.freedesktop.org
> Signed-off-by: Noralf Trønnes
> Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/drm_fb_helper.c| 194 -
> drivers/gpu/drm/i915/intel_fbdev.c | 218 -
> include/drm/drm_fb_helper.h| 23 ---
> 3 files changed, 190 insertions(+), 245 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index a6be09ae899b..eda8b63f350d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2556,6 +2556,194 @@ static void drm_setup_crtc_rotation(struct
> drm_fb_helper *fb_helper,
> fb_helper->sw_rotations |= DRM_MODE_ROTATE_0;
> }
>
> +static struct drm_fb_helper_crtc *
> +drm_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
> +{
> + int i;
> +
> + for (i = 0; i < fb_helper->crtc_count; i++)
> + if (fb_helper->crtc_info[i].mode_set.crtc == crtc)
> + return _helper->crtc_info[i];
> +
> + return NULL;
> +}
> +
> +/* Try to read the BIOS display configuration and use it for the initial
> config */
> +static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper,
> + struct drm_fb_helper_crtc **crtcs,
> + struct drm_display_mode **modes,
> + struct drm_fb_offset *offsets,
> + bool *enabled, int width, int height)
> +{
> + struct drm_device *dev = fb_helper->dev;
> + unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG);
> + unsigned long conn_configured, conn_seq;
> + int i, j;
> + bool *save_enabled;
> + bool fallback = true, ret = true;
> + int num_connectors_enabled = 0;
> + int num_connectors_detected = 0;
> + struct drm_modeset_acquire_ctx ctx;
> +
> + save_enabled = kcalloc(count, sizeof(bool), GFP_KERNEL);
> + if (!save_enabled)
> + return false;
> +
> + drm_modeset_acquire_init(, 0);
> +
> + while (drm_modeset_lock_all_ctx(dev, ) != 0)
> + drm_modeset_backoff();
> +
> + memcpy(save_enabled, enabled, count);
> + conn_seq = GENMASK(count - 1, 0);
> + conn_configured = 0;
> +retry:
> + for (i = 0; i < count; i++) {
> + struct drm_fb_helper_connector *fb_conn;
> + struct drm_connector *connector;
> + struct drm_encoder *encoder;
> + struct drm_fb_helper_crtc *new_crtc;
> +
> + fb_conn = fb_helper->connector_info[i];
> + connector = fb_conn->connector;
> +
> + if (conn_configured & BIT(i))
> + continue;
> +
> + /* First pass, only consider tiled connectors */
> + if (conn_seq == GENMASK(count - 1, 0) && !connector->has_tile)
> + continue;
> +
> + if (connector->status == connector_status_connected)
> + num_connectors_detected++;
> +
> + if (!enabled[i]) {
> + DRM_DEBUG_KMS("connector %s not enabled, skipping\n",
> + connector->name);
> + conn_configured |= BIT(i);
> + continue;
> + }
> +
> + if (connector->force == DRM_FORCE_OFF) {
> + DRM_DEBUG_KMS("connector %s is disabled by user,
> skipping\n",
> + connector->name);
> + enabled[i] = false;
> + continue;
> + }
> +
> + encoder = connector->state->best_encoder;
This patch breaks ast right here. Ast is a non-atomic driver and has
connector->state set to NULL. Reading best_encoder gives a NULL-pointer
dereference and the display turns black. Stack trace is below.
Best regards
Thomas
[ 29.583012] [drm:drm_fb_helper_firmware_config.isra.37
[drm_kms_helper]] *ERROR* drivers/gpu/drm/drm_fb_helper.c:2649
connector=db73d0b1
[ 29.583048] [drm:drm_fb_helper_firmware_config.isra.37
[drm_kms_helper]] *ERROR* drivers/gpu/drm/drm_fb_helper.c:2650
connector->state= (null)
0m] Found device[ 29.609593] BUG: unable to handle kernel NULL pointer
dereference at 0010
[ 29.609594] #PF error: [normal kernel read fault]
[ 29.609595] PGD 0 P4D 0
[ 29.609597] Oops: [#1] SMP PTI
[ 29.609599] CPU: 1 PID: 354 Comm: systemd-udevd Tainted: G
E 5.1.0-rc5-1-default+ #11
[ 29.609600] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN
FIRE X2270 M2,
Re: [PATCH v2 03/12] drm/i915/fbdev: Move intel_fb_initial_config() to fbdev helper
Den 07.04.2019 18.52, skrev Noralf Trønnes: > It is generic code and having it in the helper will let other drivers > benefit from it. > > One change was necessary assuming this to be true: > INTEL_INFO(dev_priv)->num_pipes == dev->mode_config.num_crtc > > Suggested-by: Daniel Vetter > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Rodrigo Vivi > Cc: intel-...@lists.freedesktop.org > Signed-off-by: Noralf Trønnes > Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/drm_fb_helper.c| 194 - > drivers/gpu/drm/i915/intel_fbdev.c | 218 - > include/drm/drm_fb_helper.h| 23 --- > 3 files changed, 190 insertions(+), 245 deletions(-) > Applied to drm-misc-next. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
