On Wed, 10 Oct 2018, Ville Syrjälä <ville.syrj...@linux.intel.com> wrote:
> On Wed, Oct 10, 2018 at 12:09:16AM +0300, Jani Nikula wrote:
>> For a while we carried lvds connector specific data in the lvds
>> connector, but since commit 05c72e77ccda ("drm/i915: Nuke the LVDS lid
>> notifier") we haven't needed it. Revert back to plain intel_connector.
>> 
>> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_lvds.c | 42 
>> +++++++++++----------------------------
>>  1 file changed, 12 insertions(+), 30 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
>> b/drivers/gpu/drm/i915/intel_lvds.c
>> index 1fe970cf9909..510585ed94b2 100644
>> --- a/drivers/gpu/drm/i915/intel_lvds.c
>> +++ b/drivers/gpu/drm/i915/intel_lvds.c
>> @@ -42,10 +42,6 @@
>>  #include <linux/acpi.h>
>>  
>>  /* Private structure for the integrated LVDS support */
>> -struct intel_lvds_connector {
>> -    struct intel_connector base;
>> -};
>> -
>>  struct intel_lvds_pps {
>>      /* 100us units */
>>      int t1_t2;
>> @@ -70,7 +66,7 @@ struct intel_lvds_encoder {
>>      struct intel_lvds_pps init_pps;
>>      u32 init_lvds_val;
>>  
>> -    struct intel_lvds_connector *attached_connector;
>> +    struct intel_connector *attached_connector;
>>  };
>>  
>>  static struct intel_lvds_encoder *to_lvds_encoder(struct drm_encoder 
>> *encoder)
>> @@ -78,11 +74,6 @@ static struct intel_lvds_encoder *to_lvds_encoder(struct 
>> drm_encoder *encoder)
>>      return container_of(encoder, struct intel_lvds_encoder, base.base);
>>  }
>>  
>> -static struct intel_lvds_connector *to_lvds_connector(struct drm_connector 
>> *connector)
>> -{
>> -    return container_of(connector, struct intel_lvds_connector, base.base);
>> -}
>> -
>>  bool intel_lvds_port_enabled(struct drm_i915_private *dev_priv,
>>                           i915_reg_t lvds_reg, enum pipe *pipe)
>>  {
>> @@ -396,7 +387,7 @@ static bool intel_lvds_compute_config(struct 
>> intel_encoder *intel_encoder,
>>      struct intel_lvds_encoder *lvds_encoder =
>>              to_lvds_encoder(&intel_encoder->base);
>>      struct intel_connector *intel_connector =
>> -            &lvds_encoder->attached_connector->base;
>> +            lvds_encoder->attached_connector;
>>      struct drm_display_mode *adjusted_mode = 
>> &pipe_config->base.adjusted_mode;
>>      struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>>      unsigned int lvds_bpp;
>> @@ -461,15 +452,15 @@ intel_lvds_detect(struct drm_connector *connector, 
>> bool force)
>>   */
>>  static int intel_lvds_get_modes(struct drm_connector *connector)
>>  {
>> -    struct intel_lvds_connector *lvds_connector = 
>> to_lvds_connector(connector);
>> +    struct intel_connector *intel_connector = to_intel_connector(connector);
>>      struct drm_device *dev = connector->dev;
>>      struct drm_display_mode *mode;
>>  
>>      /* use cached edid if we have one */
>> -    if (!IS_ERR_OR_NULL(lvds_connector->base.edid))
>> -            return drm_add_edid_modes(connector, lvds_connector->base.edid);
>> +    if (!IS_ERR_OR_NULL(intel_connector->edid))
>> +            return drm_add_edid_modes(connector, intel_connector->edid);
>>  
>> -    mode = drm_mode_duplicate(dev, lvds_connector->base.panel.fixed_mode);
>> +    mode = drm_mode_duplicate(dev, intel_connector->panel.fixed_mode);
>>      if (mode == NULL)
>>              return 0;
>>  
>> @@ -781,8 +772,7 @@ static bool compute_is_dual_link_lvds(struct 
>> intel_lvds_encoder *lvds_encoder)
>>              return i915_modparams.lvds_channel_mode == 2;
>>  
>>      /* single channel LVDS is limited to 112 MHz */
>> -    if (lvds_encoder->attached_connector->base.panel.fixed_mode->clock
>> -        > 112999)
>> +    if (lvds_encoder->attached_connector->panel.fixed_mode->clock > 112999)
>>              return true;
>>  
>>      if (dmi_check_system(intel_dual_link_lvds))
>> @@ -837,7 +827,6 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>>      struct drm_device *dev = &dev_priv->drm;
>>      struct intel_lvds_encoder *lvds_encoder;
>>      struct intel_encoder *intel_encoder;
>> -    struct intel_lvds_connector *lvds_connector;
>>      struct intel_connector *intel_connector;
>>      struct drm_connector *connector;
>>      struct drm_encoder *encoder;
>> @@ -890,23 +879,16 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>>      if (!lvds_encoder)
>>              return;
>>  
>> -    lvds_connector = kzalloc(sizeof(*lvds_connector), GFP_KERNEL);
>> -    if (!lvds_connector) {
>> -            kfree(lvds_encoder);
>> -            return;
>> -    }
>> -
>> -    if (intel_connector_init(&lvds_connector->base) < 0) {
>> -            kfree(lvds_connector);
>> +    intel_connector = intel_connector_alloc();
>> +    if (!intel_connector) {
>>              kfree(lvds_encoder);
>>              return;
>>      }
>>  
>> -    lvds_encoder->attached_connector = lvds_connector;
>> +    lvds_encoder->attached_connector = intel_connector;
>>  
>>      intel_encoder = &lvds_encoder->base;
>>      encoder = &intel_encoder->base;
>> -    intel_connector = &lvds_connector->base;
>>      connector = &intel_connector->base;
>>      drm_connector_init(dev, &intel_connector->base, 
>> &intel_lvds_connector_funcs,
>>                         DRM_MODE_CONNECTOR_LVDS);
>> @@ -987,7 +969,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>>      } else {
>>              edid = ERR_PTR(-ENOENT);
>>      }
>> -    lvds_connector->base.edid = edid;
>> +    intel_connector->edid = edid;
>>  
>>      list_for_each_entry(scan, &connector->probed_modes, head) {
>>              if (scan->type & DRM_MODE_TYPE_PREFERRED) {
>> @@ -1051,6 +1033,6 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>>      drm_connector_cleanup(connector);
>>      drm_encoder_cleanup(encoder);
>>      kfree(lvds_encoder);
>> -    kfree(lvds_connector);
>> +    intel_connector_free(intel_connector);
>
> I was going to say that this will do a double free on the state, but
> drm_connector_cleanup() & co. memset(0) the object so this should be
> fine.

I'll just note that we do have some work to do in the fail paths in
other connectors...

> Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

Thanks for the review, Chris and Ville. Pushed.

BR,
Jani.


>
>>      return;
>>  }
>> -- 
>> 2.11.0

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to