Re: [PATCH] gma500:Remove functions that are now deprecated and move to the newer functions in drm_dp_helper.c

From: patrik . r . jakobsson
Date: Sun May 10 2015 - 14:36:18 EST

Next message: Alexander Kuleshov: "[PATCH 0/2] x86/earlyprintk: setup earlyprintk as early as possible"
Previous message: Dan Williams: "Re: "Directly mapped persistent memory page cache""
In reply to: patrik . r . jakobsson: "Re: [PATCH] gma500:Remove functions that are now deprecated and move to the newer functions in drm_dp_helper.c"
Next in thread: patrik . r . jakobsson: "Re: [PATCH] gma500:Remove functions that are now deprecated and move to the newer functions in drm_dp_helper.c"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Sun, May 10, 2015 at 01:48:14PM -0400, nick wrote:
>
>
> On 2015-05-10 01:04 PM, patrik.r.jakobsson@xxxxxxxxx wrote:
> > On Tue, May 05, 2015 at 11:17:07AM -0400, nick wrote:
> >>
> >>
> >> On 2015-05-05 09:06 AM, Patrik Jakobsson wrote:
> >>> On Tue, May 5, 2015 at 12:29 AM, Nicholas Krause <xerofoify@xxxxxxxxx> wrote:
> >>>> This removes the deprecated functions,i2c_dp_aux_add_bus and
> >>>> i2c_dp_aux_prepare_bus and the only call in the function,
> >>>> cdv_intel_dp_i2c_init to i2c_dp_aux_add_bus respectfully.
> >>>> The call and use of these functions is now replaced alongside
> >>>> the removal of setting other values in the function,cdv_intel_dp_i2c_init
> >>>> to use the helper function, drm_dp_aux_register that can handle all this
> >>>> work internally.
> >>>
> >>> You need to fill in the drm_dp_aux structure and implement a proper transfer
> >>> function. This patch would only break things. But the cdv dp output is already
> >>> broken on my system so it needs fixing first.
> >>>
> >>> -Patrik
> >>>
> >> Patrik,
> >> Due to me not being an expert in this area of the kernel I based my work off the similar function,
> >> intel_dp_aux_init for i915. I was unsure of which helper functions for i915 need to be written
> >> differently for gma500 based solutions as I don't known the differences between these two in the
> >> graphics specs area.
> >> Thanks,
> >> Nick
> >
> > Hi Nick
> > You are _required_ to know what your patches are doing before sending them. If
> > you don't test your code and don't know what it's doing then we will not accept
> > it. You're expected to read the available documentation and relevant literature
> > before sending patches and asking questions. Otherwise someone else is doing the
> > work for you, which I assume is not the purpose of you patch submission.
> >
> > Thanks
> > Patrik
> Patrik,
> Sorry about that my question is actually this ,I am unsure of what parts of the function,intel_dp_aux_init
> need to be ported for the gma500 gpu driver. Furthermore I will list what parts for certain I known need to
> be ported over above the lines of code in the function definition I have pasted below. Please let me known
> if there are any areas I am missing. This is more just of a double check to make sure I don't miss something
> critical:)(I like to verify something before I code it).
> Sorry for the misunderstanding,

You can figure this out by reading the drm dp helpers docs and looking at
cdv_intel_dp.c. For instance, in gma500 we initialize DP_B and DP_C.

My point is that _you_ need to read the code and understand it. I cannot help
you with that.

But as I said in my first reply. DP on CDV is broken (at least on my machine) so
that should be fixed first.

Cheers
Patrik

> Nick
> static void
> 1002 intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
> 1003 {
> /*Unsure about how to port this over the functions in these lines a.k.a any differences between i915 and gma500 related to these functions?
> I read through these function definitions but would like to double check*/
> 1004 struct drm_device *dev = intel_dp_to_dev(intel_dp);
> 1005 struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> 1006 enum port port = intel_dig_port->port;
> /*Port these lines*/
> 1007 const char *name = NULL;
> 1008 int ret;
> /*Port this switch statement */
> 1009
> 1010 switch (port) {
> 1011 case PORT_A:
> 1012 intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
> 1013 name = "DPDDC-A";
> 1014 break;
> 1015 case PORT_B:
> 1016 intel_dp->aux_ch_ctl_reg = PCH_DPB_AUX_CH_CTL;
> 1017 name = "DPDDC-B";
> 1018 break;
> 1019 case PORT_C:
> 1020 intel_dp->aux_ch_ctl_reg = PCH_DPC_AUX_CH_CTL;
> 1021 name = "DPDDC-C";
> 1022 break;
> 1023 case PORT_D:
> 1024 intel_dp->aux_ch_ctl_reg = PCH_DPD_AUX_CH_CTL;
> 1025 name = "DPDDC-D";
> 1026 break;
> 1027 default:
> 1028 BUG();
> 1029 }
> 1030
> 1031 /*
> 1032 * The AUX_CTL register is usually DP_CTL + 0x10.
> 1033 *
> 1034 * On Haswell and Broadwell though:
> 1035 * - Both port A DDI_BUF_CTL and DDI_AUX_CTL are on the CPU
> 1036 * - Port B/C/D AUX channels are on the PCH, DDI_BUF_CTL on the CPU
> 1037 *
> 1038 * Skylake moves AUX_CTL back next to DDI_BUF_CTL, on the CPU.
> 1039 */
> 1040 if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
> 1041 intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
> 1042
> /*Port these lines*/
> 1043 intel_dp->aux.name = name;
> 1044 intel_dp->aux.dev = dev->dev;
> 1045 intel_dp->aux.transfer = intel_dp_aux_transfer;
> 1046
> /*Port this devugging message */
> 1047 DRM_DEBUG_KMS("registering %s bus for %s\n", name,
> 1048 connector->base.kdev->kobj.name);
> 1049
> /*Port these lines*/
> 1050 ret = drm_dp_aux_register(&intel_dp->aux);
> 1051 if (ret < 0) {
> 1052 DRM_ERROR("drm_dp_aux_register() for %s failed (%d)\n",
> 1053 name, ret);
> 1054 return;
> 1055 }
> 1056
> /*Port the of the function */
> 1057 ret = sysfs_create_link(&connector->base.kdev->kobj,
> 1058 &intel_dp->aux.ddc.dev.kobj,
> 1059 intel_dp->aux.ddc.dev.kobj.name);
> 1060 if (ret < 0) {
> 1061 DRM_ERROR("sysfs_create_link() for %s failed (%d)\n", name, ret);
> 1062 drm_dp_aux_unregister(&intel_dp->aux);
> 1063 }
> 1064 }
> 1065
> >
> >>>> Signed-off-by: Nicholas Krause <xerofoify@xxxxxxxxx>
> >>>> ---
> >>>> drivers/gpu/drm/gma500/cdv_intel_dp.c | 42 ++---------------------------------
> >>>> 1 file changed, 2 insertions(+), 40 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> >>>> index 0fafb8e..c8c20fc 100644
> >>>> --- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
> >>>> +++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> >>>> @@ -200,38 +200,6 @@ static const struct i2c_algorithm i2c_dp_aux_algo = {
> >>>> .functionality = i2c_algo_dp_aux_functionality,
> >>>> };
> >>>>
> >>>> -static void
> >>>> -i2c_dp_aux_reset_bus(struct i2c_adapter *adapter)
> >>>> -{
> >>>> - (void) i2c_algo_dp_aux_address(adapter, 0, false);
> >>>> - (void) i2c_algo_dp_aux_stop(adapter, false);
> >>>> -}
> >>>> -
> >>>> -static int
> >>>> -i2c_dp_aux_prepare_bus(struct i2c_adapter *adapter)
> >>>> -{
> >>>> - adapter->algo = &i2c_dp_aux_algo;
> >>>> - adapter->retries = 3;
> >>>> - i2c_dp_aux_reset_bus(adapter);
> >>>> - return 0;
> >>>> -}
> >>>> -
> >>>> -/*
> >>>> - * FIXME: This is the old dp aux helper, gma500 is the last driver that needs to
> >>>> - * be ported over to the new helper code in drm_dp_helper.c like i915 or radeon.
> >>>> - */
> >>>> -static int __deprecated
> >>>> -i2c_dp_aux_add_bus(struct i2c_adapter *adapter)
> >>>> -{
> >>>> - int error;
> >>>> -
> >>>> - error = i2c_dp_aux_prepare_bus(adapter);
> >>>> - if (error)
> >>>> - return error;
> >>>> - error = i2c_add_adapter(adapter);
> >>>> - return error;
> >>>> -}
> >>>> -
> >>>> #define _wait_for(COND, MS, W) ({ \
> >>>> unsigned long timeout__ = jiffies + msecs_to_jiffies(MS); \
> >>>> int ret__ = 0; \
> >>>> @@ -275,6 +243,7 @@ struct cdv_intel_dp {
> >>>> int backlight_on_delay;
> >>>> int backlight_off_delay;
> >>>> struct drm_display_mode *panel_fixed_mode; /* for eDP */
> >>>> + struct drm_dp_aux aux;
> >>>> bool panel_on;
> >>>> };
> >>>>
> >>>> @@ -855,18 +824,11 @@ cdv_intel_dp_i2c_init(struct gma_connector *connector,
> >>>> intel_dp->algo.running = false;
> >>>> intel_dp->algo.address = 0;
> >>>> intel_dp->algo.aux_ch = cdv_intel_dp_i2c_aux_ch;
> >>>> -
> >>>> memset(&intel_dp->adapter, '\0', sizeof (intel_dp->adapter));
> >>>> - intel_dp->adapter.owner = THIS_MODULE;
> >>>> - intel_dp->adapter.class = I2C_CLASS_DDC;
> >>>> - strncpy (intel_dp->adapter.name, name, sizeof(intel_dp->adapter.name) - 1);
> >>>> - intel_dp->adapter.name[sizeof(intel_dp->adapter.name) - 1] = '\0';
> >>>> - intel_dp->adapter.algo_data = &intel_dp->algo;
> >>>> - intel_dp->adapter.dev.parent = connector->base.kdev;
> >>>>
> >>>> if (is_edp(encoder))
> >>>> cdv_intel_edp_panel_vdd_on(encoder);
> >>>> - ret = i2c_dp_aux_add_bus(&intel_dp->adapter);
> >>>> + ret = drm_dp_aux_register(&intel_dp->aux);
> >>>> if (is_edp(encoder))
> >>>> cdv_intel_edp_panel_vdd_off(encoder);
> >>>>
> >>>> --
> >>>> 2.1.4
> >>>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Alexander Kuleshov: "[PATCH 0/2] x86/earlyprintk: setup earlyprintk as early as possible"
Previous message: Dan Williams: "Re: "Directly mapped persistent memory page cache""
In reply to: patrik . r . jakobsson: "Re: [PATCH] gma500:Remove functions that are now deprecated and move to the newer functions in drm_dp_helper.c"
Next in thread: patrik . r . jakobsson: "Re: [PATCH] gma500:Remove functions that are now deprecated and move to the newer functions in drm_dp_helper.c"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]